RHEL Engineering is moving the tracking of its product development work on RHEL 6 through RHEL 9 to Red Hat Jira (issues.redhat.com). If you're a Red Hat customer, please continue to file support cases via the Red Hat customer portal. If you're not, please head to the "RHEL project" in Red Hat Jira and file new tickets here. Individual Bugzilla bugs in the statuses "NEW", "ASSIGNED", and "POST" are being migrated throughout September 2023. Bugs of Red Hat partners with an assigned Engineering Partner Manager (EPM) are migrated in late September as per pre-agreed dates. Bugs against components "kernel", "kernel-rt", and "kpatch" are only migrated if still in "NEW" or "ASSIGNED". If you cannot log in to RH Jira, please consult article #7032570. That failing, please send an e-mail to the RH Jira admins at rh-issues@redhat.com to troubleshoot your issue as a user management inquiry. The email creates a ServiceNow ticket with Red Hat. Individual Bugzilla bugs that are migrated will be moved to status "CLOSED", resolution "MIGRATED", and set with "MigratedToJIRA" in "Keywords". The link to the successor Jira issue will be found under "Links", have a little "two-footprint" icon next to it, and direct you to the "RHEL project" in Red Hat Jira (issue links are of type "https://issues.redhat.com/browse/RHEL-XXXX", where "X" is a digit). This same link will be available in a blue banner at the top of the page informing you that that bug has been migrated.
Bug 2016930 - Class loader leaked due to ObjectStreamClass.lookup() [NEEDINFO]
Summary: Class loader leaked due to ObjectStreamClass.lookup()
Keywords:
Status: CLOSED CURRENTRELEASE
Alias: None
Product: Red Hat Enterprise Linux 8
Classification: Red Hat
Component: java-11-openjdk
Version: 8.4
Hardware: x86_64
OS: Linux
unspecified
medium
Target Milestone: rc
: ---
Assignee: Roman Kennke
QA Contact: OpenJDK QA
URL:
Whiteboard:
Depends On:
Blocks:
TreeView+ depends on / blocked
 
Reported: 2021-10-25 08:01 UTC by Simeon Andreev
Modified: 2022-08-04 16:30 UTC (History)
5 users (show)

Fixed In Version: java-11-openjdk-11.0.16.0.8-1.el7_9
Doc Type: If docs needed, set a value
Doc Text:
Clone Of:
Environment:
Last Closed: 2022-08-04 16:30:30 UTC
Type: Bug
Target Upstream Version:
Embargoed:
mmillson: needinfo?


Attachments (Terms of Use)


Links
System ID Private Priority Status Summary Last Updated
Red Hat Issue Tracker RHELPLAN-100597 0 None None None 2021-10-25 08:03:43 UTC
openjdk bug system JDK-8277072 0 None None None 2022-02-14 13:36:40 UTC

Description Simeon Andreev 2021-10-25 08:01:58 UTC
Description of problem:

We observe leaked class loaders due to code in 

Version-Release number of selected component (if applicable):

openjdk version "11.0.10" 2021-01-19 LTS
OpenJDK Runtime Environment 18.9 (build 11.0.10+9-LTS)
OpenJDK 64-Bit Server VM 18.9 (build 11.0.10+9-LTS, mixed mode, sharing)

How reproducible:

Create a folder with the following classes, compile them and run the main class "TestObjectStreamClass":

public class TestObjectStreamClass {

	public static void main(String[] args) throws Exception {
		TestClassLoader myOwnClassLoader = new TestClassLoader();
		Class<?> loadClass = myOwnClassLoader.loadClass("ObjectStreamClass_MemoryLeakExample");

		Object objectStreamClass_MemoryLeakExample = loadClass.newInstance();
		objectStreamClass_MemoryLeakExample.toString();

		java.lang.ref.WeakReference<Object> myOwnClassLoaderWeakReference = new java.lang.ref.WeakReference<>(myOwnClassLoader);
		System.out.println("weak reference before GC and setting to null: " + myOwnClassLoaderWeakReference.get());
		objectStreamClass_MemoryLeakExample = null;
		myOwnClassLoader = null;
		loadClass = null;
		System.out.println("weak reference before GC: " + myOwnClassLoaderWeakReference.get());

		gc();

		System.out.println("weak reference after GC: " + myOwnClassLoaderWeakReference.get());
	}

	public static void gc() {
		System.runFinalization();
		System.gc();
	}
}

public class TestClassLoader extends ClassLoader {

	@Override
	public Class<?> loadClass(String name) throws ClassNotFoundException {
		if (name.equals("TestClass") || name.equals("ObjectStreamClass_MemoryLeakExample")) {
			byte[] bt = loadClassData(name);
			return defineClass(name, bt, 0, bt.length);
		} else {
			return super.loadClass(name);
		}
	}

	private static byte[] loadClassData(String className) {
		java.io.ByteArrayOutputStream byteSt = new java.io.ByteArrayOutputStream();
		try ( java.io.InputStream is = TestClassLoader.class.getClassLoader().getResourceAsStream(className.replace(".", "/") + ".class")) {
			int len = 0;
			while ((len = is.read()) != -1) {
				byteSt.write(len);
			}
		} catch (java.io.IOException e) {
			e.printStackTrace();
		}
		return byteSt.toByteArray();
	}
}

public class ObjectStreamClass_MemoryLeakExample {
	private static final java.io.ObjectStreamField[] fields = java.io.ObjectStreamClass.lookup(TestClass.class).getFields();

	@Override
	public String toString() {
		return java.util.Arrays.toString(fields);
	}
}

public class TestClass implements java.io.Serializable {
	public String x;
}


rm *.class ; /usr/lib/jvm/java-11/bin/javac *.java && /usr/lib/jvm/java-11/bin/java TestObjectStreamClass


Observe output:

Note: TestObjectStreamClass.java uses or overrides a deprecated API.
Note: Recompile with -Xlint:deprecation for details.
weak reference before GC and setting to null: TestClassLoader@6ff3c5b5
weak reference before GC: TestClassLoader@6ff3c5b5
weak reference after GC: TestClassLoader@6ff3c5b5


If call to "java.io.ObjectStreamClass.lookup()" is commented out (i.e. the static field is initialized with a null), the weak reference is cleared as expected:

Note: TestObjectStreamClass.java uses or overrides a deprecated API.
Note: Recompile with -Xlint:deprecation for details.
weak reference before GC and setting to null: TestClassLoader@6ff3c5b5
weak reference before GC: TestClassLoader@6ff3c5b5
weak reference after GC: null


Actual results:

Weak reference is not cleared, indicating the class loader was leaked.

Expected results:

Class loader is not leaked.


Additional info:

See related OpenJDK bug: https://bugs.openjdk.java.net/browse/JDK-8199589

According to the colleague who analysed the issue, the problem is with "localDescs" and "reflectors" of the ObjectStreamClass.Caches class:

private static class Caches {
    /** cache mapping local classes -> descriptors */
    static final ConcurrentMap<WeakClassKey,Reference<?>> localDescs =
        new ConcurrentHashMap<>();

    /** cache mapping field group/local desc pairs -> field reflectors */
    static final ConcurrentMap<FieldReflectorKey,Reference<?>> reflectors =
        new ConcurrentHashMap<>();

    /** queue for WeakReferences to local classes */
    private static final ReferenceQueue<Class<?>> localDescsQueue =
        new ReferenceQueue<>();
    /** queue for WeakReferences to field reflectors keys */
    private static final ReferenceQueue<Class<?>> reflectorsQueue =
        new ReferenceQueue<>();
}

Those are apparently soft references and not weak references.

Comment 3 Simeon Andreev 2021-10-25 08:07:36 UTC
In our application we unload a specific class loader, that loads user code, at controlled points in time. If the user code calls ObjectStreamClass.Caches (in the case we discovered, it was indirectly via "new org.junit.runner.Result.Result()"), class unloading does not occur "on time" and is delayed to a future GC. Our memory leak analysis fails due to this.

Comment 6 Francisco De Melo 2021-11-12 16:52:09 UTC
This is the same issue me and Roman (rkennke) saw already last week and the conclusion was that this is indeed supposed to be a Soft Reference, not a Weak Reference.
The suggestion was for the customer to use Shenandoah, or call twice or three times the GC to clean.
He is not accepting any of the suggestions and he argues this is supposed to be a weak reference, and therefore this must be fixed.

Comment 7 Andrey Loskutov 2021-11-12 17:12:28 UTC
(In reply to Francisco De Melo from comment #6)
> He is not accepting any of the suggestions and he argues this is supposed to
> be a weak reference, and therefore this must be fixed.

"He" is me and I honestly don't care if and how the code works internally, if it uses hard, soft or weak references or no references at all :-).
Our expectation is: the code should not allow to produce memory leaks if used with default G1 GC and without loop with System.gc().

On a heap with ~12 GB System.gc() in a loop costs a lot and our application time costs real money because our application load very expensive HW where customers pay for time the HW is loaded.

Currently our application has memory leak because of the mentioned code and we don't even have a workaround.
If the solution would mean to introduce & use some extra system flag we would be also fine.
We don't want & can't actually switch to a different GC because of performance issues with Shenandoah.

Comment 9 Roman Kennke 2021-11-12 17:45:47 UTC
(In reply to Andrey Loskutov from comment #7)
> (In reply to Francisco De Melo from comment #6)
> > He is not accepting any of the suggestions and he argues this is supposed to
> > be a weak reference, and therefore this must be fixed.
> 
> "He" is me and I honestly don't care if and how the code works internally,
> if it uses hard, soft or weak references or no references at all :-).
> Our expectation is: the code should not allow to produce memory leaks if
> used with default G1 GC and without loop with System.gc().
> 
> On a heap with ~12 GB System.gc() in a loop costs a lot and our application
> time costs real money because our application load very expensive HW where
> customers pay for time the HW is loaded.
> 
> Currently our application has memory leak because of the mentioned code and
> we don't even have a workaround.
> If the solution would mean to introduce & use some extra system flag we
> would be also fine.
> We don't want & can't actually switch to a different GC because of
> performance issues with Shenandoah.

First of all, strictly speaking, it is not a memory leak. The cache will eventually be cleared, and thus the class-loader be reclaimed, when heap gets under pressure. Apparently, with 12GB heap, it will take a while until the GC figures out that cleaning the cache would be reasonable.

However, I agree that the behaviour is really not very useful, either. Very generally speaking, building a cache based on weak or soft references (doesn't matter, really) is asking for troubles, such as you observed. I am thinking that the caches should be implemented by a proper LRU cache. I will take care of it.

Thanks,
Roman

Comment 10 Roman Kennke 2021-11-12 18:34:52 UTC
Alright, I believe using SoftReference here is really non-sensical: the key in the map is a WeakReference to a Class, and the value is a SoftReference to an ObjectStreamClass, which itself also references the Class. That means, that the key will not be reclaimed as long as the Class in the value has been reclaimed, so that needs to go first. And that only happens when: 1. the corresponding ClassLoader gets reclaimed, 2. The GC determines that heap pressure is high enough to reclaim the SoftReference, and then it also needs to wait for the WeakReference to clear, after the SoftReference has gone. In any case, once the ClassLoader is gone, there is absolutely no point in retaining an entry in the cache, and vice versa: there is no reason to keep the entry in the cache, if there is no longer any way to reach the ClassLoader or the Class. So we can just as well use WeakReference instead of SoftReference for the value. I'm testing a fix and will report back.

Comment 11 Roman Kennke 2021-11-12 18:52:49 UTC
tracked upstream: https://bugs.openjdk.java.net/browse/JDK-8277072

Comment 12 Andrey Loskutov 2021-11-29 08:40:39 UTC
Just found another reincarnation of the same problem, this time via using java.io.ObjectOutputStream.writeObject(Object) for serialization. Once we serialize something, unload the class loader and reload the classes again in the new one, we see same leak. java.io.ObjectOutputStream.writeObject(Object) calls into java.io.ObjectOutputStream.writeObject0(Object, boolean) and that calls into java.io.ObjectStreamClass.lookup(Class<?>, boolean) which is where the SoftReferences are created => same issue as seen here before.

Regardless of *how* we introduce the memory leak, we've also found that we have a functional problem in our application, that affects interactive debugging. 

After the first leak is introduced, and we have 1 live + N (supposed to be unloaded) class loaders, we see that hot code replace starts to fail on classes with structural changes, because the same classes are still referenced by supposed to be unloaded classloaders.

Note, "debugging" is not meant to be something special, our application implements a debugger for the user code and that is an essential part of the functionality we offer to users.

Roman: could you share your patch (ideally for Java 11, but would be also OK for 17)? 
Do you have any insights if and when it could be merged into JDK?

Comment 13 Roman Kennke 2021-12-10 16:29:16 UTC
> Roman: could you share your patch (ideally for Java 11, but would be also OK
> for 17)? 
> Do you have any insights if and when it could be merged into JDK?


The fix, which turned out to be much more complex than the originally proposed change from SoftReference to WeakReference - which was wrong - has been integrated into JDK 19 branch just now:
https://github.com/openjdk/jdk/pull/6375

After some bake-time, it will be backported to JDK 18, JDK 17, and eventually to JDK 11 (if accepted).

Thanks,
Roman


Note You need to log in before you can comment on or make changes to this bug.