szaszm commented on code in PR #2057:
URL: https://github.com/apache/nifi-minifi-cpp/pull/2057#discussion_r2495219389
##########
libminifi/test/libtest/integration/HTTPHandlers.cpp:
##########
@@ -453,18 +453,32 @@ void HeartbeatHandler::verifySupportedOperations(const
rapidjson::Document& root
REQUIRE(operations ==
std::set<std::string>(magic_enum::enum_names<minifi::c2::Operation>().begin(),
magic_enum::enum_names<minifi::c2::Operation>().end()));
}
-bool StoppingHeartbeatHandler::handlePost(CivetServer *, struct mg_connection
*conn) {
- verify(conn);
- sendStopOperation(conn);
- return true;
-}
-void StoppingHeartbeatHandler::sendStopOperation(struct mg_connection *conn) {
- std::string resp = "{\"operation\" : \"heartbeat\", \"requested_operations\"
: [{ \"operationid\" : 41, \"operation\" : \"stop\", \"operand\" :
\"2438e3c8-015a-1000-79ca-83af40ec1991\" }, "
- "{ \"operationid\" : 42, \"operation\" : \"stop\", \"operand\" :
\"FlowController\" } ]}";
+void StoppingHeartbeatHandler::sendStartStopOperation(struct mg_connection
*conn) {
+ std::lock_guard<std::mutex> lock(post_count_mutex_);
Review Comment:
I got confused by the name: it's not really protecting post_count_ then, but
also ensures in order handling of heartbeats. I think I would handle this
differently (with a state machine), but I can't come up with a good reason why
this is bad, so I'm not objecting to using a mutex for this purpose. But its
name should be changed, since it managed to confuse me, and I find this control
flow surprising, so I think a longer description in a code comment would
benefit future readers, about how this handler works, and what purposes the
mutex serves.
--
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]
