JDK-6218682 : SubjectDomainCombiner pdCache (WeakHashMap) values strongly reference keys
  • Type: Bug
  • Component: security-libs
  • Sub-Component: java.security
  • Affected Version: 1.0,1.1,1.4.2_07,5.0,6
  • Priority: P3
  • Status: Resolved
  • Resolution: Fixed
  • OS: generic,solaris_9
  • CPU: generic,sparc
  • Submitted: 2005-01-18
  • Updated: 2010-12-08
  • Resolved: 2005-02-18
The Version table provides details related to the release that this issue/RFE will be addressed.

Unresolved : Release in which this issue/RFE will be addressed.
Resolved: Release in which this issue/RFE has been resolved.
Fixed : Release in which this issue/RFE has been fixed. The release containing this fix may be available for download as an Early Access Release or a General Availability Release.

To download the current JDK release, click here.
Other JDK 6
1.4.2_09Fixed 6 betaFixed
Related Reports
Duplicate :  
JDK-6183122 - Memory leak when unregister from secured JMXMP connection
Description
SubjectDomainCombiner creates an instance of WeakHashMap.  Key objects in this map are the "current" ProtectionDomain instances received via the combine method.  Each "current" PD is mapped to a new PD instance that holds both the contents of the "current" PD, as well as the principals from the Subject associated with this combiner.

At the moment, the WeakHashMap values are regular strong references to the newly created PD "principal-based" instances.  However, this is a problem because these values contain strong references to the corresponding key object (the "current" non-principal-based PD).  Specifically, a "principal-based" PD contains strong references to the CodeSource, signer certs, PermissionCollection and ClassLoader objects in the "current PD".  This prevents the key from being garbage-collected, resulting in an ever growing Map over time (assuming the SubjectDomainCombiner instance is shared across different ClassLoader instances).

To correct this problem, the newly created "principal-based" PD values must be stored as WeakReferences.  This will allow the keys to become weakly referencable and therefore let Map entries become garbage collected as ClassLoader instances become weakly referencable.  

Note that if the SubjectDomainCombiner instance is NOT shared across different ClassLoader instances, this problem is less severe.  The code effectively behaves as it were using a regular HashMap as opposed to a WeakHashMap - entries never leave the Map via garbage collection.  The size of the Map is roughly limited by the number of ClassLoader classpath elements.

JMX does share SubjectDomainCombiner instances across ClassLoaders, so this problem eventually results in an OutOfMemoryError (since the Map grows infinitely over time).

###@###.### 2005-1-18 22:49:03 GMT
Comments
EVALUATION No regression test. Unable to rely on deterministic garbage collection behavior ###@###.### 2005-1-22 01:29:45 GMT It is not dependent to the garbage collector behavior, but the class WeakHashMap implementation, look at the Javadoc of WeakHashMap: Implementation note: The value objects in a WeakHashMap are held by ordinary strong references. Thus care should be taken to ensure that value objects do not strongly refer to their own keys, either directly or indirectly, since that will prevent the keys from being discarded. Note that a value object may refer indirectly to its key via the WeakHashMap itself; that is, a value object may strongly refer to some other key object whose associated value object, in turn, strongly refers to the key of the first value object. One way to deal with this is to wrap values themselves within WeakReferences before inserting, as in: m.put(key, new WeakReference(value)), and then unwrapping upon each get. ###@###.### 2005-1-26 10:12:42 GMT Clarified with Shanliang that we agree on the problem in the code. Shanliang also suggested the following test could be used (below). We agreed the test is not guaranteed to always work since it depends on unspecified GC behavior. However if run by hand it should work in most cases currently. It can be used to sanity check the fix. You can write a test like this: 1) create 10 different keys and values, the classloader of a value keeps a reference to the value's key; 2) call 10 times SubjectDomainCombiner.combine() to make those 10 key-value paires saved to SubjectDomainCombiner's internal map; 3) save 10 values or keys into another WeakHashMap, like myMap.put(value, ""); 4) do check as for (int i=0; i<300; i++) { Thread.sleep(100); System.gc(); if (myMap.size() < 10) { return true // successed } } return false; // failed Another way is to specify a small heap like: java -Xmx1M Test we get OutOfmemeoryException in about 5 minutes with out test, you can reduce the heap or make bigger objects in question to speed up. ###@###.### 2005-1-28 18:27:40 GMT the escalation engineer for 1.4.2 asked me to write up the above test. i believe the following approximates what is described above: import java.lang.ref.WeakReference; import java.util.WeakHashMap; import java.security.cert.Certificate; import javax.security.auth.Subject; import javax.security.auth.SubjectDomainCombiner; import javax.security.auth.x500.X500Principal; import java.security.Principal; import java.security.BasicPermission; import java.security.CodeSource; import java.security.Permission; import java.security.Permissions; import java.security.ProtectionDomain; public class Test { public static void main(String[] args) throws Exception { // construct a PD key for the WeakHashMap inside SubjectDomainCombiner // // add an instance of MyPerm to the PD key's Permissions object. // MyPerm has a strong reference back to the PD key. Permissions perms = new Permissions(); MyPerm perm = new MyPerm("foo"); perms.add(perm); // create a strong reference to the PD key ProtectionDomain key = new ProtectionDomain (new CodeSource(null, (Certificate[])null), perms, null, null); perm.key = key; // clear out all our other references to the PD key perms = null; perm = null; // get a strong reference to MapMaker. // // MapMaker references // SubjectDomainCombiner references // WeakKeyValueMap references // PD key, new PD value // - new PD value has MyPerm that references PD key MapMaker mapMaker = new MapMaker(); // get a WeakHashMap with the PD key stored as a WeakReference. // use this map to determine whether the PD key gets GC'd properly // inside the SubjectDomainCombiner cache. WeakHashMap myMap = mapMaker.makeMap(key); boolean status = false; for (int i=0; i<300; i++) { Thread.sleep(100); // clear our sole strong reference to the PD key key = null; // now when we run GC, the only possible reason PD key // would not be reclaimed is because the SubjectDomainCombiner's // cache has a new Subject-based PD value that strongly // references the PD key. System.gc(); if (myMap.size() < 1) { status = true; // success break; } } if (status == true) { System.out.println("test passed"); } else { throw new SecurityException("test failed"); } } private static class MapMaker { // strong reference to SubjectDomainCombiner private SubjectDomainCombiner sdc; public WeakHashMap makeMap(ProtectionDomain key) { Subject s = new Subject(); s.getPrincipals().add(new X500Principal("cn=Test")); sdc = new SubjectDomainCombiner(s); ProtectionDomain[] pds = new ProtectionDomain[] { key }; // store PD key in SDC cache, with new Subject-based PD as value. // new Subject-based PD contains MyPerm, which refers back to PD key sdc.combine(pds, null); WeakHashMap returnMe = new WeakHashMap(); returnMe.put(key, ""); return returnMe; } } private static class MyPerm extends BasicPermission { // strong reference to PD key public ProtectionDomain key; public MyPerm(String name) { super(name); } public MyPerm(String name, String action) { super(name, action); } } } ###@###.### 2005-04-08 00:08:48 GMT
19-01-2005