Skip to content

fix(net): stop BackupServer.close() hanging while bind is in flight - #6962

Closed
halibobo1205 wants to merge 1 commit into
tronprotocol:developfrom
halibobo1205:fix/backup-server-close-race
Closed

fix(net): stop BackupServer.close() hanging while bind is in flight#6962
halibobo1205 wants to merge 1 commit into
tronprotocol:developfrom
halibobo1205:fix/backup-server-close-race

Conversation

@halibobo1205

Copy link
Copy Markdown
Collaborator

What does this PR do?

Fix a BackupServer shutdown race that can cause BackupServerTest to time out.

Create the event loop group in initServer(), before the server task is submitted, so close() can shut it down while bind is still pending.

close() now interrupts the server executor and waits up to 10 seconds for the event loop group, replacing the ExecutorServiceManager.shutdownAndAwaitTermination call that could wait up to 120 seconds across two termination waits. The server task requests group shutdown in finally without blocking again.

Why are these changes required?

Previously, close() closed the channel only if it had already been published, then called ExecutorServiceManager.shutdownAndAwaitTermination. If bind completed after close() checked the channel, nothing closed the socket, the server thread stayed parked in closeFuture().sync(), and close() blocked in awaitTermination long enough to trip the test's 60-second timeout.

Related CI failure (attempt 2 of that workflow run; the test failed all six attempts—the initial run plus five retries), abbreviated:

org.tron.common.backup.BackupServerTest > test FAILED
    org.junit.runners.model.TestTimedOutException: test timed out after 60 seconds
        ...
        at java.util.concurrent.ThreadPoolExecutor.awaitTermination(ThreadPoolExecutor.java:1475)
        at java.util.concurrent.Executors$DelegatedExecutorService.awaitTermination(Executors.java:675)
        at org.tron.common.es.ExecutorServiceManager.shutdownAndAwaitTermination(ExecutorServiceManager.java:90)
        at org.tron.common.backup.socket.BackupServer.close(BackupServer.java:106)
        at org.tron.common.backup.BackupServerTest.tearDown(BackupServerTest.java:42)

This PR has been tested by:

  • closeAfterStarted: waits for a successfully bound channel, closes the server, and verifies that the channel closes and both the group and executor terminate.
  • closeWhileBindIsPending: deterministically holds bind registration pending, initiates shutdown, then releases the event loop and verifies termination.
  • BackupManagerTest now uses backupServer.close() for cleanup.

Fix a shutdown race where close() runs before bind() publishes the
channel, leaving the server thread blocked until the 60-second executor
timeout.

Create the event loop group in initServer() so close() can shut it down
and interrupt pending waits.
@halibobo1205 halibobo1205 added the topic:net p2p net work, synchronization label Sep 10, 2026
@halibobo1205 halibobo1205 added this to the GreatVoyage-v4.8.3 milestone Sep 10, 2026
@halibobo1205

Copy link
Copy Markdown
Collaborator Author

Fixed by #6961

@halibobo1205 halibobo1205 removed this from the GreatVoyage-v4.8.3 milestone Sep 10, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

topic:net p2p net work, synchronization

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant