From 5cd5011ec0903d316ab6a26f89f6d0243a714bdd Mon Sep 17 00:00:00 2001 From: Lars Vogel Date: Thu, 16 Apr 2026 19:39:35 +0200 Subject: [PATCH] Fix flaky ensureCleanUpAddonCleansUp by holding CleanupAddon strongly The test created the CleanupAddon via ContextInjectionFactory.make(...) without keeping the returned reference. The DI framework only holds injected objects via WeakReference (InjectorImpl.injectedObjects), so GC could collect the addon between event registration and event firing. Once collected, the @UIEventTopic handler observed requestor.isValid() == false and silently unsubscribed without invoking the addon's method. The asyncExec that would have set partStack toBeRendered=false was never queued, leaving the assertion to fail. Storing the addon in the application context keeps a strong reference for the lifetime of the test, matching how addons are referenced in production via the application model. Reproduced locally (Linux, DISPLAY=:1) at ~1.2% failure rate over 500 iterations of the test. With the fix, 500 iterations all pass. CI also ran the test 500 times across all platforms (green) before this commit removed the temporary @RepeatedTest(500) annotation. The same pattern is applied to testBug332463 and CleanupAddonTest, which had the same latent issue. Refs https://github.com/eclipse-platform/eclipse.platform.ui/issues/3581 --- .../ui/tests/workbench/PartRenderingEngineTests.java | 11 +++++++---- .../addons/cleanupaddon/CleanupAddonTest.java | 6 +++++- 2 files changed, 12 insertions(+), 5 deletions(-) diff --git a/tests/org.eclipse.e4.ui.tests/src/org/eclipse/e4/ui/tests/workbench/PartRenderingEngineTests.java b/tests/org.eclipse.e4.ui.tests/src/org/eclipse/e4/ui/tests/workbench/PartRenderingEngineTests.java index 3fa7f8b7679..ebd0a8c4eff 100644 --- a/tests/org.eclipse.e4.ui.tests/src/org/eclipse/e4/ui/tests/workbench/PartRenderingEngineTests.java +++ b/tests/org.eclipse.e4.ui.tests/src/org/eclipse/e4/ui/tests/workbench/PartRenderingEngineTests.java @@ -2435,7 +2435,11 @@ public void ensureCleanUpAddonCleansUp() { partStackForEditor.getChildren().add(editor); partStackForEditor.setSelectedElement(editor); - ContextInjectionFactory.make(CleanupAddon.class, appContext); + // Store the addon in the context so it isn't garbage collected before + // its event handlers fire. The DI framework holds injected objects + // only via WeakReference, so an addon discarded here can be collected + // before its asyncExec runs, silently invalidating the requestor. + appContext.set(CleanupAddon.class, ContextInjectionFactory.make(CleanupAddon.class, appContext)); contextRule.createAndRunWorkbench(window); @@ -2445,8 +2449,6 @@ public void ensureCleanUpAddonCleansUp() { assertTrue(partStackForPartBPartC.isToBeRendered(), " PartStack with children should be rendered"); partService.hidePart(partB); - // Drain pending events between hides so that the event queue is clean - // when the second hidePart queues CleanupAddon's asyncExec for visCount==0 contextRule.spinEventLoop(); partService.hidePart(partC); contextRule.spinEventLoop(); @@ -2494,7 +2496,8 @@ public void testBug332463() { partStackB.getChildren().add(partC); partStackB.setSelectedElement(partC); - ContextInjectionFactory.make(CleanupAddon.class, appContext); + // See ensureCleanUpAddonCleansUp for why the addon is stored in the context. + appContext.set(CleanupAddon.class, ContextInjectionFactory.make(CleanupAddon.class, appContext)); contextRule.createAndRunWorkbench(window); diff --git a/tests/org.eclipse.e4.ui.workbench.addons.swt.test/src/org/eclipse/e4/ui/workbench/addons/cleanupaddon/CleanupAddonTest.java b/tests/org.eclipse.e4.ui.workbench.addons.swt.test/src/org/eclipse/e4/ui/workbench/addons/cleanupaddon/CleanupAddonTest.java index a5322b94e4c..0ce7b35f3a3 100644 --- a/tests/org.eclipse.e4.ui.workbench.addons.swt.test/src/org/eclipse/e4/ui/workbench/addons/cleanupaddon/CleanupAddonTest.java +++ b/tests/org.eclipse.e4.ui.workbench.addons.swt.test/src/org/eclipse/e4/ui/workbench/addons/cleanupaddon/CleanupAddonTest.java @@ -164,7 +164,11 @@ private void prepareApplicationModel() { appContext.set(MAddon.class, cleanupAddon); ContextInjectionFactory.setDefault(appContext); - ContextInjectionFactory.make(CleanupAddon.class, appContext); + // Store the addon in the context so it isn't garbage collected before + // its event handlers fire. The DI framework holds injected objects + // only via WeakReference, so an addon discarded here can be collected + // before its asyncExec runs, silently invalidating the requestor. + appContext.set(CleanupAddon.class, ContextInjectionFactory.make(CleanupAddon.class, appContext)); appContext.set(IResourceUtilities.class, new ISWTResourceUtilities() {