fix(connect): recover closed admission coordinator on plugin reload - #146
Merged
Merged
Conversation
A plugin reload (disable -> enable on the same plugin instance) left the parent-scoped BedrockAdmissionCoordinator permanently closed: disable() closes it, while WatcherRegister/WatchClient are recreated per enable in a child injector, so the new watch routed every session proposal into the closed coordinator, throwing IllegalStateException 'Bedroom admission coordinator is closed' which failed the WebSocket and put the watch into an endless reconnect loop until JVM restart. - ConnectPlatform.enable() now resets the shared coordinator before the new cycle's watcher binds: pending admissions are dropped (stale across cycles), the identity registry is reopened, and the cleanup executor that close() shut down is recreated. - WatchClient now rejects a proposal over the wire (SessionRejection) when the coordinator is closed instead of letting the ISE escape and kill the WebSocket stream. Regression tests (RED on unfixed main, GREEN with fix): - ConnectPlatformReloadRecoveryTest: enable() after disable() recovers the coordinator; full enable -> proposal -> disable -> enable -> proposal reload cycle delivers the second proposal instead of the ISE. - WatchClientTest.closedCoordinatorRejectsProposalInsteadOfFailingTheWatch: a proposal into a closed coordinator is rejected on the wire, the watch stream stays up.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes: after a plugin/server reload, EVERY join attempt logs
[connect] Connection error with WatchService: java.lang.IllegalStateException: Bedroom admission coordinator is closedand the watch keeps reconnecting. Full restart fixes it.Root cause
BedrockAdmissionCoordinatoris@Singletonand resolved from the PARENT injector (SpigotPlatformtakes it via@Injectin construction,onLoad). It is a parent-scoped singleton shared by everyenable()cycle.WatcherRegister+WatchClientare bound per-enable in the CHILD injector (WatcherModule,asEagerSingleton).ConnectPlatform.disable()callsadmissionCoordinator.close()(parent scope) — permanent./reloadon servers that reuse the plugin instance), the NEW child-scoped watcher connects but routes proposals into the CLOSED parent coordinator:WatchClient.onMessage→admissionCoordinator.proposal()→ensureOpen()throws ISE → OkHttpprocessNextFrame()catches Exception →failWebSocket→listener.onFailure→watcher.onError→ the exact log line →retry()/reconnect. Repeats on every proposal until JVM restart.Fix
ConnectPlatform.enable()now callsadmissionCoordinator.reset()before the new cycle's watcher binds: pending admissions are dropped (stale across cycles — old session players can never match), theVerifiedBedrockIdentityRegistryis reopened, and the cleanup executor thatclose()shut down is recreated.WatchClient.onMessage: a proposal into a closed coordinator is now rejected over the wire (SessionRejection,INTERNAL: Bedrock admission coordinator is closed) instead of letting the ISE escape and fail the WebSocket — the watch stream stays up and recovers on the next enable, rather than entering an endless reconnect loop.Tests (RED on unfixed main, GREEN with fix)
ConnectPlatformReloadRecoveryTest.enableRecoversCoordinatorClosedByPreviousDisable— enable() after disable() makes the shared coordinator accept proposals again.ConnectPlatformReloadRecoveryTest.reloadCycleDeliversSessionProposalThroughRecoveredCoordinator— full reload cycle (enable → proposal → disable → enable on same parent injector → proposal) delivers the second proposal instead of throwing the ISE.WatchClientTest.closedCoordinatorRejectsProposalInsteadOfFailingTheWatch— a proposal into a closed coordinator is rejected on the wire; the watch stream does not fail.Verified locally: all three tests fail with
IllegalStateException: Bedrock admission coordinator is closedon unfixed main and pass with the fix;./gradlew buildgreen.