[GitHub] [flink] pnowojski commented on a change in pull request #9271: [FLINK-13384][1.9][runtime] Fix back pressure sampling for SourceStreamTask

[GitBox <gi...@apache.org>]

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.

----------------------------------------------------------------
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:
users@infra.apache.org


With regards,
Apache Git Services