ABin-Huang opened a new pull request, #1061: URL: https://github.com/apache/tomcat/pull/1061
## Problem `ChannelCoordinator.internalStart()` calls `clusterReceiver.start()` and then immediately calls `getChannel().getLocalMember(false)` without waiting for the receiver's background thread to enter the listen loop. This creates a race window where: 1. The `NioReceiver` listener thread may not have executed `setListen(true)` yet 2. The `membershipService` may not have initialized `localMember` 3. The local member's host/port properties may be unset or default values This can cause intermittent cluster communication failures where nodes cannot connect to each other because the advertised host/port is incorrect. The original code had a `// synchronize, big time FIXME` comment at this location. ## Fix Added a readiness signaling mechanism based on `CountDownLatch`: 1. **`ChannelReceiver` interface**: Added `waitForReady(long timeout, TimeUnit unit)` default method that returns `true` immediately (backward compatible). Added `DEFAULT_READY_TIMEOUT_MS` constant. 2. **`NioReceiver`**: Added `volatile CountDownLatch readyLatch`. A fresh latch with count=1 is created in `start()` before launching the listener thread. The latch is counted down in `listen()` after `setListen(true)`. `waitForReady()` is overridden to await the latch. Latch is reset to count=0 in `stopListening()` to support restarts. 3. **`ChannelCoordinator`**: After `clusterReceiver.start()`, calls `waitForReady(5000ms)` before reading `getLocalMember()`. Throws `ChannelException` on timeout or interruption. Removed the FIXME comment. ## Testing Added `TestChannelCoordinatorStartupRace` with 5 test cases: 1. `testWaitForReadyDefaultMethodReturnsImmediately` - Verifies backward compatibility of the default method 2. `testNioReceiverReadyLatchContract` - Verifies CountDownLatch lifecycle via reflection 3. `testChannelCoordinatorWaitsForReceiverBeforeLocalMember` - Verifies exact call order: start() → waitForReady() → getLocalMember() → setLocalMemberProperties() 4. `testChannelCoordinatorThrowsWhenReceiverNotReady` - Verifies ChannelException on timeout and that getLocalMember is NOT called 5. `testChannelCoordinatorHandlesInterruptedException` - Verifies interrupt status is restored and ChannelException is thrown ## Files Changed - `java/org/apache/catalina/tribes/ChannelReceiver.java` - Add waitForReady() default method - `java/org/apache/catalina/tribes/transport/nio/NioReceiver.java` - Add CountDownLatch readiness signaling - `java/org/apache/catalina/tribes/group/ChannelCoordinator.java` - Wait for receiver readiness before reading localMember - `java/org/apache/catalina/tribes/group/LocalStrings.properties` - Add new i18n messages - `test/org/apache/catalina/tribes/group/TestChannelCoordinatorStartupRace.java` - Add unit tests Note: This PR supersedes #1060 which had merge conflicts due to the branch being based on an older main. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
