From 9b561fc629145777a4ad08d56c2c1df18d4696be Mon Sep 17 00:00:00 2001 From: Simeon Andreev Date: Tue, 17 Mar 2026 10:31:37 +0200 Subject: [PATCH 1/2] Revert "Fix race condition in CleanupAddon.subscribeRenderingChanged" This reverts commit 54b8aa2d1c2d4b00e67d18a602ae671df3ab2d72. --- .../addons/cleanupaddon/CleanupAddon.java | 17 ++++++++++------- .../workbench/PartRenderingEngineTests.java | 10 +++++++--- 2 files changed, 17 insertions(+), 10 deletions(-) diff --git a/bundles/org.eclipse.e4.ui.workbench.addons.swt/src/org/eclipse/e4/ui/workbench/addons/cleanupaddon/CleanupAddon.java b/bundles/org.eclipse.e4.ui.workbench.addons.swt/src/org/eclipse/e4/ui/workbench/addons/cleanupaddon/CleanupAddon.java index beb4e0f9dfee..c2fab02e0657 100644 --- a/bundles/org.eclipse.e4.ui.workbench.addons.swt/src/org/eclipse/e4/ui/workbench/addons/cleanupaddon/CleanupAddon.java +++ b/bundles/org.eclipse.e4.ui.workbench.addons.swt/src/org/eclipse/e4/ui/workbench/addons/cleanupaddon/CleanupAddon.java @@ -346,13 +346,16 @@ private void subscribeRenderingChanged( int visCount = modelService.countRenderableChildren(container); // Remove stacks with no visible children from the display (but not the - // model). Since event delivery is synchronous (EventBroker.send via - // EventAdmin.sendEvent and Display.syncExec), the visCount reflects - // the actual state at the time of this event and can be acted upon - // immediately without deferring via asyncExec. - if (visCount == 0 && !isLastEditorStack(container)) { - container.setToBeRendered(false); - transferPrimaryDataStackIfRemoved(container); + // model) + final MElementContainer theContainer = container; + if (visCount == 0) { + Display.getCurrent().asyncExec(() -> { + int visCount1 = modelService.countRenderableChildren(theContainer); + if (!isLastEditorStack(theContainer) && visCount1 == 0) { + theContainer.setToBeRendered(false); + transferPrimaryDataStackIfRemoved(theContainer); + } + }); } else if (container.getParent() != null) { // omit detached windows // if there are rendered elements but none are 'visible' we should // make the container invisible as well 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 d646a2150eef..b62b0d41d6ac 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 @@ -64,6 +64,7 @@ import org.eclipse.swt.widgets.Shell; import org.eclipse.swt.widgets.ToolBar; import org.eclipse.swt.widgets.Widget; +import org.eclipse.ui.tests.harness.util.DisplayHelper; import org.junit.jupiter.api.AfterEach; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Disabled; @@ -2446,9 +2447,12 @@ public void ensureCleanUpAddonCleansUp() { assertTrue(partStackForPartBPartC.isToBeRendered(), " PartStack with children should be rendered"); partService.hidePart(partB); partService.hidePart(partC); - contextRule.spinEventLoop(); - assertFalse(partStackForPartBPartC.isToBeRendered(), - "CleanupAddon should ensure that partStack is not rendered anymore, as all childs have been removed"); + // DisplayHelper.waitForCondition() handles event processing via Display.sleep() + // and retries. Calling spinEventLoop() here creates a race condition where + // events may be processed before CleanupAddon's asyncExec() is queued (line 352). + assertTrue( + DisplayHelper.waitForCondition(Display.getDefault(), 30_000, + () -> !partStackForPartBPartC.isToBeRendered()), "CleanupAddon should ensure that partStack is not rendered anymore, as all childs have been removed"); // PartStack with IPresentationEngine.NO_AUTO_COLLAPSE should not be removed // even if children are removed partService.hidePart(editor, true); From dd7da4b4bb1089ffe7d3ebdf5fdeda37d75381a6 Mon Sep 17 00:00:00 2001 From: Lars Vogel Date: Tue, 17 Mar 2026 09:50:56 +0100 Subject: [PATCH 2/2] Fix flaky ensureCleanUpAddonCleansUp test Add spinEventLoop() between the two hidePart() calls to drain pending events before the second hide. This ensures a clean event queue when CleanupAddon's asyncExec is queued for visCount==0, replacing the 30-second DisplayHelper.waitForCondition polling with a deterministic event drain. Fixes https://github.com/eclipse-platform/eclipse.platform.ui/issues/3581 --- .../tests/workbench/PartRenderingEngineTests.java | 13 ++++++------- 1 file changed, 6 insertions(+), 7 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 b62b0d41d6ac..3fa7f8b7679e 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 @@ -64,7 +64,6 @@ import org.eclipse.swt.widgets.Shell; import org.eclipse.swt.widgets.ToolBar; import org.eclipse.swt.widgets.Widget; -import org.eclipse.ui.tests.harness.util.DisplayHelper; import org.junit.jupiter.api.AfterEach; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Disabled; @@ -2446,13 +2445,13 @@ 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); - // DisplayHelper.waitForCondition() handles event processing via Display.sleep() - // and retries. Calling spinEventLoop() here creates a race condition where - // events may be processed before CleanupAddon's asyncExec() is queued (line 352). - assertTrue( - DisplayHelper.waitForCondition(Display.getDefault(), 30_000, - () -> !partStackForPartBPartC.isToBeRendered()), "CleanupAddon should ensure that partStack is not rendered anymore, as all childs have been removed"); + contextRule.spinEventLoop(); + assertFalse(partStackForPartBPartC.isToBeRendered(), + "CleanupAddon should ensure that partStack is not rendered anymore, as all childs have been removed"); // PartStack with IPresentationEngine.NO_AUTO_COLLAPSE should not be removed // even if children are removed partService.hidePart(editor, true);