szaszm commented on a change in pull request #777:
URL: https://github.com/apache/nifi-minifi-cpp/pull/777#discussion_r422195976
##########
File path: libminifi/src/FlowController.cpp
##########
@@ -248,9 +248,9 @@ int16_t FlowController::stop(bool force, uint64_t
timeToWait) {
this->timer_scheduler_->stop();
this->event_scheduler_->stop();
this->cron_scheduler_->stop();
+ thread_pool_.shutdown();
Review comment:
Given the above discussion, the changes look reasonable to me. However,
the discussion revealed that this code has numerous hidden interconnections:
processors may run after initiating stop => both repositories and controller
services need to be stopped after the thread pool, controller services need to
be stopped because of db locking, affected C2 which I don't understand in
detail.
Could you add a code comment mentioning the existence of these to the future
reader/developer? While not all of it is strictly in the scope of this fix, I
think the value/effort ratio would be outstanding here, making it worth doing
IMO.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
[email protected]