Console view bug fixes and improvements

[iloveeclipse]

Console View Bug Fixes and Improvements

Summary

This PR addresses multiple issues in the Eclipse Console infrastructure, including a deadlock regression, broken behavior with secondary console views, incorrect content change notifications, and unnecessary performance overhead from hidden console updates. It also includes code cleanup across related classes.

Issues Fixed

• Fixes Deadlock on terminating launch #2460
• Fixes ConsoleManager doesn't properly support multiple Console views #2477
• Fixes Hidden consoles trigger useless UI updates every second #2478

Changes

Bug Fixes

Don't use display thread for scheduling console update tasks (280174fe)

Fixes a regression from 7e1f60c where the main thread (holding the UI operations lock) waits for the lock on ProcessConsole to update the console name but never gets it, because the console code is blocked internally waiting for a QueueProcessingJob being executed on the UI thread. The fix avoids direct calls from the UI thread to ProcessConsole.resetName().

Fix link redraw issues with secondary consoles (4477d7ed)

page.findView(IConsoleConstants.ID_CONSOLE_VIEW) does not return console views opened in the same page with secondary IDs (i.e., any Console views opened alongside the first one). This caused all calls to ConsoleManager.refresh(IConsole) to silently fail for such views. In the SDK, this broke TextConsole.addHyperlink() — visible when opening the Java Stack Trace view as a secondary view (no hyperlinks shown). Since view IDs are persisted across sessions, these views remained broken indefinitely. The fix iterates over views registered in ConsoleManager, similar to ShowConsoleViewJob.

Fix console content notifications (e22266f1)

The original notification code had multiple issues:

• It only notified about console content changes in the active window, not all opened windows.
• It did not notify for consoles in secondary console views.
• It notified even when the changed console was already visible.

The fix ensures notifications are sent to all windows and secondary console views, while suppressing them when the changed console page is already visible (top of the stack) and the Console view itself is visible.

Don't update consoles every second if not needed (8db7286)

• Added new IConsole API to allow console pages to know whether they are visible or not. Console name updates are now scheduled only when the console page is visible (top level) in the console view. This prevents multiple opened consoles in the same view from running update tasks every second when no one can see them.
• Don't update console if the elapsed time format is set to None.
• Update console elapsed time format for running processes if preference
  changes.

Code Cleanup & Refactoring

Manual code cleanup & fixes in ConsoleManager (1de591c6)

• Extracted common code from ShowConsoleViewJob into AbstractConsoleJob
• Renamed RepaintJob to RedrawJob, switched to use AbstractConsoleJob
• Extracted anonymous UI job to WarnAboutContentChangedJob, based on AbstractConsoleJob
• Removed unnecessary fWarnQueued field — fixes a race condition where multiple console changes could trigger warnOfContentChange() but the job only scheduled for the first one
• Moved final field initializations to constructor
• Fixed potential NullPointerException in RedrawJob and ShowConsoleViewJob

Manual code cleanup ConsoleView (46309240)

• Moved final field initializations to constructor
• Removed unneeded/duplicated code and obsolete comments
• Use instanceof pattern variables
• Fixed spelling mistakes
• Changed all access to fActiveConsole via getter/setter methods

Manual code cleanup for base Console classes (4accaf99)

• Moved final field initializations to constructor
• Added trivial toString() implementation to AbstractConsole
• Fixed possible NullPointerException in IOConsole

ProcessConsole.computeName() refactoring and smaller fixes (174da5c0)

• Moved final field initializations to constructor
• Removed useless Javadocs
• Use instanceof pattern variables where possible
• Added disposed field and checks before each asyncExec() call
• Refactored computeName() into readable code:
  • Split into multiple logical parts
  • Made static methods where applicable
  • Skip execution if view is already disposed
• Use imports in ProcessConsoleTests

New API

• IConsole.pageShown() IConsole.pageHidden() — Allows console pages to be notified about their visibility state
• IConsoleManager.consoleShown() IConsoleManager.consoleHidden() — New API additions for managing console visibility state and notifications

Testing

• New test class ConsoleShowHideTests with comprehensive tests for console visibility behavior
• Updated ConsoleManagerTests and ProcessConsoleTests with additional test coverage

[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.

[iloveeclipse]

@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]

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]

Test Results

 1 980 files  +3   1 980 suites  +3   1h 32m 58s ⏱️ -27s
 4 746 tests +3   4 722 ✅ +3   24 💤 ±0  0 ❌ ±0 
14 238 runs  +9  14 056 ✅ +9  182 💤 ±0  0 ❌ ±0 

Results for commit 8db7286. ± Comparison against base commit 3638d7f.

♻️ This comment has been updated with latest results.

[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]

Pull request overview

Copilot reviewed 9 out of 9 changed files in this pull request and generated 1 comment.

---

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

[Copilot]

Pull request overview

Copilot reviewed 18 out of 18 changed files in this pull request and generated 5 comments.

Comments suppressed due to low confidence (1)

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

• The executor thread is created with a fixed name ("Console name updater"). If multiple process consoles are active, thread dumps will show many identical thread names, making diagnostics harder. Consider including a unique suffix (e.g., process id/label or System.identityHashCode(this)) in the thread name.

		consoleNameUpdateExecutor = Executors.newSingleThreadExecutor(r -> {
			Thread t = new Thread(r, "Console name updater"); //$NON-NLS-1$
			t.setDaemon(true);
			return t;

---

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

[Copilot]

Pull request overview

Copilot reviewed 18 out of 18 changed files in this pull request and generated no new comments.

Comments suppressed due to low confidence (3)

debug/org.eclipse.debug.tests/src/org/eclipse/debug/tests/console/ProcessConsoleTests.java:702

• The assertion failure message says "minimized" but this block hides the view via activePage.hideView(consoleView). Consider updating the message text to reflect the actual action (hidden vs minimized) to make test failures easier to interpret.

				activePage.hideView(consoleView);

				// Wait >1 second for update
				TestUtil.processUIEvents(2000);
				assertEquals(console1NameAfterReshow, console1.getName(), "Console name should not update when console view is minimized");
				assertEquals(console2NameAfterReshow, console2.getName(), "Console name should not update when console view is minimized");

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

• pageHidden() calls computeName(false) but ignores the return value, and computeName(false) has no side effects when triggerAsyncUpdate is false. This makes the call a no-op aside from extra work; consider removing it (visibility is already tracked via isVisible = false, which stops the updater via canUpdateConsoleName()).

	@Override
	public void pageHidden() {
		isVisible = false;
		computeName(false);
	}

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

• Javadoc: "not more visible" is grammatically incorrect/unclear. Consider changing to "no longer visible" (or similar) to make the API contract easier to understand for implementers.

	 * <p>
	 * Subclasses may override this method to be notified when the console page for
	 * this console is not more visible to user.
	 * </p>

---

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

[Copilot]

Pull request overview

Copilot reviewed 18 out of 18 changed files in this pull request and generated 2 comments.

Comments suppressed due to low confidence (2)

debug/org.eclipse.debug.tests/src/org/eclipse/debug/tests/console/ConsoleShowHideTests.java:65

• Use IConsoleConstants.ID_CONSOLE_VIEW instead of the hard-coded view id string ("org.eclipse.ui.console.ConsoleView") to avoid drift if the id ever changes and to match usage elsewhere in the test suite.

			consoles[i] = new ConsoleMock(i + 1);
		}
		consoleView = (ConsoleView) activePage.showView("org.eclipse.ui.console.ConsoleView");
		activePage.activate(consoleView);
		TestUtil.processUIEvents(100);

debug/org.eclipse.debug.tests/src/org/eclipse/debug/tests/console/ConsoleMock.java:92

• Spelling/grammar: "This listener is get called" should be "This listener is called".

				// This listener is get called if the page is really shown
				// in the console view
				getControl().addListener(SWT.Show, event -> {
					int count = showCalled.incrementAndGet();
					if (count == 1) {

---

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

[Copilot]

Pull request overview

Copilot reviewed 18 out of 18 changed files in this pull request and generated no new comments.

---

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