Awesome; this is great work, thank you.

@codeanticode Can you review the parts of the OpenGL parts? If they look good, we're good to merge.

yep, will review asap! Thanks for these fixes, @jdf

Don't thank me until it's been proven not to crash. :)

@jdf The refactoring to reduce code duplication looks good to me. However, the use of the RefList is simply following the technique described in the original article:

"Notice that reference objects are discovered by the garbage collector and added to their associated reference queues only if they are reachable themselves. Otherwise, they are simply reclaimed like any other unreachable object. This is why you add all NativeImage3 instances to the static list -- actually, any data structure will suffice -- to ensure that they remain reachable and processed when their referents become unreachable. Naturally, you also have to make sure that you remove them from the list when you dispose of them. This is done in the dispose() method."

Understood. Will ping again when I've corrected it.

OK; please take another look.

I think it's fixed. However, you also need to check type of argument in equals(), since you are mixing all the subclasses in one list, otherwise you get ClassCastException in reachableWeakReferences.remove(this);

If in doubt, run this stress test with -Xmx32m and print debug message in dispose()

  public void settings() {
    size(640, 480, P3D);
    smooth(2);
  }

  public void setup() {
  }

  public void draw() {
    PGraphicsOpenGL pg = (PGraphicsOpenGL)createGraphics(width, height, P3D);
    pg.beginDraw();
    pg.background(random(255), random(255), random(255));
    pg.endDraw();
    image(pg, 0, 0);
  }

(If your code is wrong you need to stop it after ~10 seconds, otherwise it might crash your computer as it crashed mine.)

Looks good, thanks @jdf. Let's wait what @codeanticode thinks.

However, it does the same thing as before (just with less code), so you have to check if it fixes your problem. The code worked well previously (we tested it with @codeanticode before 3.0 came out), so I'm not sure how the leak could occur. Everything should be released in next garbage collector run, unless there is some other sneaky problem. Edit: the exception thread was probably the reason.

@JakubValtar First, thanks very much for your help.

The test you suggested worked well to verify disposals taking place. (Although for Python Mode I had to give it 48M to run at all :} ).

I think the real problem was the non-exiting thread that waited forever for an unhandled exception to be thrown (while closed over its containing PGraphicsOpenGL), along with the Apple-specific one.

@jdt You're welcome :) I ran it through VisualVM (great for heap analysis) and I think you also need to clear the exception handler in PAppletJythonDriver at line 628.

@JakubValtar I don't know who @jdt is, but I believe they spend a lot of time working in Eclipse. :)

Yes, I know about the PAppletJythonDriver fix; I've got that going in my own repo. Again, thank you.

For what it's worth, I use YourKit for memory debugging (and other profiling tasks). I'll also download VisualVM and give it a try.

Oops, sorry :)

VisualVM is part of JDK, at least on Windows (jvisualvm.exe). I never used YourKit but I think it's better than whatever ships with JDK.

I have no idea what this project is, but I wholeheartedly support the elimination of memory leaks.

Yes, I think the problem was the exception-catching thread, seems to me that now is properly terminated when the animation thread stops.

@benfry I think the GL fixes are ready to be merged.

Great; merged.

Just noticed that the System.err.println() line is still present in Disposable.dispose() https://github.com/processing/processing/blob/master/core/src/processing/opengl/PGraphicsOpenGL.java#L82

@jdf Shouldn't be removed?

Yes.

@jdf I've just been looking at this pull request and testing the latest version of the Python mode, and there is definitely a memory leak still there! Praxis LIVE actually does a similar thing to the Python mode here, and I need to update my fix hence looking at this. I'm using reflection to clean out the reference lists and remove the PGL references in the resource classes at the end of each sketch run.

One problem is that the reference list is a static field - when I suggested using this method of cleanup to @codeanticode I suggested using instance fields, perhaps on PSurfaceJOGL, and explicitly disposing. The second issue is that the resource classes have a strong reference to PGL in them which creates a circular reference that stops a lot of things being cleaned up. Of course, if the second issue didn't exist, you'd end up cleaning up resources that no longer exist because the JOGL context has gone, so in some ways it's a memory leak masking something worse.

This pull request has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.