From be434268002f2f2fe7d8f15bf1bc5c085d162ca7 Mon Sep 17 00:00:00 2001 From: Lars Vogel Date: Thu, 23 Apr 2026 19:54:39 +0200 Subject: [PATCH 1/4] Avoid flashing shutdown dialog when save is fast Replace the CancelableProgressMonitorJobsDialog used during workspace shutdown with a plain ProgressMonitorJobsDialog that opens only after IProgressService#getLongOperationTime has elapsed. During a fast save only a busy cursor is shown, which fixes issue 1269. The previous implementation eagerly opened the dialog and wired a cancel button whose only effect was to clear a sub-task label. Its CancelableProgressMonitorWrapper relied on hard-coded work-amount math to toggle cancellation around the history-pruning phase, an assumption that drifted out of sync with SaveManager. That plumbing is removed along with its now-unused messages, since cancelling shutdown is no longer offered. --- .../ide/application/IDEWorkbenchAdvisor.java | 143 +++++------------- .../ui/internal/ide/IDEWorkbenchMessages.java | 3 - .../ui/internal/ide/messages.properties | 3 - .../application/IDEWorkbenchAdvisorTest.java | 59 ++++++++ 4 files changed, 100 insertions(+), 108 deletions(-) diff --git a/bundles/org.eclipse.ui.ide.application/src/org/eclipse/ui/internal/ide/application/IDEWorkbenchAdvisor.java b/bundles/org.eclipse.ui.ide.application/src/org/eclipse/ui/internal/ide/application/IDEWorkbenchAdvisor.java index 30c6d5ad70b..75904a06f37 100644 --- a/bundles/org.eclipse.ui.ide.application/src/org/eclipse/ui/internal/ide/application/IDEWorkbenchAdvisor.java +++ b/bundles/org.eclipse.ui.ide.application/src/org/eclipse/ui/internal/ide/application/IDEWorkbenchAdvisor.java @@ -42,7 +42,6 @@ import org.eclipse.core.runtime.IStatus; import org.eclipse.core.runtime.MultiStatus; import org.eclipse.core.runtime.Platform; -import org.eclipse.core.runtime.ProgressMonitorWrapper; import org.eclipse.core.runtime.Status; import org.eclipse.core.runtime.jobs.ISchedulingRule; import org.eclipse.core.runtime.jobs.Job; @@ -58,13 +57,10 @@ import org.eclipse.jface.util.Policy; import org.eclipse.jface.window.Window; import org.eclipse.swt.SWT; -import org.eclipse.swt.events.SelectionAdapter; -import org.eclipse.swt.events.SelectionEvent; +import org.eclipse.swt.custom.BusyIndicator; import org.eclipse.swt.graphics.Resource; -import org.eclipse.swt.widgets.Composite; import org.eclipse.swt.widgets.Display; import org.eclipse.swt.widgets.Listener; -import org.eclipse.swt.widgets.Shell; import org.eclipse.ui.PlatformUI; import org.eclipse.ui.application.IWorkbenchConfigurer; import org.eclipse.ui.application.IWorkbenchWindowConfigurer; @@ -83,6 +79,7 @@ import org.eclipse.ui.internal.ide.IDEWorkbenchMessages; import org.eclipse.ui.internal.ide.IDEWorkbenchPlugin; import org.eclipse.ui.internal.ide.undo.WorkspaceUndoMonitor; +import org.eclipse.ui.internal.progress.ProgressManagerUtil; import org.eclipse.ui.internal.progress.ProgressMonitorJobsDialog; import org.eclipse.ui.progress.IProgressService; import org.eclipse.ui.statushandlers.AbstractStatusHandler; @@ -464,119 +461,61 @@ public IStatus runInWorkspace(IProgressMonitor monitor) throws CoreException { } } - protected static class CancelableProgressMonitorWrapper extends - ProgressMonitorWrapper { - private double total = 0; - private final ProgressMonitorJobsDialog dialog; - - CancelableProgressMonitorWrapper(IProgressMonitor monitor, - ProgressMonitorJobsDialog dialog) { - super(monitor); - this.dialog = dialog; - } - - @Override - public void internalWorked(double work) { - super.internalWorked(work); - total += work; - updateProgressDetails(); - } - - @Override - public void worked(int work) { - super.worked(work); - total += work; - updateProgressDetails(); - } - - @Override - public void beginTask(String name, int totalWork) { - super.beginTask(name, totalWork); - subTask(IDEWorkbenchMessages.IDEWorkbenchAdvisor_preHistoryCompaction); - } - - private void updateProgressDetails() { - if (!isCanceled() && Math.abs(total - 4.0) < 0.0001 /* right before history compacting */) { - subTask(IDEWorkbenchMessages.IDEWorkbenchAdvisor_cancelHistoryPruning); - dialog.setCancelable(true); - } - if (Math.abs(total - 5.0) < 0.0001 /* history compacting finished */) { - subTask(IDEWorkbenchMessages.IDEWorkbenchAdvisor_postHistoryCompaction); - dialog.setCancelable(false); - } - } - } - - protected static class CancelableProgressMonitorJobsDialog extends - ProgressMonitorJobsDialog { - - public CancelableProgressMonitorJobsDialog(Shell parent) { - super(parent); - } - - @Override - protected void createButtonsForButtonBar(Composite parent) { - super.createButtonsForButtonBar(parent); - registerCancelButtonListener(); - } - - public void registerCancelButtonListener() { - cancel.addSelectionListener(new SelectionAdapter() { - @Override - public void widgetSelected(SelectionEvent e) { - subTaskLabel.setText(""); //$NON-NLS-1$ - } - }); - } - } - /** * Disconnect from the core workspace. * + * Shows the progress dialog only if the save operation takes longer than + * the {@link IProgressService#getLongOperationTime() long operation time}; + * otherwise a busy cursor is shown while the save runs on a worker thread. + * This avoids a flashing dialog for fast shutdowns. + * * Locks workspace in a background thread, should not be called while * holding any workspace locks. */ protected void disconnectFromWorkspace() { - // save the workspace final MultiStatus status = new MultiStatus(IDEWorkbenchPlugin.IDE_WORKBENCH, 1, IDEWorkbenchMessages.ProblemSavingWorkbench); - try { - final ProgressMonitorJobsDialog p = new CancelableProgressMonitorJobsDialog( - null); - final boolean applyPolicy = ResourcesPlugin.getWorkspace() - .getDescription().isApplyFileStatePolicy(); + final ProgressMonitorJobsDialog dialog = new ProgressMonitorJobsDialog(null); + dialog.setOpenOnRun(false); - IRunnableWithProgress runnable = monitor -> { - try { - if (applyPolicy) { - monitor = new CancelableProgressMonitorWrapper(monitor, p); - } + IRunnableWithProgress runnable = monitor -> { + try { + status.merge(((Workspace) ResourcesPlugin.getWorkspace()).save(true, true, monitor)); + } catch (CoreException e) { + status.merge(e.getStatus()); + } + }; - status.merge(((Workspace) ResourcesPlugin.getWorkspace()).save(true, true, monitor)); - } catch (CoreException e) { - status.merge(e.getStatus()); - } - }; + final Display display = PlatformUI.getWorkbench().getDisplay(); + final int longOperationTime = PlatformUI.getWorkbench().getProgressService().getLongOperationTime(); + final Runnable openDialog = () -> { + if (ProgressManagerUtil.safeToOpen(dialog, null)) { + dialog.open(); + } + }; + display.timerExec(longOperationTime, openDialog); - p.run(true, false, runnable); - } catch (InvocationTargetException e) { - status - .merge(new Status(IStatus.ERROR, - IDEWorkbenchPlugin.IDE_WORKBENCH, 1, - IDEWorkbenchMessages.InternalError, e - .getTargetException())); - } catch (InterruptedException e) { - status.merge(new Status(IStatus.ERROR, - IDEWorkbenchPlugin.IDE_WORKBENCH, 1, - IDEWorkbenchMessages.InternalError, e)); + try { + BusyIndicator.showWhile(display, () -> { + try { + dialog.run(true, false, runnable); + } catch (InvocationTargetException e) { + status.merge(new Status(IStatus.ERROR, IDEWorkbenchPlugin.IDE_WORKBENCH, 1, + IDEWorkbenchMessages.InternalError, e.getTargetException())); + } catch (InterruptedException e) { + status.merge(new Status(IStatus.ERROR, IDEWorkbenchPlugin.IDE_WORKBENCH, 1, + IDEWorkbenchMessages.InternalError, e)); + } + }); + } finally { + display.timerExec(-1, openDialog); } + if (!status.isOK()) { - ErrorDialog.openError(null, - IDEWorkbenchMessages.ProblemsSavingWorkspace, null, status, + ErrorDialog.openError(null, IDEWorkbenchMessages.ProblemsSavingWorkspace, null, status, IStatus.ERROR | IStatus.WARNING); - IDEWorkbenchPlugin.log( - IDEWorkbenchMessages.ProblemsSavingWorkspace, status); + IDEWorkbenchPlugin.log(IDEWorkbenchMessages.ProblemsSavingWorkspace, status); } } diff --git a/bundles/org.eclipse.ui.ide/src/org/eclipse/ui/internal/ide/IDEWorkbenchMessages.java b/bundles/org.eclipse.ui.ide/src/org/eclipse/ui/internal/ide/IDEWorkbenchMessages.java index bec664b18a5..09eb0bc9531 100644 --- a/bundles/org.eclipse.ui.ide/src/org/eclipse/ui/internal/ide/IDEWorkbenchMessages.java +++ b/bundles/org.eclipse.ui.ide/src/org/eclipse/ui/internal/ide/IDEWorkbenchMessages.java @@ -37,9 +37,6 @@ public class IDEWorkbenchMessages extends NLS { // package: org.eclipse.ui.ide public static String IDEWorkbenchAdvisor_noPerspective; - public static String IDEWorkbenchAdvisor_cancelHistoryPruning; - public static String IDEWorkbenchAdvisor_preHistoryCompaction; - public static String IDEWorkbenchAdvisor_postHistoryCompaction; public static String IDE_noFileEditorSelectedUserCanceled; public static String IDE_noFileEditorFound; diff --git a/bundles/org.eclipse.ui.ide/src/org/eclipse/ui/internal/ide/messages.properties b/bundles/org.eclipse.ui.ide/src/org/eclipse/ui/internal/ide/messages.properties index 348c34e50b9..ae357abe00d 100644 --- a/bundles/org.eclipse.ui.ide/src/org/eclipse/ui/internal/ide/messages.properties +++ b/bundles/org.eclipse.ui.ide/src/org/eclipse/ui/internal/ide/messages.properties @@ -41,9 +41,6 @@ # package: org.eclipse.ui.ide IDEWorkbenchAdvisor_noPerspective=No perspectives are open. To open a perspective, press this button: -IDEWorkbenchAdvisor_cancelHistoryPruning=Cancel to skip compacting local history. -IDEWorkbenchAdvisor_preHistoryCompaction=Saving workbench state. -IDEWorkbenchAdvisor_postHistoryCompaction=Disconnecting from workspace. IDE_noFileEditorSelectedUserCanceled = No editor selected, operation canceled by user. IDE_noFileEditorFound = No editor found to edit the file resource. diff --git a/tests/org.eclipse.ui.ide.application.tests/src/org/eclipse/ui/internal/ide/application/IDEWorkbenchAdvisorTest.java b/tests/org.eclipse.ui.ide.application.tests/src/org/eclipse/ui/internal/ide/application/IDEWorkbenchAdvisorTest.java index b62112f9c31..ea7e37e1e07 100644 --- a/tests/org.eclipse.ui.ide.application.tests/src/org/eclipse/ui/internal/ide/application/IDEWorkbenchAdvisorTest.java +++ b/tests/org.eclipse.ui.ide.application.tests/src/org/eclipse/ui/internal/ide/application/IDEWorkbenchAdvisorTest.java @@ -41,6 +41,7 @@ public class IDEWorkbenchAdvisorTest { private static final String PLUGIN_ID = "org.eclipse.ui.ide.application.tests"; private Display display = null; private ISchedulingRule rule; + private final IWorkspace workspace = ResourcesPlugin.getWorkspace(); private static final class SaveHook implements ISaveParticipant, Closeable { public ISaveContext saving = null; @@ -200,6 +201,64 @@ public void postShutdown() { } } + /** + * Workbench shutdown should complete cleanly when the workspace save takes + * longer than the progress service's long operation time. In that case the + * progress dialog is expected to open after the delay, but the save must + * still run to completion and the save hooks must fire. + * + * Regression test for issue 1269 (flashing shutdown dialog). + */ + @Test + public void testShutdownWithSlowSave() throws CoreException { + try (SaveHook saveHook = new SaveHook()) { + // Exceeds the default IProgressService#getLongOperationTime (800ms), + // so the new code path schedules and opens the progress dialog. + final long sleepMillis = 1200L; + workspace.addSaveParticipant(PLUGIN_ID + ".slow", new ISaveParticipant() { + @Override + public void saving(ISaveContext context) { + try { + Thread.sleep(sleepMillis); + } catch (InterruptedException e) { + Thread.currentThread().interrupt(); + } + } + + @Override + public void prepareToSave(ISaveContext context) { + } + + @Override + public void doneSaving(ISaveContext context) { + } + + @Override + public void rollback(ISaveContext context) { + } + }); + try { + IDEWorkbenchAdvisor advisor = new IDEWorkbenchAdvisor() { + @Override + public void postStartup() { + super.postStartup(); + display.asyncExec(() -> PlatformUI.getWorkbench().close()); + } + }; + int returnCode = PlatformUI.createAndRunWorkbench(display, advisor); + assertEquals(PlatformUI.RETURN_OK, returnCode); + dispatchDisplay(); + + assertNotNull(saveHook.prepareToSave); + assertNotNull(saveHook.saving); + assertNotNull(saveHook.doneSaving); + assertNull(saveHook.rollback); + } finally { + workspace.removeSaveParticipant(PLUGIN_ID + ".slow"); + } + } + } + /** * Workbench shutdown should disconnect workspace if it is not locked * From 44b4cff092405430a405705b425ff7e51ed617c0 Mon Sep 17 00:00:00 2001 From: Lars Vogel Date: Fri, 24 Apr 2026 06:42:20 +0200 Subject: [PATCH 2/4] Capture long-operation time before workbench shutdown Avoid calling PlatformUI.getWorkbench() from disconnectFromWorkspace(), which runs from postShutdown() and may execute (via the workspace-locked timerExec retry) after the workbench has been torn down. The display is fetched via Display.getCurrent(), matching the existing pattern in postShutdown(), and the long-operation time is captured in initialize(). --- .../ide/application/IDEWorkbenchAdvisor.java | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-) diff --git a/bundles/org.eclipse.ui.ide.application/src/org/eclipse/ui/internal/ide/application/IDEWorkbenchAdvisor.java b/bundles/org.eclipse.ui.ide.application/src/org/eclipse/ui/internal/ide/application/IDEWorkbenchAdvisor.java index 75904a06f37..10b43ea702a 100644 --- a/bundles/org.eclipse.ui.ide.application/src/org/eclipse/ui/internal/ide/application/IDEWorkbenchAdvisor.java +++ b/bundles/org.eclipse.ui.ide.application/src/org/eclipse/ui/internal/ide/application/IDEWorkbenchAdvisor.java @@ -153,6 +153,13 @@ public class IDEWorkbenchAdvisor extends WorkbenchAdvisor { */ private final int workspaceWaitDelay; + /** + * Captured during {@link #initialize(IWorkbenchConfigurer)} so + * {@link #disconnectFromWorkspace()} does not have to reach through + * {@link PlatformUI#getWorkbench()} after the workbench has been shut down. + */ + private int savedLongOperationTime; + private final Listener closeListener = event -> { boolean doExit = IDEWorkbenchWindowAdvisor.promptOnExit(null); event.doit = doExit; @@ -191,6 +198,8 @@ public IDEWorkbenchAdvisor(DelayedEventsProcessor processor) { @Override public void initialize(IWorkbenchConfigurer configurer) { + savedLongOperationTime = configurer.getWorkbench().getProgressService().getLongOperationTime(); + PluginActionBuilder.setAllowIdeLogging(true); initResourceTracking(); @@ -487,8 +496,8 @@ protected void disconnectFromWorkspace() { } }; - final Display display = PlatformUI.getWorkbench().getDisplay(); - final int longOperationTime = PlatformUI.getWorkbench().getProgressService().getLongOperationTime(); + final Display display = Display.getCurrent(); + final int longOperationTime = savedLongOperationTime; final Runnable openDialog = () -> { if (ProgressManagerUtil.safeToOpen(dialog, null)) { dialog.open(); From e1a5ecea876457ef42f120cec19a01fd3594dec4 Mon Sep 17 00:00:00 2001 From: Lars Vogel Date: Fri, 24 Apr 2026 11:38:27 +0200 Subject: [PATCH 3/4] Guard shutdown dialog opener against workbench teardown Review feedback: during postShutdown() the workbench may already be gone, so reaching through PlatformUI via ProgressManagerUtil.safeToOpen is unsafe. Scan display.getShells() for modal shells directly instead, rename the scheduled runnable to openDialogLater to make the schedule/cancel pairing obvious, and document the timerExec(-1, ...) cancel call. --- .../ide/application/IDEWorkbenchAdvisor.java | 21 ++++++++++++++----- 1 file changed, 16 insertions(+), 5 deletions(-) diff --git a/bundles/org.eclipse.ui.ide.application/src/org/eclipse/ui/internal/ide/application/IDEWorkbenchAdvisor.java b/bundles/org.eclipse.ui.ide.application/src/org/eclipse/ui/internal/ide/application/IDEWorkbenchAdvisor.java index 10b43ea702a..1cba5423e2d 100644 --- a/bundles/org.eclipse.ui.ide.application/src/org/eclipse/ui/internal/ide/application/IDEWorkbenchAdvisor.java +++ b/bundles/org.eclipse.ui.ide.application/src/org/eclipse/ui/internal/ide/application/IDEWorkbenchAdvisor.java @@ -61,6 +61,7 @@ import org.eclipse.swt.graphics.Resource; import org.eclipse.swt.widgets.Display; import org.eclipse.swt.widgets.Listener; +import org.eclipse.swt.widgets.Shell; import org.eclipse.ui.PlatformUI; import org.eclipse.ui.application.IWorkbenchConfigurer; import org.eclipse.ui.application.IWorkbenchWindowConfigurer; @@ -79,7 +80,6 @@ import org.eclipse.ui.internal.ide.IDEWorkbenchMessages; import org.eclipse.ui.internal.ide.IDEWorkbenchPlugin; import org.eclipse.ui.internal.ide.undo.WorkspaceUndoMonitor; -import org.eclipse.ui.internal.progress.ProgressManagerUtil; import org.eclipse.ui.internal.progress.ProgressMonitorJobsDialog; import org.eclipse.ui.progress.IProgressService; import org.eclipse.ui.statushandlers.AbstractStatusHandler; @@ -498,12 +498,12 @@ protected void disconnectFromWorkspace() { final Display display = Display.getCurrent(); final int longOperationTime = savedLongOperationTime; - final Runnable openDialog = () -> { - if (ProgressManagerUtil.safeToOpen(dialog, null)) { + final Runnable openDialogLater = () -> { + if (!display.isDisposed() && !hasModalShell(display)) { dialog.open(); } }; - display.timerExec(longOperationTime, openDialog); + display.timerExec(longOperationTime, openDialogLater); try { BusyIndicator.showWhile(display, () -> { @@ -518,7 +518,8 @@ protected void disconnectFromWorkspace() { } }); } finally { - display.timerExec(-1, openDialog); + // Cancel openDialogLater if it is still pending; no-op if it already ran. + display.timerExec(-1, openDialogLater); } if (!status.isOK()) { @@ -528,6 +529,16 @@ protected void disconnectFromWorkspace() { } } + private static boolean hasModalShell(Display display) { + final int modal = SWT.APPLICATION_MODAL | SWT.SYSTEM_MODAL | SWT.PRIMARY_MODAL; + for (Shell shell : display.getShells()) { + if (!shell.isDisposed() && shell.isVisible() && (shell.getStyle() & modal) != 0) { + return true; + } + } + return false; + } + @Override public IAdaptable getDefaultPageInput() { return ResourcesPlugin.getWorkspace().getRoot(); From 7c22ccba368f10faf6695d64e329e9e3fe940698 Mon Sep 17 00:00:00 2001 From: Lars Vogel Date: Mon, 27 Apr 2026 06:50:17 +0200 Subject: [PATCH 4/4] Derive slow-save sleep from getLongOperationTime() The shutdown test hard-coded a 1200ms sleep based on the current 800ms default of IProgressService#getLongOperationTime(). Read the live value in postStartup() and add a fixed margin so the test still exercises the delayed-open path if the platform default changes. --- .../ide/application/IDEWorkbenchAdvisorTest.java | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-) diff --git a/tests/org.eclipse.ui.ide.application.tests/src/org/eclipse/ui/internal/ide/application/IDEWorkbenchAdvisorTest.java b/tests/org.eclipse.ui.ide.application.tests/src/org/eclipse/ui/internal/ide/application/IDEWorkbenchAdvisorTest.java index ea7e37e1e07..1990ee437e4 100644 --- a/tests/org.eclipse.ui.ide.application.tests/src/org/eclipse/ui/internal/ide/application/IDEWorkbenchAdvisorTest.java +++ b/tests/org.eclipse.ui.ide.application.tests/src/org/eclipse/ui/internal/ide/application/IDEWorkbenchAdvisorTest.java @@ -20,6 +20,7 @@ import java.io.Closeable; import java.util.concurrent.atomic.AtomicInteger; +import java.util.concurrent.atomic.AtomicLong; import org.eclipse.core.resources.ISaveContext; import org.eclipse.core.resources.ISaveParticipant; @@ -212,14 +213,16 @@ public void postShutdown() { @Test public void testShutdownWithSlowSave() throws CoreException { try (SaveHook saveHook = new SaveHook()) { - // Exceeds the default IProgressService#getLongOperationTime (800ms), - // so the new code path schedules and opens the progress dialog. - final long sleepMillis = 1200L; + // Sleep duration is derived from the live IProgressService#getLongOperationTime() + // value in postStartup(), so the test exercises the delayed-open path even if + // the platform default ever changes. + final long sleepMarginMillis = 500L; + final AtomicLong sleepMillis = new AtomicLong(); workspace.addSaveParticipant(PLUGIN_ID + ".slow", new ISaveParticipant() { @Override public void saving(ISaveContext context) { try { - Thread.sleep(sleepMillis); + Thread.sleep(sleepMillis.get()); } catch (InterruptedException e) { Thread.currentThread().interrupt(); } @@ -242,6 +245,9 @@ public void rollback(ISaveContext context) { @Override public void postStartup() { super.postStartup(); + long longOperationTime = PlatformUI.getWorkbench().getProgressService() + .getLongOperationTime(); + sleepMillis.set(longOperationTime + sleepMarginMillis); display.asyncExec(() -> PlatformUI.getWorkbench().close()); } };