[GitHub] [flink] StephanEwen edited a comment on issue #8280: [FLINK-12297]Harden ClosureCleaner to handle the wrapped function

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

StephanEwen edited a comment on issue #8280: [FLINK-12297]Harden ClosureCleaner to handle the wrapped function
URL: https://github.com/apache/flink/pull/8280#issuecomment-489105632
 
 
   This PR does a few things at the same time:
     1. It recurses into fields, cleaning them.
     2. It skips closure cleaning of functions that have a `writeObject` method or are `Externalizable`.
     3. It adds checks to skip classes that are not inner classes.
   
   Recursion into non-transient and non-static fields probably solves the "wrapping function" problem, but it does much more than that. While this can be a good addition, we definitely need a way to turn this off, so we need to extend the closure cleaner configuration to support "don't clean", "clean", and "clean recursive".
   
   I think that (2) and (3) are good additions, but the check should probably be at the top level, not only before checking fields. Especially (3) would help make this faster, because many classes can be skipped right away.
   
   This check should be simpler, though. We don't need to distinguish member/local/anonymous/etc classes.
   ```java
   public static void clean(Object function) {
       if (function == null) {
           return
       }
   
       final Class<?> clazz = function.getClass();
       if (clazz.getEnclosingClass() == null || !Modifiers.isStatic(clazz.getModifiers())) {
           return;
       }
   
       // do the cleaning
   }
   ```

----------------------------------------------------------------
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