7 JavaLeakTest fail since I20251007-1800

[iloveeclipse]

See for example https://download.eclipse.org/eclipse/downloads/drops4/I20251007-1800/testresults/html/org.eclipse.jdt.ui.tests_ep438I-unit-linux-x86_64-java21_linux.gtk.x86_64_21.html

java.lang.AssertionError: Expected instance count: 0, actual: 1
Element 0
org.eclipse.jdt.internal.ui.javaeditor.CompilationUnitEditor
org.eclipse.jdt.internal.ui.javaeditor.CompilationUnitEditor$AdaptedSourceViewer#this$0 -> org.eclipse.jdt.internal.ui.javaeditor.CompilationUnitEditor@36dd4ac6
org.eclipse.jface.text.source.projection.ProjectionViewer$UpdateDocumentListener#this$0 -> org.eclipse.jdt.internal.ui.javaeditor.CompilationUnitEditor$AdaptedSourceViewer@28797e8a
java.lang.Object[0] -> org.eclipse.jface.text.source.projection.ProjectionViewer$UpdateDocumentListener@68d08318
org.eclipse.core.runtime.ListenerList#listeners -> [Ljava.lang.Object;@29b720cd
org.eclipse.jface.text.AbstractDocument#fDocumentListeners -> [org.eclipse.jface.text.source.projection.ProjectionViewer$UpdateDocumentListener@68d08318]
org.eclipse.jface.text.TextSelection#fDocument -> org.eclipse.core.internal.filebuffers.SynchronizableDocument@12864353
org.eclipse.ui.internal.handlers.ActionDelegateHandlerProxy#currentSelection -> TextSelection [offset: 0, startLine: 0, length: 0, document: org.eclipse.core.internal.filebuffers.
org.eclipse.ui.internal.handlers.HandlerActivation#handler -> ActionDelegateHandlerProxy(org.eclipse.jdt.ui.tests.leaks.JavaLeakTestActionDelegate@748078e0,org.e
java.lang.Object[84] -> [ActionDelegateHandlerProxy(org.eclipse.jdt.ui.tests.leaks.JavaLeakTestActionDelegate@748078e0,org.
java.util.ArrayList#elementData -> [Ljava.lang.Object;@1356527a
org.eclipse.ui.internal.menus.LegacyActionPersistence#handlerActivations -> [[ActionDelegateHandlerProxy(null,org.eclipse.debug.internal.ui.actions.breakpoints.RetargetToggleB
org.eclipse.ui.internal.services.RegistryPersistence$$Lambda/0x00007f76ef33fdd8#arg$1 -> org.eclipse.ui.internal.menus.LegacyActionPersistence@50df3772
org.eclipse.core.internal.registry.ExtensionRegistry$ListenerInfo#listener -> org.eclipse.ui.internal.services.RegistryPersistence$$Lambda/0x00007f76ef33fdd8@453f92db
java.lang.Object[32] -> org.eclipse.core.internal.registry.ExtensionRegistry$ListenerInfo@453f92db
org.eclipse.core.runtime.ListenerList#listeners -> [Ljava.lang.Object;@11bc64f6
org.eclipse.core.internal.registry.ExtensionRegistry#listeners -> [org.eclipse.core.internal.registry.ExtensionRegistry$ListenerInfo@14f13587, org.eclipse.core.inter
org.eclipse.osgi.internal.serviceregistry.ServiceRegistrationImpl#service -> org.eclipse.core.internal.registry.ExtensionRegistry@7107a4a3
java.lang.Object[51] -> {org.eclipse.core.runtime.IExtensionRegistry}={service.id=51, service.bundleid=176, service.scope=s
java.util.ArrayList#elementData -> [Ljava.lang.Object;@2a47fb5d
org.eclipse.osgi.internal.serviceregistry.ServiceRegistry#allPublishedServices -> [{org.eclipse.osgi.framework.log.FrameworkLog}={service.id=4, service.bundleid=0, service.scope=bun
org.eclipse.osgi.internal.framework.EquinoxContainer#serviceRegistry -> org.eclipse.osgi.internal.serviceregistry.ServiceRegistry@74c997b1
org.eclipse.osgi.internal.loader.BundleLoader#container -> Equinox Container: d57b11db-e23c-4f58-90ea-e4b32796a4ee
org.eclipse.osgi.internal.loader.EquinoxClassLoader#delegate -> org.eclipse.jdt.ui.tests_3.16.300.v20250929-2035
org.eclipse.osgi.internal.loader.EquinoxClassLoader -> org.eclipse.osgi.internal.loader.EquinoxClassLoader@2bd15a2d[org.eclipse.jdt.ui.tests:3.16.300.v202

. Actual: 0
at org.junit.Assert.fail(Assert.java:89)
at org.junit.Assert.failEquals(Assert.java:187)
at org.junit.Assert.assertNotEquals(Assert.java:201)
at org.eclipse.jdt.ui.leaktest.LeakTestCase.assertInstanceCount(LeakTestCase.java:133)
at org.eclipse.jdt.ui.tests.leaks.JavaLeakTest.testJavaEditorActionDelegate(JavaLeakTest.java:483)
at java.base/java.lang.reflect.Method.invoke(Method.java:580)

This is a regression from eclipse-platform/eclipse.platform.ui#3074.

The UpdateDocumentListener added in that PR is not always removed, causing memory leak.

[iloveeclipse]

This patch fixes the leak, however I'm not sure whether we have troubles with not removing listeners from documents if they are exchanged multiple times on the viewer.

diff --git a/bundles/org.eclipse.jface.text/projection/org/eclipse/jface/text/source/projection/ProjectionViewer.java b/bundles/org.eclipse.jface.text/projection/org/eclipse/jface/text/source/projection/ProjectionViewer.java
index 50fbf0e..a99a855 100644
--- a/bundles/org.eclipse.jface.text/projection/org/eclipse/jface/text/source/projection/ProjectionViewer.java
+++ b/bundles/org.eclipse.jface.text/projection/org/eclipse/jface/text/source/projection/ProjectionViewer.java
@@ -1482,2 +1482,8 @@
 		fWasProjectionEnabled= false;
+		if (fUpdateDocumentListener != null) {
+			IDocument document= getDocument();
+			if (document != null) {
+				document.removeDocumentListener(fUpdateDocumentListener);
+			}
+		}
 		super.handleDispose();

[iloveeclipse]

I'm not sure whether we have troubles with not removing listeners from documents if they are exchanged multiple times on the viewer.

Looking on API, it is possible to set different documents, so a proper fix is needed.

I've pushed eclipse-platform/eclipse.platform.ui#3382 to unblock JDT UI tests in the hope that full fix will be provided later on.

[danthe1st]

Oh, it seems like I forgot updating

I'm not sure whether we have troubles with not removing listeners from documents if they are exchanged multiple times on the viewer.

@iloveeclipse What do you mean exchanging the listener multiple times per viewer? Do you mean when the document of a ProjectionViewer is changed? setDocument is actually another place where the listener needs to be removed.

diff --git a/bundles/org.eclipse.jface.text/projection/org/eclipse/jface/text/source/projection/ProjectionViewer.java b/bundles/org.eclipse.jface.text/projection/org/eclipse/jface/text/source/projection/ProjectionViewer.java
index 50fbf0e165..852923833a 100644
--- a/bundles/org.eclipse.jface.text/projection/org/eclipse/jface/text/source/projection/ProjectionViewer.java
+++ b/bundles/org.eclipse.jface.text/projection/org/eclipse/jface/text/source/projection/ProjectionViewer.java
@@ -400,6 +400,9 @@ public class ProjectionViewer extends SourceViewer implements ITextViewerExtensi
                }
 
                if (fProjectionAnnotationModel != null) {
+                       if (document != null) {
+                               document.removeDocumentListener(fUpdateDocumentListener);
+                       }
                        wasProjectionEnabled= removeProjectionAnnotationModel(getVisualAnnotationModel()) != null;
                        fProjectionAnnotationModel= null;
                }

Or do you mean removing the same listener multiple times?

[iloveeclipse]

setDocument is actually another place where the listener needs to be removed.

I mean exactly that, I saw there are multiple setDocument() methods and I honestly have no idea which could be related (or all).

I've pushed eclipse-platform/eclipse.platform.ui#3382 which fixes JDT fails for now, because if they would constantly fail, they may hide yet another regression, and it is not always trivial to find the root cause after few days and many PR's merged...

It would be good if you could analyze all listener "escape" places & provide a better fix.

[danthe1st]

I mean exactly that, I saw there are multiple setDocument() methods and I honestly have no idea which could be related (or all).

There are four setDocument() in total but they all call setDocument(IDocument, IAnnotationModel, int, int) (done in SourceViewer.java) which is the one that is overridden here so the simple solution of adding a check in that method should work for that.

However, there is also SourceViewer#disposeVisualAnnotationModel that is called both from setDocument (though possibly multiple times but that shouldn't be an issue for removing listeners?) and handleDispose. I don't know which option you prefer.