Bugfix cm send loop - #288
Conversation
When sending a message, if the send operation fails, trigger a disconnect event to start the reconnection process. Otherwise, sending loop might send messages to a closed socket consuming CPU resources and causing a high CPU usage in a short period of time. Signed-off-by: Mykhailo Lohvynenko <mykhailo_lohvynenko@epam.com>
a3555a8 to
86c8651
Compare
This patch wraps exception as error in close connection, so subscribers will be notified about the connection closure even if web socket shutdown raises an exception. Signed-off-by: Mykhailo Lohvynenko <mykhailo_lohvynenko@epam.com>
|
There was a problem hiding this comment.
Pull request overview
This PR addresses reconnect behavior in the CM communication layer when sends fail during a disconnect/reconnect cycle, and adds a regression test to cover the scenario. It also adjusts shutdown/disconnect handling to ensure subscribers are notified of disconnect events even when graceful shutdown throws.
Changes:
- Add a new
ReconnectOnSendFailuretest that drives concurrent send-queue activity during disconnect/reconnect. - Update connection teardown to always notify disconnect subscribers even if
WebSocket::shutdown()throws, and return the shutdown error. - Attempt to trigger disconnect from the send-queue thread on
Poco::Net::NetException.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
src/cm/communication/tests/communication.cpp |
Adds regression test to reproduce send/reconnect contention scenario. |
src/cm/communication/communication.cpp |
Adjusts disconnect/close behavior and adds disconnect-on-send-failure handling. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| } catch (const Poco::Net::NetException& e) { | ||
| LOG_ERR() << "Failed to send message" << Log::Field(common::utils::ToAosError(e)); | ||
|
|
||
| if (auto err = Disconnect(); !err.IsNone()) { | ||
| LOG_ERR() << "Failed to disconnect from cloud web socket server" << Log::Field(err); | ||
| } |
| } | ||
| }); | ||
|
|
||
| ASSERT_EQ(firstMessageSent.get_future().wait_for(std::chrono::seconds(5)), std::future_status::ready); |
mykola-kobets-epam
left a comment
mykola-kobets-epam
left a comment
There was a problem hiding this comment.
Reviewed-by: Mykola Kobets <mykola_kobets@epam.com>
al1img
left a comment
al1img
left a comment
There was a problem hiding this comment.
Reviewed-by: Oleksandr Grytsov <oleksandr_grytsov@epam.com>
No description provided.