bobpaulin commented on code in PR #11543:
URL: https://github.com/apache/nifi/pull/11543#discussion_r3792261509
##########
nifi-framework-bundle/nifi-framework/nifi-framework-components/src/main/java/org/apache/nifi/controller/StandardProcessorNode.java:
##########
@@ -2029,59 +2030,97 @@ public List<ConnectorMethod> getConnectorMethods() {
@Override
public String invokeConnectorMethod(final String methodName, final
Map<String, String> jsonArguments, final ProcessContext processContext) throws
InvocationFailedException {
- final ConfigurableComponent component = getComponent();
-
- try (final NarCloseable ignored =
NarCloseable.withComponentNarLoader(getExtensionManager(),
component.getClass(), getIdentifier())) {
- final Method implementationMethod =
discoverConnectorMethod(component.getClass(), methodName);
- final MethodArgument[] methodArguments =
getConnectorMethodArguments(methodName, implementationMethod, component);
- final List<Object> argumentValues = new ArrayList<>();
-
- for (final MethodArgument methodArgument : methodArguments) {
- if (ProcessContext.class.equals(methodArgument.type())) {
- continue;
- }
-
- final String jsonValue =
jsonArguments.get(methodArgument.name());
- if (jsonValue == null && methodArgument.required()) {
- throw new IllegalArgumentException("Cannot invoke
Connector Method '" + methodName + "' on " + this + " because the required
argument '"
- + methodArgument.name() + "' was not provided");
+ final boolean classpathDifferent =
isClasspathDifferent(processContext.getProperties());
+
+ if (classpathDifferent || isReloadAdditionalResourcesNecessary()) {
+ LOG.debug("Classpath reload required for Connector Method
invocation. Create temporary InstanceClassLoader for {}", this);
+ final ExtensionManager extensionManager = getExtensionManager();
+ final Bundle bundle =
extensionManager.getBundle(getBundleCoordinate());
+ final Set<URL> classpathUrls =
getAdditionalClasspathResources(processContext.getProperties().keySet(),
+ descriptor ->
processContext.getProperty(descriptor).getValue());
+ final String classloaderIsolationKey =
getClassLoaderIsolationKey(processContext);
+
+ final ClassLoader currentClassLoader =
Thread.currentThread().getContextClassLoader();
+ final InstanceClassLoader detectedClassLoader =
extensionManager.createInstanceClassLoader(getCanonicalClassName(),
getIdentifier(), bundle, classpathUrls, false,
+ classloaderIsolationKey);
+ try {
+
Thread.currentThread().setContextClassLoader(detectedClassLoader);
+ final Processor tempProcessor =
componentInstanceFactory.createProcessorInstance(this, detectedClassLoader);
+ try {
+ return invokeConnectorMethodOnComponent(tempProcessor,
methodName, jsonArguments, processContext);
+ } finally {
+
ReflectionUtils.quietlyInvokeMethodsWithAnnotation(OnRemoved.class,
tempProcessor, processContext);
Review Comment:
Thanks @markap14 for calling these out. It is clear to me now that these
could create problems with the current behavior and further create more
problems if connector methods were to be added to these components.
The intent on invoking remove was the define a place where things could be
cleaned up from loading additional classes. For example with the
DBCPConnectionPool unregisters the JDBC drivers on this place. The consequence
of not doing this is a memory leak from the DriverManager holding on to these
driver classes.
Considering these drawbacks to using OnRemove it seems like it may be
helpful to introduce an additional lifecycle annotation to handle these cases
specifically. Happy to follow up this PR with a proposal for that if it makes
sense.
For this PR can we consider leaving the OnRemove behavior in place for the
verification calls (as it already exists) and excluding the OnRemove call from
any Connector Method invocation? I think this path provides consistency in the
existing verify calls without introducing any of the additional bad behaviors
you're calling out above for connector methods.
--
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]
