Don't use display thread for scheduling console update tasks by iloveeclipse · Pull Request #2461 · eclipse-platform/eclipse.platform

iloveeclipse · 2026-02-20T17:34:51Z

This fixes regression from 7e1f60c, where main thread (which owns lock on UI operations) waits for the lock on ProcessConsole to update console name but never gets it because console code is blocked internally waiting for a QueueProcessingJob being executed on UI thread.

The solution is to avoid direct calls from UI to ProcessConsole.resetName().

Fixes #2460

Copilot

Pull request overview

This PR fixes a deadlock issue (#2460) that occurred when terminating launches. The deadlock happened because the UI thread would wait for a lock on ProcessConsole to update the console name, while console code was blocked waiting for a QueueProcessingJob to execute on the same UI thread. The fix introduces a separate ScheduledExecutorService to handle console name updates asynchronously, avoiding direct calls from the UI thread to ProcessConsole.resetName().

Changes:

Replaced UI thread timer-based scheduling (Display.timerExec) with a dedicated ScheduledExecutorService for console name updates
Added proper executor cleanup in the dispose() method
Introduced tracking of pending name update tasks to avoid duplicate scheduling

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-02-20T17:38:47Z

debug/org.eclipse.debug.ui/ui/org/eclipse/debug/internal/ui/views/console/ProcessConsole.java

+							currentPending = consoleNameUpdateExecutor.schedule(() -> {
+								pendingNameUpdate = null;
+								resetName(false);
+							}, 1, TimeUnit.SECONDS);
+						}


The assignment currentPending = consoleNameUpdateExecutor.schedule(...) on line 366 updates the local variable but does not update the instance field pendingNameUpdate. This means the check on line 365 (currentPending == null || currentPending.isDone()) will always evaluate to true after the first scheduling, potentially causing multiple concurrent scheduled tasks instead of preventing them. The result of schedule() should be assigned to pendingNameUpdate to properly track the scheduled future.

Copilot · 2026-02-20T17:38:48Z

debug/org.eclipse.debug.ui/ui/org/eclipse/debug/internal/ui/views/console/ProcessConsole.java

+						ScheduledFuture<?> currentPending = pendingNameUpdate;
+						if (currentPending == null || currentPending.isDone()) {
+							currentPending = consoleNameUpdateExecutor.schedule(() -> {
+								pendingNameUpdate = null;
+								resetName(false);
+							}, 1, TimeUnit.SECONDS);


The read of pendingNameUpdate on line 364 and the potential write on line 367 are not properly synchronized, which can lead to race conditions in a multi-threaded environment. Multiple threads calling computeName() concurrently could read pendingNameUpdate as null simultaneously, causing multiple tasks to be scheduled despite the intent to schedule only one. Consider using AtomicReference for pendingNameUpdate or adding proper synchronization around this check-and-set pattern.

Suggested change

ScheduledFuture<?> currentPending = pendingNameUpdate;

if (currentPending == null || currentPending.isDone()) {

currentPending = consoleNameUpdateExecutor.schedule(() -> {

pendingNameUpdate = null;

resetName(false);

}, 1, TimeUnit.SECONDS);

synchronized (this) {

ScheduledFuture<?> currentPending = pendingNameUpdate;

if (currentPending == null || currentPending.isDone()) {

pendingNameUpdate = consoleNameUpdateExecutor.schedule(() -> {

synchronized (ProcessConsole.this) {

pendingNameUpdate = null;

resetName(false);

}

}, 1, TimeUnit.SECONDS);

}

Copilot · 2026-02-20T17:38:48Z

debug/org.eclipse.debug.ui/ui/org/eclipse/debug/internal/ui/views/console/ProcessConsole.java

@@ -531,6 +547,7 @@ public IProcess getProcess() {
 	protected void dispose() {
 		super.dispose();
 		fColorProvider.disconnect();


When the console is disposed, shutdownNow() is called on line 550, which will attempt to stop all actively executing tasks. However, if a scheduled task is in the middle of calling resetName(false), it could encounter errors since the console may be in an inconsistent state during disposal. Consider canceling the pendingNameUpdate task explicitly before calling shutdownNow(), or check if the console is being disposed at the start of the scheduled task.

Suggested change

fColorProvider.disconnect();

fColorProvider.disconnect();

if (pendingNameUpdate != null) {

pendingNameUpdate.cancel(false);

}

iloveeclipse · 2026-02-20T17:40:52Z

@trancexpress : if you can reproduce, please try this PR.

Beside this, I wonder if this task that updates console every second must be always executed, even if console is hidden behind other one. This is definitely a resource waste. Start ~6 processes with Console and enjoy that every 100 ms some task is executed updating some of the consoles.

This makes no sense. So probably the code has to be rewritten to avoid parallel updates of hidden console pages.

trancexpress · 2026-02-20T17:42:30Z

I wasn't able to reproduce the deadlock when I tried. Likely it needs stepping with breakpoints, I've not checked how to do that.

github-actions · 2026-02-20T18:07:30Z

Test Results

1 977 files ±0 1 977 suites ±0 1h 31m 1s ⏱️ -21s
4 743 tests ±0 4 719 ✅ ±0 24 💤 ±0 0 ❌ ±0
14 229 runs ±0 14 047 ✅ ±0 182 💤 ±0 0 ❌ ±0

Results for commit 36cf40b. ± Comparison against base commit 2a3becb.

♻️ This comment has been updated with latest results.

- Extracted common code from ShowConsoleViewJob to AbstractConsoleJob - Renamed RepaintJob to RedrawJob, switched to use AbstractConsoleJob - Extracted anonymous UI job to WarnAboutContentChangedJob, based on AbstractConsoleJob - fWarnQueued is not needed anymore for WarnAboutContentChangedJob - This above fixes the race condition where multiple consoles changes could have triggered the warnOfContentChange(), but the job only scheduled for the first one - Moved final field inits to constructor See eclipse-platform#2477

`page.findView(IConsoleConstants.ID_CONSOLE_VIEW)` API doesn't return console views opened in same page with secondary id's (== any Console views opened next to the first one in same page). So all calls to `ConsoleManager.refresh(IConsole)` were not working for such views. In SDK, this would only affect `TextConsole.addHyperlink()` code and is visible if opening Java Stack Trace view as secondary view - it will not show hyperlinks. Worse, since view id's are persisted across sessions, such views would not work properly as long as they present in the perspective. For the fix, make use of views registered in ConsoleManager and iterate over them, similar to ShowConsoleViewJob code. See eclipse-platform#2477

Original code had multiple issues: it didn't notified about console content changes in all opened windows, it didn't notified for the console changes in all secondary console views but it notified even if the console with changes was visible. As noticed already before, `page.findView(IConsoleConstants.ID_CONSOLE_VIEW)` API doesn't return console views opened in same page with secondary id's (== any Console views opened next to the first one in same page). So WarnAboutContentChangedJob only supported Console views without secondary ids and only in the currently active window. - Notify console changes in all windows, not only in the active one - Notify also for secondary consoles - Don't notify about console changes if changed console page is visible (== top of the stack) in the current Console view and Console view is also visible. Fixes eclipse-platform#2477

- Moved final field inits to constructor - Get rid of unneeded or duplicated code, obsoleted comments - Use instanceof variables - Fix spelling mistakes - Changed all access to fActiveConsole via get/set methods

- Moved final field inits to constructor - Added trivial toString() implementation to the AbstractConsole

- Moved final field inits to constructor - Removed useless javadocs - Use instanceof variables where possible - Added "disposed" field and checks before each asyncExec() for that - Refactored computeName() to readable code -- split into multiple logical parts -- made things static which are static -- don't do anything if view is already disposed

This fixes regression from 7e1f60c, where main thread (which owns lock on UI operations) waits for the lock on ProcessConsole to update console name but never gets it because console code is blocked internally waiting for a QueueProcessingJob being executed on UI thread. The solution is to avoid direct calls from UI to ProcessConsole.resetName(). Fixes eclipse-platform#2460

- Added new API to allow console pages knew whether they are visible or not. - Use new IConsole API to schedule console updates only if console page is visible (top level) in the console view. This prevents multiple opened consoles in same console view run update tasks every second, even if no one can see the updated console name. Fixes eclipse-platform#2478

Copilot

Pull request overview

Copilot reviewed 9 out of 9 changed files in this pull request and generated 6 comments.

Comments suppressed due to low confidence (1)

debug/org.eclipse.ui.console/src/org/eclipse/ui/console/IConsole.java:127

The documentation contains a grammatical error: "is not more visible" should be "is no longer visible".

	 * this console is not more visible to user.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-02-25T17:06:03Z

debug/org.eclipse.ui.console/src/org/eclipse/ui/internal/console/ConsoleView.java

+		if (isThisPart(partRef)) {
+			IConsole console = getConsole();
+			if (console != null) {
+				console.pageShown();


There's an inconsistency in how pageShown() and consoleHidden() are called. When a view becomes visible (line 840), console.pageShown() is called directly without going through ConsoleManager.consoleShown(). However, when a view is hidden (line 830), it calls getConsoleManager().consoleHidden(console). This asymmetry could lead to incorrect visibility tracking, as the ConsoleManager's isConsoleVisibleSomewhere() check is bypassed for the pageShown case. Consider calling getConsoleManager().consoleShown(console) at line 840 for consistency.

Suggested change

console.pageShown();

getConsoleManager().consoleShown(console);

Copilot · 2026-02-25T17:06:03Z

debug/org.eclipse.ui.console/src/org/eclipse/ui/internal/console/ConsoleView.java

 			}
 			if (recConsole != null) {
 				activateParticipants(recConsole);
+				recConsole.pageShown();


Similar to the partVisible/partHidden inconsistency, when switching between consoles in showPageRec, line 205 calls recConsole.pageShown() directly instead of going through getConsoleManager().consoleShown(recConsole). This bypasses the isConsoleVisibleSomewhere() check that prevents duplicate notifications when the same console is visible in multiple views. For consistency with line 201, consider using the ConsoleManager method here as well.

Suggested change

recConsole.pageShown();

getConsoleManager().consoleShown(recConsole);

debug/org.eclipse.ui.console/src/org/eclipse/ui/internal/console/ConsoleManager.java

Copilot · 2026-02-25T17:06:04Z

debug/org.eclipse.ui.console/src/org/eclipse/ui/internal/console/ConsoleManager.java

+						Control control = view.getCurrentPage().getControl();
+						if (!control.isDisposed()) {


Potential NullPointerException: view.getCurrentPage() could return null, and calling getControl() on null would cause an NPE. Add a null check before accessing the control.

Suggested change

Control control = view.getCurrentPage().getControl();

if (!control.isDisposed()) {

Control control = null;

if (view.getCurrentPage() != null) {

control = view.getCurrentPage().getControl();

}

if (control != null && !control.isDisposed()) {

Copilot · 2026-02-25T17:06:04Z

debug/org.eclipse.debug.ui/ui/org/eclipse/debug/internal/ui/views/console/ProcessConsole.java

+		synchronized (consoleNameUpdateExecutor) {
+			if (updateRunning) {
+				return;
+			}
+			updateRunning = true;
+			consoleNameUpdateExecutor.submit(() -> {
+				while (canUpdateConsoleName()) {
+					try {
+						Thread.sleep(1000);
+					} catch (InterruptedException e) {
+						// ignore and continue to update name
+					}
+					if (!canUpdateConsoleName()) {
+						break;
+					}
+					showName(false, computeName(false));
+				}
+				updateRunning = false;
+			});
+		}


The synchronization on consoleNameUpdateExecutor is incorrect. The updateRunning flag is being accessed outside the synchronized block (line 443) without proper synchronization, which could lead to a race condition. Additionally, synchronizing on the ExecutorService itself is not recommended practice. Consider using a dedicated lock object or making updateRunning atomic.

Copilot · 2026-02-25T17:06:05Z

debug/org.eclipse.debug.ui/ui/org/eclipse/debug/internal/ui/views/console/ProcessConsole.java

 	protected void dispose() {
 		super.dispose();
+		consoleNameUpdateExecutor.shutdownNow();
 		fColorProvider.disconnect();
 		DebugPlugin.getDefault().removeDebugEventListener(this);
 		DebugUIPlugin.getDefault().getPreferenceStore().removePropertyChangeListener(this);
 		JFaceResources.getFontRegistry().removeListener(this);
 		closeStreams();
 		disposeStreams();
+		disposed = true;
 	}


The disposed flag is set to true (line 599) after shutting down the executor (line 592), but the executor's task checks disposed asynchronously. This creates a race where the executor could be shut down while the task is checking canUpdateConsoleName() which checks disposed. Consider setting disposed = true before calling shutdownNow() to ensure the background task stops checking conditions before the executor is terminated.

iloveeclipse requested a review from Copilot February 20, 2026 17:35

Copilot started reviewing on behalf of iloveeclipse February 20, 2026 17:35 View session

Copilot AI reviewed Feb 20, 2026

View reviewed changes

iloveeclipse added 8 commits February 25, 2026 14:05

Manual code cleanup ConsoleView

955d2b8

- Moved final field inits to constructor - Get rid of unneeded or duplicated code, obsoleted comments - Use instanceof variables - Fix spelling mistakes - Changed all access to fActiveConsole via get/set methods

Manual code cleanup for base Console classes

5dab2a3

- Moved final field inits to constructor - Added trivial toString() implementation to the AbstractConsole

iloveeclipse force-pushed the issue_2460 branch from 16c60c8 to 36cf40b Compare February 25, 2026 17:00

iloveeclipse requested a review from Copilot February 25, 2026 17:00

Copilot started reviewing on behalf of iloveeclipse February 25, 2026 17:01 View session

This was linked to issues Feb 25, 2026

Hidden consoles trigger useless UI updates every second #2478

Open

ConsoleManager doesn't properly support multiple Console views #2477

Open

Copilot AI reviewed Feb 25, 2026

View reviewed changes

	console.pageShown();
	getConsoleManager().consoleShown(console);

	recConsole.pageShown();
	getConsoleManager().consoleShown(recConsole);

		Control control = view.getCurrentPage().getControl();
		if (!control.isDisposed()) {

Conversation

iloveeclipse commented Feb 20, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI Feb 20, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 20, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 20, 2026

Choose a reason for hiding this comment

Uh oh!

iloveeclipse commented Feb 20, 2026

Uh oh!

trancexpress commented Feb 20, 2026

Uh oh!

github-actions bot commented Feb 20, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Test Results

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI Feb 25, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 25, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Copilot AI Feb 25, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 25, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 25, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

github-actions bot commented Feb 20, 2026 •

edited

Loading