views:

188

answers:

3

I have a JFrame that accepts top-level drops of files. However after a drop has occurred, references to the frame are held indefinitely inside some Swing internal classes. I believe that disposing of the frame should release all of its resources, so what am I doing wrong?

Example

import java.awt.datatransfer.DataFlavor;
import java.io.File;
import java.util.List;

import javax.swing.JFrame;
import javax.swing.JLabel;
import javax.swing.TransferHandler;

public class DnDLeakTester extends JFrame {
    public static void main(String[] args) {
        new DnDLeakTester();

        //Prevent main from returning or the jvm will exit
        while (true) {
            try {
                Thread.sleep(10000);
            } catch (InterruptedException e) {

            }
        }
    }
    public DnDLeakTester() {
        super("I'm leaky");

        add(new JLabel("Drop stuff here"));

        setTransferHandler(new TransferHandler() {
            @Override
            public boolean canImport(final TransferSupport support) {
                return (support.isDrop() && support
                        .isDataFlavorSupported(DataFlavor.javaFileListFlavor));
            }

            @Override
            public boolean importData(final TransferSupport support) {
                if (!canImport(support)) {
                    return false;
                }

                try {
                    final List<File> files = (List<File>) 
                            support.getTransferable().getTransferData(DataFlavor.javaFileListFlavor);

                    for (final File f : files) {
                        System.out.println(f.getName());
                    }
                } catch (Exception e) {
                    e.printStackTrace();
                }

                return true;
            }
        });

        setDefaultCloseOperation(DISPOSE_ON_CLOSE);
        pack();
        setVisible(true);
    }
}

To reproduce, run the code and drop some files on the frame. Close the frame so it's disposed of.

To verify the leak I take a heap dump using JConsole and analyse it with the Eclipse Memory Analysis tool. It shows that sun.awt.AppContext is holding a reference to the frame through its hashmap. It looks like TransferSupport is at fault.

image of path to GC root

What am I doing wrong? Should I be asking the DnD support code to clean itself up somehow?

I'm running JDK 1.6 update 19.

+1  A: 

Does it change your results if you add this to your class?

@Override
public void dispose()
{
    setTransferHandler(null);
    setDropTarget(null);    // New
    super.dispose();
}

Update: I've added another call to the dispose. I think setting the drop target to null should release some more references. I don't see anything else on the component that can be used to get the DnD code to let go of your component.

Devon_C_Miller
Thanks for your suggestion Devon. No, I'm afraid it doesn't change anything.
tom
Sorry, still no luck. I also tried getDropTarget().setComponent(null), but it doesn't work either. The problem is that there isn't any code inside TransferHandler or its inner classes to remove the DropHandler from the AppContext. There just isn't a remove() to complement the put()
tom
There does not appear to be any way to release the reference held by AppContext. Fortunately, the key used in the put is the Class of DropTarget, so there can only be 1 outstanding instance. Document it and minimize the damage by having dispose() release everything else. Like getContentPane().removeAll().
Devon_C_Miller
A: 

Having scoured the source of the relevant classes, I'm convinced this is an unavoidable leak in TransferHandler.

The object in AppContext's map, a DropHandler, is never removed. Since the key is the DropHandler.class object, and DropHandler is a private inner class, the only way it could be removed from outside TransferHandler is by emptying the whole map, or with reflection trickery.

DropHandler holds a reference to a TransferSupport object, which is never cleared. The TransferSupport object holds a reference to the component (in my case a JFrame), which isn't cleared either.

tom
+1  A: 

Although the DropHandler is not removed from the static AppContext map, that is not really the root cause, but merely one of the causes in the chain. (The drop handler is intended to be a singleton, and not cleared up until the AppContext class is unloaded, which in practice is never.) Use of a singleton DropHandler is by design.

The real cause of the leak is that the DropHandler sets up an instance of TransferSupport, that is reused for each DnD operation, and during a DnD operation, provides it with a reference to the component involved in DnD. The problem is that it does not clear the reference when DnD finishes. If the DropHandler called TransferSupport.setDNDVariables(null,null) when the DnD exited, then the problem would go away. This is also the most logical solution, since the reference to the component is only required while DnD is in progress. Other approaches, such as clearing the AppContext map are circumventing the design rather than fixing a small oversight.

But even if we fix this, the frame would still not be collected. Unfortunately, there appears to be another problem: When I commented out all the DnD related code, reducing to just a simple JFrame, this too was not being collected. The retaning reference was in javax.swing.BufferStrategyPaintManager. There is a bug report for this, as yet unfixed.

So, if we fix the DnD, we hit another retention problem with repainting. Fortunately, all of these bugs hold on to only one frame (hopefully the same one!), so it is not as bad as it could be. The frame is disposed, so native resources are released, and all content can be removed, allowing that to be freed, reducing the severity of the memory leak.

So, to finally answer your question, you are not doing anything wrong, you are just giving some of the bugs in the JDK a little air time!

UPDATE: The repaint manager bug has a quick fix - adding

-Dswing.bufferPerWindow=false

To the jvm startup options avoids the bug. With this bug quashed, it makes sense to post a fix for the DnD bug:

To fix the DnD problem, you can add a call to this method at the end of importData().

            private void cancelDnD(TransferSupport support)
            {
                /*TransferSupport.setDNDVariables(Component component, DropTargetEvent event) 
                Call setDNDVariables(null, null) to free the component.
*/
                try
                {
                    Method m = support.getClass().getDeclaredMethod("setDNDVariables", new Class[] { Component.class, DropTargetEvent.class });
                    m.setAccessible(true);
                    m.invoke(support, null, null);
                    System.out.println("cancelledDnd");
                }
                catch (Exception e)
                {
                }
            }
mdma
Thanks for the info. I had spotted BufferStrategyPaintManager retaining some classes but was distracted with the DnD stuff. I accept that the reflection trick will work - but I don't intend to use it
tom
Sure, it's your choice if you use the fix or not, but now at least you know the scope of the problem and can decide how you deal with it. Like you. I'd probably not bother adding the fix, and knowing the problem can be partially mitigated, probably do nothing or at most, remove content from frames on dispose. But that is in hindsight now the extent of the problem is known. I was very curious to know why the leak was happening and what the consequences were, and it's nice to know that if we suddenly find a fix is needed, that one is available. I can sleep easy tonight! :-)
mdma