fix(net): fix RejectedExecutionException during shutdown trxHandlePool#8
fix(net): fix RejectedExecutionException during shutdown trxHandlePool#80xbigapple wants to merge 2 commits intodevelopfrom
Conversation
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
Pull request overview
Fixes shutdown-time RejectedExecutionException in TransactionsMsgHandler by preventing new task submissions after close and changing executor shutdown ordering to stop the scheduler before the worker pool.
Changes:
- Add an
isClosedflag and guardprocessMessage/iteration to stop submitting work during shutdown. - Change shutdown order to terminate
smartContractExecutorbeforetrxHandlePool, and catchRejectedExecutionExceptionon submit. - Add a unit test to ensure
processMessage()does not throw afterclose().
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| framework/src/main/java/org/tron/core/net/messagehandler/TransactionsMsgHandler.java | Adds close-gating and adjusts executor shutdown order to avoid rejected submissions during shutdown. |
| framework/src/test/java/org/tron/core/net/messagehandler/TransactionsMsgHandlerTest.java | Adds a regression test ensuring processMessage is safe after handler close. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
framework/src/main/java/org/tron/core/net/messagehandler/TransactionsMsgHandler.java
Show resolved
Hide resolved
framework/src/test/java/org/tron/core/net/messagehandler/TransactionsMsgHandlerTest.java
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| isClosed = true; | ||
| // Shutdown the producer (scheduler) first. | ||
| // Pending tasks in smartContractQueue will be discarded. | ||
| ExecutorServiceManager.shutdownAndAwaitTermination(smartContractExecutor, smartEsName); | ||
| // Then shutdown the consumer (worker pool) to finish tasks already submitted to trxHandlePool. | ||
| ExecutorServiceManager.shutdownAndAwaitTermination(trxHandlePool, trxEsName); |
There was a problem hiding this comment.
close() comment says pending smartContractQueue tasks will be discarded, but the method only stops the scheduler and shuts down the pools; it never clears/drains smartContractQueue. If this handler remains referenced (it’s a Spring @Component), queued TrxEvents (including PeerConnection references) will be retained until JVM exit. Consider clearing/draining the queue after the scheduler has fully terminated (after shutdownAndAwaitTermination(smartContractExecutor, ...)) to make the discard behavior real and release references.
| // Re-check isClosed status during iteration to handle concurrent shutdown | ||
| if (isClosed) { | ||
| logger.warn("TransactionsMsgHandler is closed during processing, stop submit."); | ||
| break; | ||
| } |
There was a problem hiding this comment.
When isClosed flips to true (or trxHandlePool starts rejecting), this loop breaks early, but the earlier loop has already removed all tx items from peer.getAdvInvRequest(). That can leave some transactions neither queued nor requested anymore. To avoid silently dropping txs, consider removing items from advInvRequest only after a tx is successfully enqueued/submitted (or stop removing once shutdown is detected).
What does this PR do?
fix RejectedExecutionException during shutdown trxHandlePool
Why are these changes required?
This PR has been tested by:
Follow up
Extra details
Summary by cubic
Fixes RejectedExecutionException during shutdown by gating task submissions in
TransactionsMsgHandlerand shutting down the scheduler before the worker pool. Ensures clean shutdown without exceptions after close, dropping only queued smart-contract tasks.isClosedflag to block new tasks and guard iteration.smartContractExecutorfirst, thentrxHandlePool; drop pendingsmartContractQueueitems.processMessagewhen closed; re-check during batch processing.RejectedExecutionExceptionon submit totrxHandlePooland stop submitting.testProcessMessageAfterCloseto ensure messages are ignored afterclose().Written for commit c3e2c56. Summary will update on new commits.