Bug 1312 - GLContextShareSet memory leak when repeatedly creating and removing GLDrawables
Summary: GLContextShareSet memory leak when repeatedly creating and removing GLDrawables
Status: RESOLVED FIXED
Alias: None
Product: Jogl
Classification: JogAmp
Component: opengl (show other bugs)
Version: 2.4.0
Hardware: All all
: P4 normal
Assignee: Sven Gothel
URL:
Depends on:
Blocks:
 
Reported: 2016-06-09 09:13 CEST by Tom Nuydens
Modified: 2019-12-31 05:50 CET (History)
2 users (show)

See Also:
Type: DEFECT
SCM Refs:
gluegen 5979d66fb0c619be32ebcf9fbe644cf6d757345f gluegen c6fabb0ac94000afe29156f170c63080a37c034b gluegen 178c7b9d40e06a04790542241912ca21d2c7b92f gluegen add34060fde735ea932ca49fbe79e473c5a4f40b jogl 658e25429aa150fad45a7c81a5a08f9ca35c4479 jogl 79833c9e4741bec9d1f56ea8b322679756b16f70
Workaround: ---

Attachments
Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Tom Nuydens 2016-06-09 09:13:35 CEST 
My use case is an application that has one "main" GLDrawable which is always visible, and one or more "secondary" GLDrawables that can be opened and closed any number of times (e.g. in a separate window). I want all these drawables to do GL context sharing.

As long as the main drawable exists, however, GLContextShareSet seems to keep strong references to all the secondary ones that I have ever created. Hence, even though they have been destroyed, the secondary ones cannot be GC'ed. I couldn't see a good reason for this, and was able to work around it by replacing the maps in GLContextShareSet with "weak key" maps.
Comment 1 Tom Nuydens 2016-06-13 11:36:40 CEST 
I submitted my workaround in a pull request:
https://github.com/sgothel/jogl/pull/101
Comment 2 Julien Gouesse 2016-07-06 09:01:46 CEST 
Hi

I agree with your main statement, GLContextShareSet shouldn't keep strong references on objects that could become useless, it shouldn't prevent their garbage collection. I just wonder if there is a more simple solution to fix this bug. I have looked at your pull request.
Comment 3 Sven Gothel 2018-01-16 07:14:25 CET 
Hi Tom, 

1st of all I would like to see a specific unit test for this,
we should have some already available - maybe we can fine tune them.

The actual bug you are hinting would be (using our terminology):

"GLContextShareSet seems to keep strong references to all the shared ones 
that I have ever created. 
Hence, even though they have been destroyed, the shared ones cannot be GC'ed"

'GLContextShareSet.unregisterSharing(this)' never gets called?

Which points us to the logic in GLContextImpl.destroy() #498:

+++

if( GLContextShareSet.contextDestroyed(this) &&
    !GLContextShareSet.hasCreatedSharedLeft(this) ) {
  GLContextShareSet.unregisterSharing(this);
}

+++

I will investigate this one now.

The 'hard lifecycle' with strong references is intended to enforce
a proper user lifecycle call hierarchy, e.g. calling 'destroy()'.

Let's see what we can do here ..
Comment 4 Sven Gothel 2019-12-31 05:50:44 CET 
Introducing WeakIdentityHashMap:
gluegen 5979d66fb0c619be32ebcf9fbe644cf6d757345f
gluegen c6fabb0ac94000afe29156f170c63080a37c034b
gluegen 178c7b9d40e06a04790542241912ca21d2c7b92f
gluegen add34060fde735ea932ca49fbe79e473c5a4f40b

+++

jogl 658e25429aa150fad45a7c81a5a08f9ca35c4479
jogl 79833c9e4741bec9d1f56ea8b322679756b16f70

Bug 1312: GLContextShareSet: Utilize WeakIdentityHashMap for shareMap and its destroyedShares
    
Picking up Tom Nuydens suggestion to utilize a WeakIdentityHashMap instead of a IdentityHashMap,
allowing destroyed GLContext to be removed from the GLContextShareSet through the GC.
    
TestSharedContextVBOES2NEWT5 demonstrates the use-case, having one master context
and several slaves being spawn off, killed and new sets to be spawn off.
Here the GLContextShareSet shall not hard-reference the destroyed and user-unreferenced context,
but allowing the system to GC 'em.

+++

Thank you Tom.

I kept the createdShares as hard-references on purpose.
TestSharedContextVBOES2NEWT5 passes the test.