Daniel-Pereira-Linux opened a new pull request, #14542: URL: https://github.com/apache/cloudstack/pull/14542
### Description This PR fixes console proxy threads spinning at 100% CPU in `NoVncClient.waitForNoVNCReply()` when noVNC never replies. `waitForNoVNCReply()` looped on `isWaitForNoVnc()` with no sleep, no timeout and no check that the connection was still alive. If the VNC socket or the websocket was closed before noVNC replied (for example, the VM is stopped or rebooted while the console is connecting), nothing ever cleared the flag and the thread spun forever, until the `cloud` service in the CPVM was restarted. Changes: - `waitForNoVNCReply()` now waits on the existing lock instead of spinning. `setWaitForNoVnc()` and `close()` wake it up. It gives up with a `CloudRuntimeException` when the client is closed, the thread is interrupted, or no reply arrives within 30 seconds. - Since the wait can now fail, the viewer thread in `ConsoleProxyNoVncClient` catches `RuntimeException` as well as `IOException`, and closes the client and the session, so the VNC socket and the websocket are not left open. - The wait flag is now set before sending the message that noVNC replies to (security result and server init). Otherwise a fast reply could be missed and, with the new timeout, a healthy session would be dropped after 30 seconds. Fixes: #14538 ### Types of changes - [ ] Breaking change (fix or feature that would cause existing functionality to change) - [ ] New feature (non-breaking change which adds functionality) - [x] Bug fix (non-breaking change which fixes an issue) - [ ] Enhancement (improves an existing feature and functionality) - [ ] Cleanup (Code refactoring and cleanup, that may add test cases) - [ ] Build/CI - [ ] Test (unit or integration test code) ### Feature/Enhancement Scale or Bug Severity #### Bug Severity - [ ] BLOCKER - [ ] Critical - [ ] Major - [x] Minor - [ ] Trivial ### How Has This Been Tested? Unit tests only. I have **not** tested this on a real CPVM with a KVM VM being restarted during the console connection. - Added `NoVncClientTest` and extended `ConsoleProxyNoVncClientTest`: reply arrives, no wait pending, timeout, client closed, thread interrupted, handshake failure closes the session, and closing the viewer while waiting stops the worker. - Ran the unit tests of `services/console-proxy/server` (with checkstyle) on JDK 21: all pass. - With the previous `NoVncClient`, the new tests fail (including the worker that never stops after `closeClient()`), which reproduces the issue at unit test level. #### How did you try to break this feature and the system with this change? - Closing the client while another thread is waiting, and interrupting the waiting thread. - A noVNC reply that arrives before the message is fully sent (ordering of the wait flag). - A handshake failure in the viewer thread, to check the VNC socket and the websocket are released. Not covered: TLS/VeNCrypt backends, the tunnel and websocket-proxy paths. -- 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]
