[Feature] decouple json-rpc filter processing from Manager

[0xbigapple]

Summary

The json-rpc filter event-processing thread currently is located within Manager (core layer), forcing it to hold TronJsonRpcImpl (API layer) via a @Lazy injection — a reverse circular dependency. This proposal moves that thread into TronJsonRpcImpl; Manager only posts filter events into a standalone queue bean, and the reverse edge disappears.

Problem

Motivation

#6732 turned handleLogsFilter/handleBLockFilter from static into instance methods, so Manager's event-processing thread needs a TronJsonRpcImpl instance; the review deferred the resulting @Lazy cycle to a follow-up (discussion) — this issue serves as the dedicated follow-up to address that technical debt.

Current State

Manager ──@Lazy @Autowired field──▶ TronJsonRpcImpl ──@Autowired field──▶ Manager

Each arrow reads "depends on"; the labels are injection styles. TronJsonRpcImpl's constructor parameters Wallet/NodeInfoService each depend on Manager as well, closing the same cycle indirectly.

Manager owns the filter queue, the consumer thread, and its shutdown; the consumer calls back into the API layer.

Limitations or Risks

• FullNode sets allowCircularReferences(false), so a directly injected cycle is rejected at startup; @Lazy (like ObjectProvider or a runtime getBean()) only makes Spring tolerate the cycle — the reverse edge stays, obscuring the component graph and complicating lifecycle changes.
• The reverse core → API edge risks encouraging future changes to repeat the same @Lazy pattern, widening the breach.
• One latent defect remains in the current implementation: the consumer stop flag is a non-volatile boolean (the shutdown-time write has no visibility guarantee for that thread), and the loop does not exit on InterruptedException — under the Java memory model, neither the normal nor the forced stop path is reliable.

Proposed Solution

Proposed Design

Eliminate the reverse dependency entirely rather than suppressing it via @lazy: a standalone FilterCapsuleQueue bean is introduced as an intermediary between the two layers — Manager only produces into it; TronJsonRpcImpl owns the consumer thread (start on @PostConstruct, stop via idempotent close()). ApplicationImpl.shutdown() calls close() explicitly after producers stop and before dbManager.close() — Spring destroys beans only after the database closes, so bean destruction alone would let the consumer outlive it (ApplicationImpl is already the shutdown-orchestrating composition root). Filter API behavior stays unchanged (unbounded queue; pending entries are not drained on shutdown, while an in-flight capsule may complete).

Key Changes

• Module: framework only
• org.tron.common.logsfilter.queue.FilterCapsuleQueue (new bean, beside the existing org.tron.common.logsfilter.capsule package)
• Manager: drop the @Lazy field, the consumer loop, and its thread lifecycle
• TronJsonRpcImpl: constructor gains a Manager parameter, consumer lifecycle, rewritten close() ordering (stop and await the consumer first, then shut down logsFilterPool)
• ApplicationImpl.shutdown(): explicit consumer stop
• No configuration or API changes

Impact

• Stability: fixes the latent stop-path defect above along the way; graceful shutdown stops the consumer before the database closes.
• Performance: no steady-state change expected — processing path, queue, and thread count are unchanged.
• Developer Experience: the core layer no longer references the API layer; framework/src/main becomes @Lazy-free.

Compatibility

• Breaking Change: No
• Default Behavior Change: No — filter delivery, queue semantics, and shutdown semantics are preserved
• Migration Required: No

Additional Notes

• Do you have ideas regarding implementation? Yes
• Are you willing to implement this feature? Yes

[lxcmyf]

Two lifecycle details seem worth making explicit in this refactor:

1. Manager currently starts filterEs only when isJsonRpcFilterEnabled() is true. Since TronJsonRpcImpl is an eager singleton, an unconditional @PostConstruct start would create a consumer thread even when both filter APIs are disabled. The new owner should preserve the same enable gate, and close() should be tested when the consumer was never started.
2. Moving the loop verbatim would preserve its catch (Throwable) behavior, which also swallows TronError, VirtualMachineError, ThreadDeath, and LinkageError. Ordinary per-capsule failures can remain isolated, but process-fatal errors should terminate/propagate according to the node fatal-error policy rather than being logged and followed by another loop iteration. A test distinguishing recoverable RuntimeException from fatal Error would help lock down that boundary.

[0xbigapple]

Two lifecycle details seem worth making explicit in this refactor:

1. Manager currently starts filterEs only when isJsonRpcFilterEnabled() is true. Since TronJsonRpcImpl is an eager singleton, an unconditional @PostConstruct start would create a consumer thread even when both filter APIs are disabled. The new owner should preserve the same enable gate, and close() should be tested when the consumer was never started.
2. Moving the loop verbatim would preserve its catch (Throwable) behavior, which also swallows TronError, VirtualMachineError, ThreadDeath, and LinkageError. Ordinary per-capsule failures can remain isolated, but process-fatal errors should terminate/propagate according to the node fatal-error policy rather than being logged and followed by another loop iteration. A test distinguishing recoverable RuntimeException from fatal Error would help lock down that boundary.

@lxcmyf Thanks for the comments.

1. Yes, we need to add the enable check in the @PostConstruct of TronJsonRpcImpl, so the consumer only starts when isJsonRpcFilterEnabled() is true.

2. I think the error handling should stay as before. This refactor changes ownership and lifecycle, not error handling, so the loop is moved unchanged. The consumer thread only does in-memory matching on the capsule (the database reads happen on the producer side in Manager.postLogsFilter), so no TronError throw site is reachable from it, and a swallowed failure only affects eth_getFilterChanges for that block as before. Handling of VirtualMachineError / ThreadDeath / LinkageError is a node-wide question rather than something specific to this loop, so I would rather leave it out of this issue; we can open a separate one if needed.

[lxcmyf]

Thanks, the enable gate addresses the first concern. I agree that the fatal-error policy can be tracked separately to keep this refactor focused.

One nuance: the absence of an explicit TronError throw site does not make catch (Throwable) harmless—VirtualMachineError, ThreadDeath, and LinkageError can still originate from allocation, class loading, or runtime internals in an in-memory path. Please link a follow-up before merge so this behavior remains visible.

For this issue, the tests should still verify that interruption terminates the consumer and that repeated close() is safe in both enabled and disabled modes.

[0xbigapple]

Thanks, the enable gate addresses the first concern. I agree that the fatal-error policy can be tracked separately to keep this refactor focused.

One nuance: the absence of an explicit TronError throw site does not make catch (Throwable) harmless—VirtualMachineError, ThreadDeath, and LinkageError can still originate from allocation, class loading, or runtime internals in an in-memory path. Please link a follow-up before merge so this behavior remains visible.

For this issue, the tests should still verify that interruption terminates the consumer and that repeated close() is safe in both enabled and disabled modes.

@lxcmyf Thanks for the reply.

For the fatal-error policy, a separate issue needs a detailed analysis and a concrete proposal first, so I will raise it once that is ready.

For the tests: I will add cases covering interruption of the consumer thread and repeated close() in both enabled and disabled modes.

[waynercheung]

+1 on the direction. Explicitly stopping the consumer in ApplicationImpl.shutdown() before dbManager.close() fits the existing shutdown orchestration, including the ordering already used for EventService.

@0xbigapple Thanks for confirming the lifecycle tests. Three details would help make that coverage precise:

1. Repeated close reflects the proposed production sequence. TronJsonRpcImpl implements Closeable, and Spring 5.3.39 infers its close() callback. With the explicit application-level call added, normal shutdown will invoke it again during bean destruction in super.doClose(). It would be useful to cover that sequence: explicit close with the consumer running, followed by context destruction, plus the disabled case.

2. Graceful stop and interruption need separate tests. ExecutorService.shutdown() does not interrupt the running consumer task, even when it is waiting in the capsule queue's poll(). The graceful path therefore needs a reliably visible stop flag. Please cover both an empty-queue consumer stopping without forced cancellation, and interruption during poll() while the stop flag is still true, verifying that the loop exits.

3. Include an in-flight parallel matching case. handleLogsFilter() waits on logsFilterPool.submit(...).join(), which does not return merely because the consumer is interrupted. A latch-based test could verify that graceful shutdown waits for the current capsule before shutting down the matching pool and clearing caches, without draining queued capsules (setFilterParallelThreshold makes the parallel branch reachable from a unit test). Please also clarify what happens if shutdown times out or the closing thread is interrupted, since shutdownAndAwaitTermination() can return without confirming termination.

Happy to review the PR.

[lxcmyf]

That works for me. The planned tests cover the lifecycle concerns for this refactor. Please link the fatal-error follow-up back to this issue when it is opened so the remaining behavior is traceable. No further blocker from my side.