Copilot commented on code in PR #2163:
URL: https://github.com/apache/nifi-minifi-cpp/pull/2163#discussion_r3181341115
##########
libminifi/src/FlowController.cpp:
##########
@@ -130,6 +131,10 @@ nonstd::expected<void, std::string>
FlowController::applyConfiguration(const std
std::lock_guard<std::recursive_mutex> flow_lock(mutex_);
stop();
+ // Clear the controller services now that all timer callback threads have
been stopped
+ clearControllerServices();
+ controller_service_provider_impl_ =
flow_configuration_->getControllerServiceProvider();
Review Comment:
This change is addressing a deadlock/race between flow reload and the
onSchedule retry timer, but there doesn’t appear to be a regression test that
exercises a flow update while the retry timer is active (e.g., a processor
repeatedly failing onSchedule with administrative yield enabled, then calling
applyConfiguration). Adding a test that would previously hang and now completes
would help prevent future reintroductions.
##########
libminifi/src/FlowController.cpp:
##########
@@ -110,7 +110,8 @@ FlowController::~FlowController() {
nonstd::expected<void, std::string> FlowController::applyConfiguration(const
std::string &source, const std::string &configurePayload, const
std::optional<std::string>& flow_id) {
std::unique_ptr<core::ProcessGroup> newRoot;
try {
- newRoot = updateFromPayload(source, configurePayload, flow_id);
+ // Parse the new flow here and and clear controller services later to
avoid race condition with the onScheduleTimer_ callback thread
Review Comment:
Typo/grammar in the new comment: "here and and" has a duplicated word, and
it reads awkwardly (also missing article before "race condition"). Please fix
the comment text so it’s clear and correct.
--
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]
