pnowojski commented on a change in pull request #9271:
[FLINK-13384][1.9][runtime] Fix back pressure sampling for SourceStreamTask
URL: https://github.com/apache/flink/pull/9271#discussion_r309646392
##########
File path:
flink-runtime/src/main/java/org/apache/flink/runtime/taskmanager/Task.java
##########
@@ -593,6 +593,10 @@ else if (current == ExecutionState.CANCELING) {
LOG.info("Loading JAR files for task {}.", this);
userCodeClassLoader = createUserCodeClassloader();
+
+ // make sure the user code classloader is accessible
thread-locally
+
executingThread.setContextClassLoader(userCodeClassLoader);
Review comment:
Maybe moving this just before
```
invokable = loadAndInstantiateInvokable(userCodeClassLoader,
nameOfInvokableClass, env);
```
would be more clean and safe? And maybe add some comment like
```
// We are setting the correct context class loader for the whole interaction
with the `invokable`
```
?
The contract that we would introduce here, is that user code should be
executed with the `userCodeClassLoader`. Before constructing the invokable, we
are not touching any user code, so there should be no point (and could
potentially do some harm) in setting context class loader so early.
Arguably this is a separate bug fix that just wasn't discovered before.
Also technically speaking it would be better to reset the context class
loader when we are done interacting with the `invokable`, however this might be
risky change, so I wouldn't do it now, but sometime in the future.
----------------------------------------------------------------
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]
With regards,
Apache Git Services