Github user joewitt commented on a diff in the pull request:
https://github.com/apache/nifi/pull/836#discussion_r74753025
--- Diff:
nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-framework-core/src/main/java/org/apache/nifi/controller/StandardProcessorNode.java
---
@@ -1321,12 +1325,21 @@ public void run() {
this.unscheduled = true;
}
try {
- if (activeThreadMonitorCallback.call()) {
+ if (scheduleState.isScheduled()) {
+
schedulingAgent.unschedule(StandardProcessorNode.this, scheduleState);
+ }
--- End diff --
I don't read it as a leak of ScheduleState. This class and the class
invoking stop are part of a closed model. Having that here means logic for
unscheduling, waiting for completion, and invoking onStopped are all nicely
aligned in a single location.
@markap14 I am wondering why we don't unschedule the processorNode before
we invoke onUnscheduled in the processor? Technically speaking couldn't we
continue to give it threads between telling the processor it has been
unscheduled and then actually unscheduling it? That and the finding that we're
not wrapping the call to the actual processor's unscheduled method(s) in
NarCloseable are my two outstanding questions here.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at [email protected] or file a JIRA ticket
with INFRA.
---
