IProgressService.run(fork, cancelable, runnable) behaves inconsistently #4202

I noticed an inconsistent behavior while working on #4086.

• I verified I can reproduce this issue against latest Integration Build of Eclipse SDK

Summary

org.eclipse.ui.progress.IProgressService.run(boolean fork, boolean cancelable, IRunnableWithProgress runnable) does not consistently honor the "Always run in background" preference (Window/Preferences > General). Depending on the fork/cancelable combination passed in, the preference is silently ignored and a modal progress dialog pops up anyway — even though the user explicitly asked for operations to run in the background.

There are other confusing (but arguably "working as documented") behaviors around thread-affinity, the fork=false contract and the blocking nature of run(...) that make the overall API very easy to misuse. A demo view (ProgressServiceView, see below) makes all of this directly observable/reproducible.

A screen recording demonstrating the bug and the fix using this view is attached to this issue.

Environment

• Eclipse Platform UI (org.eclipse.ui.workbench), class org.eclipse.ui.internal.progress.ProgressManager
• Reproduced via a new example view: org.eclipse.ui.examples.jobs.views.ProgressServiceView (See linked PR: Fix/progress service run #4203)

Root cause

ProgressManager.run(...) branches on !fork || !cancelable to decide how to run the operation:

• When fork == false or cancelable == false, ProgressManager.run(...) itself correctly calls shouldRunInBackground() and, when the preference is enabled, runs the ProgressMonitorJobsDialog without opening it.
• When fork == true && cancelable == true (the most common call shape — e.g. simply run(true, true, runnable)), it delegates to busyCursorWhile(runnable), which never checks the preference and always schedules the dialog-opening job.

Net effect: with "Always run in background" enabled, run(true, true, runnable) still shows a modal progress dialog. The preference is only honored for some fork/cancelable combinations, not all of them, even though nothing in the IProgressService or IRunnableContext javadoc suggests the preference should only apply to some combinations.

A related bug in the same branch: when fork == false or cancelable == false and the preference is not enabled, the dialog's openOnRun flag is left at its default (true). The dialog is therefore opened immediately, ignoring the 800ms delay of IProgressService#getLongOperationTime(). #4203 fixes this too.

Reproduction steps (via ProgressServiceView)

NOTE: ProgressServiceView is added in the linked PR: #4203

1. Open ProgressServiceView (Window > Show View > Other... > Progress Examples > Progress Service).
2. Check "Always run in background" (or set it under Preferences > General — the view's checkbox stays in sync either way).
3. Check fork, cancelable and call from UI thread, set a duration long enough to exceed the long-operation delay of 800 ms (e.g. 3000 ms), click Run.
4. Observed (buggy) behavior: a modal progress dialog appears anyway, blocking interaction with the rest of the workbench, even though the user asked for background execution.
5. Expected behavior: no dialog should appear; the UI shouldn't freeze, showing the operation ran in the background exactly like the other fork/cancelable combinations already do correctly.

Other confusing (non-bug, but easy-to-misuse) behaviors surfaced by the same view

These match documented contracts, but they explain why this API is easy to get wrong in practice. All of them can be reproduced in ProgressServiceView by combining the fork, cancelable, call from UI thread and pump UI events while running checkboxes.

Calling IProgressService.run(...) from a non-UI thread

This is not supported, whatever fork, cancelable or the "Always run in background" preference are set to. The class javadoc of IProgressService states:

All methods on the progress service must be called from the UI thread.

Before #4203 the call failed with a generic SWTException ("Invalid thread access"). It was thrown by Display.getShells() while the parent shell of the progress dialog was being computed, and gave no hint about why the call isn't allowed. With #4203, ProgressManager.run(...) catches that SWTException and throws an IllegalStateException instead. The new exception explains the restriction and points to Jobs for running long operations from a background thread. The original SWTException is kept as its cause.

Uncheck call from UI thread in the view to reproduce this. The exception is logged to the Error Log.

run(false, cancelable, runnable) blocks the UI thread

With fork == false the runnable runs synchronously in the calling thread, which (see above) is always the UI thread. IRunnableContext#run only says:

If fork is false, the current thread is used to run the runnable.

The responsibility to keep the UI responsive is documented on the ProgressMonitorDialog implementation, which ProgressManager delegates to for every fork/cancelable combination:

• ProgressMonitorDialog#run:

  It is recommended that fork is set to true in most cases. If fork is set to false, the runnable will run in the UI thread and it is the runnable's responsibility to call Display.readAndDispatch() to ensure UI responsiveness.

• The class javadoc of ProgressMonitorDialog:

  Note that not forking the process will result in it running in the UI which may starve the UI. [...] It is recommended that fork is set to true in most cases.

ProgressManager doesn't open the dialog right away. It schedules a job that opens it after IProgressService#getLongOperationTime() (800ms) and calls setOpenOnRun(false) on the dialog. ProgressMonitorDialog#setOpenOnRun warns about exactly this situation:

NOTE: Setting this to false and not forking a process may starve any asyncExec that tries to open the dialog later.

So with fork == false:

• if the runnable does not pump events, the UI freezes and no progress dialog is shown at all, however long the operation takes;
• if the runnable does pump events (Display.readAndDispatch()), the UI stays responsive and the dialog appears after the 800ms delay (unless "Always run in background" is set).

The freeze is the caller's responsibility, not something the framework can fix on the callee side. Check/uncheck pump UI events while running (with fork unchecked) in the view to compare both cases.

run(true, cancelable, runnable) always blocks the caller

IProgressService#run says that it:

might run the runnable asynchronously if fork is true.

In practice, the workbench implementation (ProgressManager) always blocks the calling (UI) thread until the runnable has finished, for every fork/cancelable combination. While it runs, the UI stays responsive because the event loop keeps running inside the call. This is consistent with ProgressMonitorDialog#run ("blocks until the runnable has been run, regardless of the value of fork") and IProgressService#busyCursorWhile ("The calling thread will be blocked for the duration of the execution of the supplied runnable"). Callers should not rely on run(true, ...) returning before the operation has finished, and they also don't need to guard against it.

Community

• I understand reporting an issue to this OSS project does not mandate anyone to fix it. Other contributors may consider the issue, or not, at their own convenience. The most efficient way to get it fixed is that I fix it myself and contribute it back as a good quality patch to the project.