Latest Posts »
Latest Comments »
Popular Posts »

ClassLoaderLocal: How to avoid ClassLoader leaks on application redeploy

Written by Jevgeni Kabanov on June 15, 2009 – 10:51 am

“OutOfMemoryError: PermGen” is a very common message to see after a few redeploys. The reason why it’s so common is that it’s amazingly easy to leak a class loader. It’s enough to hold a single outside reference to an object instantiated from a class loaded by the said class loader to prevent that class loader from being GC-d.

In this post I’ll review how we solved this problem in JavaRebel, and share the solution with you. It’s not a magical solution, but it will help alleviate some of the problems introduced in both libraries and applications in Java EE.

The most common way to leak is to register some kind of a callback object and never deregister it. E.g. look at the following code:

PLAIN TEXT
JAVA:
  1. Core.addListener(new MyListener());

If Core is a part of the framework/platform/container then it will hold to MyListener long after the application was redeployed and the class loader left hanging.

Let’s see if we can do anything to solve this. The Core implementation looks something like this:

PLAIN TEXT
JAVA:
  1. public class Core {
  2.   List listeners = new ArrayList();
  3.  
  4.   void addListener(Listener l) {
  5.     listeners.add(l);
  6.   }
  7.  
  8.   void fireListeners() {
  9.     // Exercise for the reader!
  10.   }
  11. }

The problem is that listeners provides a strong reference to the Listener object. What if we replace it by a weak one?

PLAIN TEXT
JAVA:
  1. public class Core {
  2.   List listeners = new ArrayList();
  3.  
  4.   void addListener(Listener l) {
  5.     listeners.add(new WeakReference(l));
  6.   }
  7.  
  8.   void fireListeners() {
  9.     // Exercise for the reader!
  10.   }
  11. }

Unfortunately although this does solve the problem of GC-ing the class loader, it doesn’t really work. The Listener behind the weak reference will be GC-d at first opportunity and after that it’ll no longer receive any callbacks. To illustrate why it’s a problem the code above is basically equivalent to throwing the reference away altogether:

PLAIN TEXT
JAVA:
  1. public class Core {
  2.   List listeners = new ArrayList();
  3.  
  4.   void addListener(Listener l) {
  5.     // Listener is ignored and GC-d
  6.   }
  7. }

Replacing weak reference with a soft one doesn’t improve the situation, just delays the inevitable a bit further. Both are useful for caches, where objects can be recreated at will, but not in this case where we have an externally created object.

So what do we do? What we’d like to do is have the Listener reference to depend on the class loader somehow. Unfortunately, to the best of my knowledge, there isn’t a ready-made solution for that, and there’s no way to achieve it with any combinations of weak references without causing problems.

What we’d like to have is an ability to add a strong reference to the class loader: in other words have it carry a custom property:

PLAIN TEXT
JAVA:
  1. void addListener(Listener l) {
  2.   ClassLoader cl = l.getClass().getClassLoader();
  3.   List lls = (List) cl.getProperty(“CoreListeners”);
  4.   if (lls == null) {
  5.     lls = new ArrayList();
  6.     cl.putProperty(“CoreListeners”, lls);
  7.   }
  8.   lls.add(l);
  9. }

That would work, wouldn’t it? Well, not quite. We also need to save a reference to the class loaders, so that we could later go over all of them. Here the WeakHashMap is useful:

PLAIN TEXT
JAVA:
  1. Map classLoaders = new WeakHashMap();
  2.  
  3. void addListener(Listener l) {
  4.    //…
  5.   classLoaders.put(cl, Boolean.TRUE);
  6. }

There’s not WeakHashSet in Java, so we’re just using a boolean flag as the value.

So this would probably work, but unfortunately class loaders don’t have a getProperty()/putProperty() API. However, it turns out that with a bit of a hack we can simulate it, by generating a unique class per class loader to hold the properties for us. Let’s see how it’s done!

We start with a little boilerplate:

PLAIN TEXT
JAVA:
  1. class ClassLoaderLocalMap {
  2.   private static Method defineMethod;
  3.   private static Method findLoadedClass;
  4.  
  5.   static {
  6.     try {
  7.       defineMethod = ClassLoader.class.getDeclaredMethod(
  8.          “defineClass”,
  9.           new Class[] {
  10.             String.class,
  11.             byte[].class,
  12.             int.class,
  13.             int.class });
  14.       defineMethod.setAccessible(true);
  15.  
  16.       findLoadedClass =
  17.         ClassLoader.class.getDeclaredMethod(
  18.           “findLoadedClass”,
  19.           new Class[] { String.class});
  20.       findLoadedClass.setAccessible(true);   
  21.     }
  22.     catch (NoSuchMethodException e) {
  23.       throw new RuntimeException(e);
  24.     }
  25.   } 
  26. }

This will give us access to ClassLoader protected methods defineClass() and findLoadedClass() later on. Now let’s setup the basic API:

PLAIN TEXT
JAVA:
  1. public static void put(
  2.   ClassLoader cl,
  3.   Object  key,
  4.   Object value) {
  5.   // Synchronizing over ClassLoader is safest
  6.   synchronized (cl) {
  7.     getLocalMap(cl).put(key, value);
  8.   }
  9. }
  10.  
  11. public static Object get(
  13.   Object key) {
  14.   // Synchronizing over ClassLoader is safest
  15.   synchronized (cl) {
  16.     return getLocalMap(cl).get(key);
  17.   }
  18. }

getLocalMap() method should return a map of entries associated with the class loader. How should that work?

Next we introduce a map from class loaders to unique holder class names. We also introduce a nextHolderName() method that generates unique names:

PLAIN TEXT
JAVA:
  1. private static final Map classLoaderToHolderClassName =
  2.   Collections.synchronizedMap(new WeakHashMap())
  3. private static volatile int counter = 1;
  4.  
  5. private static String nextHolderName() {
  6.   return “ClassLoaderLocalMapHolder$$GEN$$” + counter++;
  7. }

Finally we can implement the getLocalMap() method (to save space I removed all exception handling):

PLAIN TEXT
JAVA:
  1. private static Map getLocalMap(ClassLoader cl) {
  2.   String holderClassName =
  3.     (String) classLoaderToHolderClassName.get(cl);
  4.   if (holderClassName == null) {
  5.     holderClassName= nextHolderName();
  6.     classLoaderToHolderClassName.put(
  7.       cl, holderClassName);
  8.   }
  9.  
  10.   Class holderClass =
  11.     (Class) findLoadedClass.invoke(
  12.       cl,
  13.       new Object[] {propertiesClassName});
  14.  
  15.   if (holderClass  == null) {
  16.     byte[] classBytes =
  17.       buildHolderByteCode(holderClassName);
  18.  
  19.     holderClass = (Class) defineMethod.invoke(cl,
  20.         new Object[] {
  21.           holderClassName,
  22.           classBytes,
  23.           new Integer(0),
  24.           new Integer(classBytes.length)});
  25.   }
  26.  
  27.   return (Map) holderClass
  28.     .getDeclaredField(“localMap”).get(null);
  29. }

The last method to implement is buildHolderByteCode. It’s quite trivial and builds the following class renamed to the unique name:

PLAIN TEXT
JAVA:
  1. public class ClassLoaderLocalMapHolder$$GEN$$X {
  2.   public static final Map localMap = new HashMap();
  3. }

The code can be derived using ASMifier with just a little customization, you can look it up in the full source code.

Although we could now easily implement the original example it makes sense to do just a little bit extra effort and introduce a ClassLoaderLocal, with behavior similar to the ThreadLocal:

PLAIN TEXT
JAVA:
  1. public class ClassLoaderLocal {
  2.   private Object key = new Object();
  3.  
  4.   public Object get(ClassLoader cl) {
  5.     if (!ClassLoaderProperties.containsKey(cl, key))
  6.       return null;
  7.     return ClassLoaderProperties.get(cl, key);
  8.   }
  9.  
  10.   public void set(ClassLoader cl, Object value) {
  11.     ClassLoaderProperties.put(cl, key, value);
  12.   }
  13. }

So the original example now becomes:

PLAIN TEXT
JAVA:
  1. Map classLoaders = new WeakHashMap();
  2. ClassLoaderLocal cll = new ClassLoaderLocal();
  3.  
  4. void addListener(Listener l) {
  5.   ClassLoader cl = l.getClass().getClassLoader();
  6.   List lls = (List) cll.get(cl);
  7.   if (lls == null) {
  8.     lls = new ArrayList();
  9.     cll.set(cl, lls);
  10.   }
  11.   lls.add(l);
  12.  
  13.   classLoaders.put(cl, Boolean.TRUE);
  14. }

In this code if any listener comes from a freed class loader, then it will be GC-d from both Core.classLoaders and ClassLoaderProperties.classLoaderToHolderClassName, as both are WeakHashMaps and there are no strong references to the class loaders. The generated ClassLoaderLocalMapHolder$$GEN$$X class will also be GC-d along with the class loader, so we have effectively eliminated a class loader leak without explicit cleanup calls from the user.

I hope this code will be useful for someone. I cannot give any guarantees whether it will work or not and it’s clearly a hack (though a solid hack). Please use it if you actually understand what is happening.

If you see a bug in the code or have a good suggestion, please be sure to comment. There could be a free JavaRebel license in it for you :)

Full source code: ClassLoaderLocalMap.java, ClassLoaderLocal.java.


Posted in Featured, creative | 31 Comments »

5 Comments 24 Other Comments

31 Comments to “ClassLoaderLocal: How to avoid ClassLoader leaks on application redeploy”

  1. Erkki Lindpere Says:
    June 15th, 2009 at 2:24 pm

    I have a good suggestion: implement a generic WeakListenerList on top of this :)

    Something like:

    public class WeakListenerList implements Iterable {
    Map classLoaders = new WeakHashMap();
    ClassLoaderLocal cll = new ClassLoaderLocal();

    public Iterator iterator() {
    // create iterator
    }

    public void add(Object l) {
    ClassLoader cl = l.getClass().getClassLoader();
    List lls = (List) cll.get(cl);
    if (lls == null) {
    lls = new ArrayList();
    cll.set(cl, lls);
    }
    lls.add(l);
    classLoaders.put(cl, Boolean.TRUE);
    }

    }

  2. Jevgeni Kabanov Says:
    June 15th, 2009 at 2:28 pm

    @Erkki

    Yeah, that’s the obvious one, though :) Naming is a problem here, it’s not really weak, it’s only weak as far as class loaders are concerned. Something like WeakLoadingList and WeakLoadingMap could be useful.

  3. Jevgeni Kabanov Says:
    June 15th, 2009 at 2:29 pm

    @Erkki

    Oh, and implementing a List (that has order) is very hard. WeakLoadingSet is possible, though.

  4. Onne Says:
    June 15th, 2009 at 2:53 pm

    Assuming Listener is a class loaded from the otherwise leaking classloader, wouldn’t it be simpler to do:

    static final ArrayList ls = …;
    public Listener() {
    ls.add(this);
    }

    or better, have a ‘useful’ (e.g. a class that will be garbage only after you move on) class be the listener, like such:

    class XXX implements Listener
    …
    Core.addListener(this);
    …

    Since the whole problem is that a `new Listener()` is not strongly held by anything belonging to the otherwise leaking classloader; and may not be strongly held by the Core, or we have a leak…

    The only reason I see for such an elaborate setup is if the target audience cannot be explained that listeners must have a strong reference from inside the container. But since the audience are Java developers, I don’t know if it would be worth the trouble.

    And even if; would it be easier to setup a small class to be loaded together with the container classes, and use that? Through like `Class.forName(“small class”, listener.getClass().getClassLoader());`?

  5. h2o2 Says:
    June 15th, 2009 at 3:08 pm

    Baloney. The fix is to remove the listener on undeploy (or not to have strongly referenced synchronous listeners in the first place), not to jerk around with classloaders in order to "fix" unrelated problems. Your code is broken, fix it. A great example for the culture of workarounds and blame-shifting.

    This comment was originally posted on Reddit

  6. Jevgeni Kabanov Says:
    June 15th, 2009 at 3:57 pm

    @Onne

    All the solutions you describe rely on programmers writing correctly behaving code. If that would work we wouldn’t have class loader leaks in the first place. The ClassLoaderLocal works defensively, assuming the worst and making the library writer do more work to eliminate this kind of errors. It isn’t actually anyhow specially elaborate, just accesses protected APIs, which is completely legal.

    I don’t understand how the “small class” solution would help. It would always be loaded in the same root container class loader, no matter what you pass to Class.forName().

  7. ekabanov Says:
    June 15th, 2009 at 4:01 pm

    That’s a great philosophy. Should garbage collection also be forbidden as a "fix" to manual memory management?

    This comment was originally posted on Reddit

  8. jkilgrow Says:
    June 15th, 2009 at 4:46 pm

    Really? Really?!?
    If you have to write this kind of code, IMO, either the framework is broken or the JVM is broken. Maybe Java is broken. This should NOT happen.

    Where is the problem? Is it in the code that we write? The users of the JVM should not have to do this type of thing. Why isn’t it straightforward? Do we really want to start messing with the classloader? Something is very wrong and I DON’T think it is with the code that runs in the JVM.

  9. Jevgeni Kabanov Says:
    June 15th, 2009 at 5:21 pm

    @jkilgrow

    At the moment when undeploying an application, Tomcat will go through all classes loaded by the web class loader and set their fields to null. This doesn’t solve the problem, but does help sometimes.

    This does happen, and this solution is the best I know of. Nothing is broken, it’s just life.

  10. Onne Says:
    June 15th, 2009 at 5:35 pm

    @Jevgeni Kabanov

    Unless the server writer would mess up in Core.addListener …

    About the helper class: Obviously you have to ensure it is only on the container class path, not any parents.

    Actually, even if you wish to pursuit your direction, you don’t have to generate unique classes etc, the following will do, using a more ‘efficient’ interface is left as an exercise :)

    —-ClassLoaderTest.java:
    import java.io.InputStream;
    import java.lang.reflect.Method;
    import java.util.ArrayList;
    import java.net.*;

    public class ClassLoaderTest {
    public static ArrayList strongrefs = new ArrayList();
    public static void addReference(Object o) {
    System.out.println(“my classloader: “+ ClassLoaderTest.class.getClassLoader());
    strongrefs.add(o);
    }

    public static void main(String[] args) {
    ClassLoader cl = ClassLoaderTest.class.getClassLoader();
    System.out.println(cl);

    try {
    ClassLoader ncl = new URLClassLoader(new URL[] {});
    Method defineClass = ClassLoader.class.getDeclaredMethod(“defineClass”, String.class, byte[].class, int.class, int.class);
    defineClass.setAccessible(true);

    InputStream in = cl.getResource(“ClassLoaderTest.class”).openStream();
    byte[] bytes = new byte[in.available()];
    in.read(bytes);
    Class loaded = (Class) defineClass.invoke(ncl, “ClassLoaderTest”, bytes, 0, bytes.length);

    loaded.getDeclaredMethod(“addReference”, Object.class).invoke(null, new Object[]{“listener”});
    } catch (Exception e) {
    e.printStackTrace();
    }
    }
    }
    —-

  11. h2o2 Says:
    June 15th, 2009 at 5:49 pm

    No. My point was that this solution is yet another *partial* fix which doesn’t work anywhere else. Fixing the orignal application-level bug (or the JVM) does, without embedding hidden assumptions in an architecture or increasing the fragility of the underlying platform.

    This comment was originally posted on Reddit

  12. Jevgeni Kabanov Says:
    June 15th, 2009 at 5:54 pm

    @Onne

    That won’t work, you’ll get LinkageError’s because multiple class definitions in the class loader hierarchy are not allowed.

  13. Jevgeni Kabanov Says:
    June 15th, 2009 at 5:55 pm

    @Onne

    If helper class is on the container classpath it will only be loaded once, there’s no “per class loader” behavior.

  14. ekabanov Says:
    June 15th, 2009 at 6:04 pm

    Where doesn’t it work? AFAIK it doesn’t use any hidden assumptions and should work on any JVM in any container. It’s very straightforward.

    This comment was originally posted on Reddit

  15. Onne Says:
    June 15th, 2009 at 6:36 pm

    don’t think so, the example works fine, and the same class is loaded twice by two different classloaders, which form a hierarchy, try it …

    With ‘container’ classloader, I ment whatever loads the contents, it is WebAppX in http://tomcat.apache.org/tomcat-5.5-doc/class-loader-howto.html

    And you control that particular classloader. Likely it is a URLClassLoader, simply add a small jar with the helper class. (And ensure that no parent of this classloader has that jar on its class.)

    If it is a custom classloader, simply always make it load that class privately, by overriding loadClass.

  16. Jevgeni Kabanov Says:
    June 15th, 2009 at 6:55 pm

    @Onne

    I’m pretty sure of the LinkageError’s. Besides everything else loadClass() will return the top class in the hierarchy no matter what was defined locally.

    Injecting a same named class per web class loader might work, though most application containers will have over 100 instances of class loaders per application, which makes it hard to keep them from interfering. I don’t see any problems with the proposed solution, it’s way less invasive than a lot of stuff that containers do.

  17. Yousuf Says:
    June 15th, 2009 at 10:03 pm

    I think a much simpler solution would be to have something like a unregisterListener method. Most platfomrs/frameworks I see have something like that which is called when the application is undeployed.

    This way you don’t get Classloader leaks

  18. mazin Says:
    June 15th, 2009 at 10:56 pm

    The hidden assumption is that you can get java.util.logging to use _ClassLoaderLocal_s. Good luck with that :)

    This comment was originally posted on Reddit

  19. slaurent Says:
    June 16th, 2009 at 12:12 am

    Excellent hack ! It’s probably very useful in container code to make it more robust to this type of leak.

    BTW, speaking of hacks, in order to work around another type of classloader leak (through JRE or container threads created while the context classloader is set to the webapp’s) I had proposed this : http://www.nabble.com/Proposition-for-a-WebAppClassLoader-enhancement-:-ExpendableClassLoader-td22917197.html

    Unfortunately nobody replied to my post :-(
    I’d be glad to know your opinion on this hack.

  20. Tor Says:
    June 16th, 2009 at 4:26 am

    So could someone with more intimate knowledge about tomcat provide some code to enhance its webclassloader, to use this technique?

  21. Onne Says:
    June 16th, 2009 at 2:31 pm

    @Jevgeni Kabanov

    > I’m pretty sure of the LinkageError’s. Besides
    > everything else loadClass() will return the top class …

    obviously the answer is no twice: my sample demonstrates it, and classes are loaded as (loader,class) pairs in the jvm, demonstrated by:

    900 unsigned int d_hash = dictionary()->compute_hash(class_name, class_loader);

    in file: http://hg.openjdk.java.net/jdk6/jdk6/hotspot/file/fc30e742f2fc/src/share/vm/classfile/systemDictionary.cpp

    loadClass() does a findLoadedClass() first which will check the current classloader if it loaded the class. Only then will it descent down to parent …

    a fully working solution:

    —- Holder.java:
    import java.util.ArrayList;
    // has to be in own file, since it must be a *public* class
    public class Holder {
    public static ArrayList strongrefs = new ArrayList();
    }

    —- ClassLoaderTest.java:
    import java.io.InputStream;
    import java.lang.ref.WeakReference;
    import java.lang.reflect.Method;
    import java.util.*;

    public class ClassLoaderTest {
    static WeakHashMap<ClassLoader, ArrayList> holders = new WeakHashMap<ClassLoader, ArrayList>();
    static ArrayList<WeakReference> listeners = new ArrayList<WeakReference>();

    public static Method defineClass;
    public static byte[] holderBytes;
    static {
    try {
    defineClass = ClassLoader.class.getDeclaredMethod(“defineClass”, String.class, byte[].class, int.class, int.class);
    defineClass.setAccessible(true);

    InputStream in = ClassLoaderTest.class.getClassLoader().getResource(“Holder.class”).openStream();
    holderBytes = new byte[in.available()];
    in.read(holderBytes);
    } catch (Exception e) { e.printStackTrace(); }
    }

    public static ArrayList addHolder(ClassLoader cls) {
    try {
    Class loaded = (Class) defineClass.invoke(cls, “Holder”, holderBytes, 0, holderBytes.length);
    ArrayList ls = (ArrayList) loaded.getDeclaredField(“strongrefs”).get(null);
    // TODO write-lock required here
    synchronized(holders) { holders.put(cls, ls); }
    } catch (Exception e) { e.printStackTrace(); }

    // if we were racing to do addHolder, we got a LinkageError: attempted duplicate class definition…
    // and class loading/resolving/initializing is atomic, thanks java, so this is enough
    // TODO read-lock is enough
    synchronized(holders) { return holders.get(cls); }
    }

    public static void addListener(Observer listener) {
    ClassLoader cls = listener.getClass().getClassLoader();
    ArrayList ls;
    // TODO read-lock is sufficient
    synchronized(holders) { ls = holders.get(cls); }
    if (ls == null) ls = addHolder(cls);

    // create a strong reference from its classloader to listener
    ls.add(listener);

    // create a weak reference for signaling
    listeners.add(new WeakReference(listener));
    }

    public static void main(String[] args) {
    addListener(new Observer() {
    public void update(Observable o, Object arg) {
    System.out.println(“ha”);
    }
    });
    }
    }

  22. Jevgeni Kabanov Says:
    June 16th, 2009 at 2:51 pm

    @Onnne

    Spec says it’s not allowed http://java.sun.com/docs/books/jvms/second_edition/html/ConstantPool.doc.html#78621 and loadClass() is overridden by almost any custom class loader in a variety of ways. Good luck with that, I had enough fun debugging such errors for a lifetime…

    Also the “working” code you proposed leaks class loaders, since holders has strong references to listeners that hold the classes that hold the class loaders.

    Read/Write lock might be a good idea, though. Thanks for the discussion!

  23. Onne Says:
    June 16th, 2009 at 3:58 pm

    @Jevgeni Kabanov

    1. It does not violate the spec, since the last two of the four condition for link failure are not met.

    2. My solution does not depend on loadClass at all, only defineClass, which is a final method and cannot be overridden (and after a check is a native method). So it absolutely does not matter what any classloader subclass does.

    You are right about the leak though, sorry. We should weak reference the ArrayLists as well in our holders map, or we still hold strong refs to the classloader.

    Honestly, both solutions are quite similar, just mine is half the lines of code, because it does not customize the class, and does not implement some kind of classloaderlocals map. But maybe that last one is useful?

    —

    But I must restate that this is not a good solution at all. Because this problem is not related to classloading, just classloading makes it worse since it fills up permgen.

    Imagine a heavyweight view in a GUI app that registers observers, but fails deregister them; after opening that view lots of time, the vm has a lot of objects that are still reachable through observable->listener->target, worse, they still get signals and process them. But these objects will never do anything visible, they belong to long ago removed views.

    Only a WeakObservable is a real solution. Slightly more difficult as the regular Observable, since you must ensure to strongly hold on to the listeners you give it.

    However, you will quickly see a bug appear around views not receiving signals even after you have register them … and bugs programmers can fix. Memory leaks are much harder to find and fix.

    good discussion indeed :)

    -Onne

  24. Jevgeni Kabanov Says:
    June 16th, 2009 at 4:12 pm

    1) I disagree :) Unfortunately the spec doesn’t state it clear enough, but AFAIK defining same named class in non-sibling class loaders is not allowed. Checking section 4 of the original paper referred in the spec (http://citeseerx.ist.psu.edu/viewdoc/download?doi=10.1.1.77.6898&rep=rep1&type=pdf), it’s clear that you don’t get an error only because you don’t actually do anything useful with the class that would generate reasonable constraints. I think I’ll leave it at that.
    2) True.

    I was trying to solve a particular problem of class loader leaking in Java EE, not every memory leak in the Java language. The example you bring doesn’t involve any class loaders and isn’t relevant :)

  25. Onne Says:
    June 16th, 2009 at 6:00 pm

    The spec is quite clear, and the Holder class is not violating the rules *exactly* because it does not introduce unresolvable constrains.

    The only constraint it introduces is: Holder.class.getClass().classLoader().loadClass(“java.util.ArrayList”) == this.getClass().classLoader().loadClass(“java.util.ArrayList”);

    which it is … you cannot disagree with it since it is not a beauty thing, you can disagree only if you find a counter example, which I believe is you cannot do (but would love to see one).

    If “defining same named class in non-sibling class loaders is not allowed”, I guess it would say so in the spec, wouldn’t it?

    (It actually does introduce more constrains, namely on Class and String and likely some more … since Object introduces those)

    As a matter of fact, you can easily create a fun ClassCastException:

    Holder x = (Holder) loadedHolder.newInstance();

    It says: cannot cast Holder to Holder … yay!

    —

    I understand you were trying to fix a classloader leak, but the underlying cause for the leak is more general, and cannot be fixed in the Observable implementation, since you cannot generally know the target of a Listener and ‘fix a strong reference’ (If you could, java would have a fourth Reference type: the BoundReference; as long as that thing is alive, keep the target of the BoundReference alive.)

    Only in this specific case we can, since we don’t fix it to the target, but to the classloader of both Listener and target. And in this case, that is good enough.

    It can be fixed in general, however, if we modify the expectations of Observable slightly: namely registering something to Observable does not introduce strong a reference to that something.

    (But again, I do understand that likely this is useless to you, since in the real world, you have real Observable’s and their semantics cannot be changed anymore… and because of that, your proposed solution is actually quite nice.)

    Well, love to see a counter example, but otherwise I’ll stop harassing you :) hope you didn’t mind …

  26. Jevgeni Kabanov Says:
    June 16th, 2009 at 6:21 pm

    @Onne

    I don’t mind at all. But let’s discuss this or other stuff further with some kind of beverage at hand :)

  27. Matt Drees Says:
    June 17th, 2009 at 5:44 am

    FWIW, I think you need some form of synchronization in ClassLoaderLocalMap.nextHolderName(). I’m pretty sure an increment operation isn’t atomic, even on volatile fields. ( See http://tech.puredanger.com/2009/01/30/java-concurrency-bugs-atomic/ )

    Thanks for the post!

  28. Jevgeni Kabanov Says:
    June 17th, 2009 at 10:41 am

    @Onne

    Yeah and I guess I am too practical. Truth is even if the spec is OK for that particular class JVMs implement the spec differently and I just wouldn’t risk with fine lines like that when it can lead to an error on some Sun JVM 1.4_07.

  29. Jevgeni Kabanov Says:
    June 17th, 2009 at 11:04 am

    @Matt

    Thanks for the catch!

  30. Dmitri Says:
    June 22nd, 2009 at 1:12 pm

    Any good ideas on how to find the offenders?

    I have a 100M+ EAR application that apparently leaks classloaders. So far I’ve been able to trace some of the live objects up to the static fields in one app class, but I can’t find out who leaks the classloader.

  31. Jevgeni Kabanov Says:
    June 23rd, 2009 at 10:29 am

    @Dmitri

    I use Eclipse Memory Analyzer, but tools only get you so far. You just have to understand very well how the machinery works :( That said in many cases EMA leak suspects will reveal the leaks right away.

Leave a Comment

Additional comments powered by BackType