From 1cfa38948ccbdac251140a8f498eda53c2542c26 Mon Sep 17 00:00:00 2001 From: Alvaro Sanchez-Leon Date: Mon, 3 Oct 2016 13:16:55 -0400 Subject: [PATCH] Bug 303808: Share GDB process streams among console pages The GDB process streams now stay opened regardless of the life-cycle of associated console views. This allows to close the Debugger Console view without affecting the debugging session. At the same time, closing and re-opening the Debugger Console causes new console pages to be created, but should not cause multiple jobs to read from the same input stream. Change-Id: Ief78aa2053e5a54514773a8f24f0a465364a7351 Signed-off-by: Alvaro Sanchez-Leon Signed-off-by: Marc Khouzam --- .../ui/console/GdbCliConsoleManager.java | 2 +- .../ui/console/GdbFullCliConsole.java | 116 +++++++++++++++++- .../ui/console/GdbFullCliConsolePage.java | 87 ++++--------- ...tor.java => GdbTerminalPageConnector.java} | 59 ++------- .../console/IGdbTerminalControlConnector.java | 37 ++++++ 5 files changed, 183 insertions(+), 118 deletions(-) rename dsf-gdb/org.eclipse.cdt.dsf.gdb.ui/src/org/eclipse/cdt/dsf/gdb/internal/ui/console/{GdbTerminalConnector.java => GdbTerminalPageConnector.java} (63%) create mode 100644 dsf-gdb/org.eclipse.cdt.dsf.gdb.ui/src/org/eclipse/cdt/dsf/gdb/internal/ui/console/IGdbTerminalControlConnector.java diff --git a/dsf-gdb/org.eclipse.cdt.dsf.gdb.ui/src/org/eclipse/cdt/dsf/gdb/internal/ui/console/GdbCliConsoleManager.java b/dsf-gdb/org.eclipse.cdt.dsf.gdb.ui/src/org/eclipse/cdt/dsf/gdb/internal/ui/console/GdbCliConsoleManager.java index bef81c92448..49e9a048a1f 100644 --- a/dsf-gdb/org.eclipse.cdt.dsf.gdb.ui/src/org/eclipse/cdt/dsf/gdb/internal/ui/console/GdbCliConsoleManager.java +++ b/dsf-gdb/org.eclipse.cdt.dsf.gdb.ui/src/org/eclipse/cdt/dsf/gdb/internal/ui/console/GdbCliConsoleManager.java @@ -187,7 +187,7 @@ public class GdbCliConsoleManager implements ILaunchesListener2 { IDebuggerConsole console; if (backend != null && backend.isFullGdbConsoleSupported()) { // Create a full GDB cli console - console = new GdbFullCliConsole(fLaunch, consoleTitle); + console = new GdbFullCliConsole(fLaunch, consoleTitle, backend.getProcess(), backend.getProcessPty()); } else if (control != null) { // Create a simple text console for the cli. console = new GdbBasicCliConsole(fLaunch, consoleTitle, control.getGDBBackendProcess()); diff --git a/dsf-gdb/org.eclipse.cdt.dsf.gdb.ui/src/org/eclipse/cdt/dsf/gdb/internal/ui/console/GdbFullCliConsole.java b/dsf-gdb/org.eclipse.cdt.dsf.gdb.ui/src/org/eclipse/cdt/dsf/gdb/internal/ui/console/GdbFullCliConsole.java index 315080bbad2..881bfa3635b 100644 --- a/dsf-gdb/org.eclipse.cdt.dsf.gdb.ui/src/org/eclipse/cdt/dsf/gdb/internal/ui/console/GdbFullCliConsole.java +++ b/dsf-gdb/org.eclipse.cdt.dsf.gdb.ui/src/org/eclipse/cdt/dsf/gdb/internal/ui/console/GdbFullCliConsole.java @@ -7,12 +7,24 @@ *******************************************************************************/ package org.eclipse.cdt.dsf.gdb.internal.ui.console; +import java.io.IOException; +import java.io.InputStream; +import java.io.OutputStream; +import java.util.HashSet; +import java.util.Set; + import org.eclipse.cdt.debug.ui.debuggerconsole.IDebuggerConsole; import org.eclipse.cdt.debug.ui.debuggerconsole.IDebuggerConsoleView; +import org.eclipse.cdt.utils.pty.PTY; import org.eclipse.core.runtime.CoreException; +import org.eclipse.core.runtime.IProgressMonitor; +import org.eclipse.core.runtime.IStatus; +import org.eclipse.core.runtime.Status; +import org.eclipse.core.runtime.jobs.Job; import org.eclipse.debug.core.ILaunch; import org.eclipse.debug.core.ILaunchConfiguration; import org.eclipse.debug.ui.DebugUITools; +import org.eclipse.tm.internal.terminal.provisional.api.ITerminalControl; import org.eclipse.ui.PlatformUI; import org.eclipse.ui.console.AbstractConsole; import org.eclipse.ui.console.IConsoleView; @@ -27,19 +39,113 @@ import org.eclipse.ui.part.IPageBookViewPage; public class GdbFullCliConsole extends AbstractConsole implements IDebuggerConsole, IGdbCliConsole { private final ILaunch fLaunch; private final String fLabel; - private GdbFullCliConsolePage fConsolePage; + private final PTY fGdbPty; - public GdbFullCliConsole(ILaunch launch, String label) { + private GdbFullCliConsolePage fConsolePage; + private final GdbTerminalConnector fTerminalConnector; + + public GdbFullCliConsole(ILaunch launch, String label, Process process, PTY pty) { super(label, null, false); fLaunch = launch; fLabel = label; + fGdbPty = pty; // Create a lifecycle listener to call init() and dispose() new GdbConsoleLifecycleListener(this); + fTerminalConnector = new GdbTerminalConnector(process); resetName(); } + @Override + protected void dispose() { + fTerminalConnector.dispose(); + super.dispose(); + } + + /** + * This class will read from the GDB process output and error streams and will + * write it to any registered ITerminalControl. + * It must continue reading from the streams, even if there are no ITerminalControl + * to write to. This is important to prevent GDB's output buffer from getting full + * and then completely stopping. + */ + private final class GdbTerminalConnector implements IGdbTerminalControlConnector { + private final Set fTerminalPageControls = new HashSet<>(); + private final Process fProcess; + private final Job fOutputStreamJob; + private final Job fErrorStreamJob; + + public GdbTerminalConnector(Process process) { + fProcess = process; + + fOutputStreamJob = new OutputReadJob(process.getInputStream()); + fOutputStreamJob.schedule(); + fErrorStreamJob = new OutputReadJob(process.getErrorStream()); + fErrorStreamJob.schedule(); + } + + public void dispose() { + fOutputStreamJob.cancel(); + fErrorStreamJob.cancel(); + } + + @Override + public void addPageTerminalControl(ITerminalControl terminalControl) { + fTerminalPageControls.add(terminalControl); + } + + @Override + public void removePageTerminalControl(ITerminalControl terminalControl) { + if (terminalControl != null) { + fTerminalPageControls.remove(terminalControl); + } + } + + @Override + public OutputStream getTerminalToRemoteStream() { + // When the user writes to the terminal, it should be sent + // directly to GDB + return fProcess.getOutputStream(); + } + + private class OutputReadJob extends Job { + { + setSystem(true); + } + + private InputStream fInputStream; + + private OutputReadJob(InputStream inputStream) { + super("GDB CLI output Job"); //$NON-NLS-1$ + fInputStream = inputStream; + } + + @Override + protected IStatus run(IProgressMonitor monitor) { + try { + byte[] b = new byte[1024]; + int read = 0; + do { + if (monitor.isCanceled()) { + break; + } + + read = fInputStream.read(b); + if (read > 0) { + for (ITerminalControl control : fTerminalPageControls) { + control.getRemoteToTerminalOutputStream().write(b, 0, read); + } + } + } while (read >= 0); + } catch (IOException e) { + } + + return Status.OK_STATUS; + } + } + } + @Override public ILaunch getLaunch() { return fLaunch; } @@ -93,7 +199,7 @@ public class GdbFullCliConsole extends AbstractConsole implements IDebuggerConso @Override public IPageBookViewPage createDebuggerPage(IDebuggerConsoleView view) { view.setFocus(); - fConsolePage = new GdbFullCliConsolePage(this, view); + fConsolePage = new GdbFullCliConsolePage(this, view, fGdbPty); return fConsolePage; } @@ -103,4 +209,8 @@ public class GdbFullCliConsole extends AbstractConsole implements IDebuggerConso fConsolePage.setInvertedColors(enable); } } + + public IGdbTerminalControlConnector getTerminalControlConnector() { + return fTerminalConnector; + } } diff --git a/dsf-gdb/org.eclipse.cdt.dsf.gdb.ui/src/org/eclipse/cdt/dsf/gdb/internal/ui/console/GdbFullCliConsolePage.java b/dsf-gdb/org.eclipse.cdt.dsf.gdb.ui/src/org/eclipse/cdt/dsf/gdb/internal/ui/console/GdbFullCliConsolePage.java index 5cb28d53085..c0d3f8214b7 100644 --- a/dsf-gdb/org.eclipse.cdt.dsf.gdb.ui/src/org/eclipse/cdt/dsf/gdb/internal/ui/console/GdbFullCliConsolePage.java +++ b/dsf-gdb/org.eclipse.cdt.dsf.gdb.ui/src/org/eclipse/cdt/dsf/gdb/internal/ui/console/GdbFullCliConsolePage.java @@ -9,18 +9,12 @@ package org.eclipse.cdt.dsf.gdb.internal.ui.console; import java.io.UnsupportedEncodingException; import java.nio.charset.Charset; -import java.util.concurrent.RejectedExecutionException; import org.eclipse.cdt.debug.internal.ui.views.debuggerconsole.DebuggerConsoleView; import org.eclipse.cdt.debug.ui.debuggerconsole.IDebuggerConsole; import org.eclipse.cdt.debug.ui.debuggerconsole.IDebuggerConsoleView; -import org.eclipse.cdt.dsf.concurrent.DsfRunnable; import org.eclipse.cdt.dsf.gdb.IGdbDebugPreferenceConstants; import org.eclipse.cdt.dsf.gdb.internal.ui.GdbUIPlugin; -import org.eclipse.cdt.dsf.gdb.launching.GdbLaunch; -import org.eclipse.cdt.dsf.gdb.service.IGDBBackend; -import org.eclipse.cdt.dsf.service.DsfServicesTracker; -import org.eclipse.cdt.dsf.service.DsfSession; import org.eclipse.cdt.utils.pty.PTY; import org.eclipse.core.runtime.IAdaptable; import org.eclipse.debug.core.ILaunch; @@ -50,13 +44,13 @@ import org.eclipse.ui.part.Page; public class GdbFullCliConsolePage extends Page implements IDebugContextListener { - private final DsfSession fSession; private final ILaunch fLaunch; - private PTY fGdbPty; + private final PTY fGdbPty; private Composite fMainComposite; private final IDebuggerConsoleView fView; private final IDebuggerConsole fConsole; + private final IGdbTerminalControlConnector fGdbTerminalControlConnector; private MenuManager fMenuManager; @@ -76,16 +70,12 @@ public class GdbFullCliConsolePage extends Page implements IDebugContextListener private IPropertyChangeListener fConsolePropertyChangeListener; - public GdbFullCliConsolePage(GdbFullCliConsole gdbConsole, IDebuggerConsoleView view) { + public GdbFullCliConsolePage(GdbFullCliConsole gdbConsole, IDebuggerConsoleView view, PTY pty) { fConsole = gdbConsole; + fGdbTerminalControlConnector = gdbConsole.getTerminalControlConnector(); fView = view; fLaunch = gdbConsole.getLaunch(); - if (fLaunch instanceof GdbLaunch) { - fSession = ((GdbLaunch)fLaunch).getSession(); - } else { - fSession = null; - assert false; - } + fGdbPty = pty; fConsolePropertyChangeListener = new IPropertyChangeListener() { @Override @@ -124,9 +114,6 @@ public class GdbFullCliConsolePage extends Page implements IDebugContextListener createTerminalControl(); createContextMenu(); configureToolBar(getSite().getActionBars().getToolBarManager()); - - // Hook the terminal control to the GDB process - attachTerminalToGdbProcess(); } private void createTerminalControl() { @@ -139,15 +126,34 @@ public class GdbFullCliConsolePage extends Page implements IDebugContextListener fMainComposite, new ITerminalConnector[] {}, true); + + fTerminalControl.setConnector(new GdbTerminalPageConnector(fGdbTerminalControlConnector, fGdbPty)); try { fTerminalControl.setEncoding(Charset.defaultCharset().name()); } catch (UnsupportedEncodingException e) { } + if (fTerminalControl instanceof ITerminalControl) { + ((ITerminalControl)fTerminalControl).setConnectOnEnterIfClosed(false); + ((ITerminalControl)fTerminalControl).setVT100LineWrapping(true); + } + // Set the inverted colors option based on the stored preference IPreferenceStore store = GdbUIPlugin.getDefault().getPreferenceStore(); setInvertedColors(store.getBoolean(IGdbDebugPreferenceConstants.PREF_CONSOLE_INVERTED_COLORS)); + + // Must use syncExec because the logic within must complete before the rest + // of the class methods (specifically getProcess()) is called + fMainComposite.getDisplay().syncExec(new Runnable() { + @Override + public void run() { + if (fTerminalControl != null && !fTerminalControl.isDisposed()) { + fTerminalControl.clearTerminal(); + fTerminalControl.connectTerminal(); + } + } + }); } protected void createContextMenu() { @@ -206,51 +212,6 @@ public class GdbFullCliConsolePage extends Page implements IDebugContextListener fTerminalControl.setFocus(); } - protected void attachTerminalToGdbProcess() { - if (fSession == null) { - return; - } - - try { - fSession.getExecutor().submit(new DsfRunnable() { - @Override - public void run() { - DsfServicesTracker tracker = new DsfServicesTracker(GdbUIPlugin.getBundleContext(), fSession.getId()); - IGDBBackend backend = tracker.getService(IGDBBackend.class); - tracker.dispose(); - - if (backend != null) { - if (backend.getProcess() != null) { - fGdbPty = backend.getProcessPty(); - attachTerminal(backend.getProcess()); - } - } - } - }); - } catch (RejectedExecutionException e) { - } - } - - protected void attachTerminal(Process process) { - fTerminalControl.setConnector(new GdbTerminalConnector(process, fGdbPty)); - if (fTerminalControl instanceof ITerminalControl) { - ((ITerminalControl)fTerminalControl).setConnectOnEnterIfClosed(false); - ((ITerminalControl)fTerminalControl).setVT100LineWrapping(true); - } - - // Must use syncExec because the logic within must complete before the rest - // of the class methods (specifically getProcess()) is called - fMainComposite.getDisplay().syncExec(new Runnable() { - @Override - public void run() { - if (fTerminalControl != null && !fTerminalControl.isDisposed()) { - fTerminalControl.clearTerminal(); - fTerminalControl.connectTerminal(); - } - } - }); - } - /** * Returns the launch to which the current selection belongs. * diff --git a/dsf-gdb/org.eclipse.cdt.dsf.gdb.ui/src/org/eclipse/cdt/dsf/gdb/internal/ui/console/GdbTerminalConnector.java b/dsf-gdb/org.eclipse.cdt.dsf.gdb.ui/src/org/eclipse/cdt/dsf/gdb/internal/ui/console/GdbTerminalPageConnector.java similarity index 63% rename from dsf-gdb/org.eclipse.cdt.dsf.gdb.ui/src/org/eclipse/cdt/dsf/gdb/internal/ui/console/GdbTerminalConnector.java rename to dsf-gdb/org.eclipse.cdt.dsf.gdb.ui/src/org/eclipse/cdt/dsf/gdb/internal/ui/console/GdbTerminalPageConnector.java index d0fb6973724..acb46f4b70e 100644 --- a/dsf-gdb/org.eclipse.cdt.dsf.gdb.ui/src/org/eclipse/cdt/dsf/gdb/internal/ui/console/GdbTerminalConnector.java +++ b/dsf-gdb/org.eclipse.cdt.dsf.gdb.ui/src/org/eclipse/cdt/dsf/gdb/internal/ui/console/GdbTerminalPageConnector.java @@ -7,16 +7,10 @@ *******************************************************************************/ package org.eclipse.cdt.dsf.gdb.internal.ui.console; -import java.io.IOException; -import java.io.InputStream; import java.io.OutputStream; import org.eclipse.cdt.utils.pty.PTY; -import org.eclipse.core.runtime.IProgressMonitor; -import org.eclipse.core.runtime.IStatus; import org.eclipse.core.runtime.PlatformObject; -import org.eclipse.core.runtime.Status; -import org.eclipse.core.runtime.jobs.Job; import org.eclipse.tm.internal.terminal.provisional.api.ISettingsStore; import org.eclipse.tm.internal.terminal.provisional.api.ITerminalConnector; import org.eclipse.tm.internal.terminal.provisional.api.ITerminalControl; @@ -25,25 +19,22 @@ import org.eclipse.tm.internal.terminal.provisional.api.TerminalState; /** * Class that connects the GDB process I/O with the terminal. */ -public class GdbTerminalConnector extends PlatformObject implements ITerminalConnector { +public class GdbTerminalPageConnector extends PlatformObject implements ITerminalConnector { private int fTerminalWidth, fTerminalHeight; private ITerminalControl fControl; - private final Process fProcess; private final PTY fPty; + private final IGdbTerminalControlConnector fGdbTerminalCtrlConnector; - public GdbTerminalConnector(Process process, PTY pty) { - if (process == null) { - throw new IllegalArgumentException("Invalid Process"); //$NON-NLS-1$ - } - - fProcess = process; + public GdbTerminalPageConnector(IGdbTerminalControlConnector gdbTerminalCtrlConnector, PTY pty) { fPty = pty; + fGdbTerminalCtrlConnector = gdbTerminalCtrlConnector; } @Override public void disconnect() { - // Set the terminal control state to CLOSED. + fGdbTerminalCtrlConnector.removePageTerminalControl(fControl); + if (fControl != null) { fControl.setState(TerminalState.CLOSED); } @@ -51,9 +42,7 @@ public class GdbTerminalConnector extends PlatformObject implements ITerminalCon @Override public OutputStream getTerminalToRemoteStream() { - // When the user writes to the terminal, it should be sent - // directly to GDB - return fProcess.getOutputStream(); + return fGdbTerminalCtrlConnector.getTerminalToRemoteStream(); } @Override @@ -63,10 +52,7 @@ public class GdbTerminalConnector extends PlatformObject implements ITerminalCon } fControl = control; - - // connect the streams - new OutputReadJob(fProcess.getInputStream()).schedule(); - new OutputReadJob(fProcess.getErrorStream()).schedule(); + fGdbTerminalCtrlConnector.addPageTerminalControl(fControl); // Set the terminal control state to CONNECTED fControl.setState(TerminalState.CONNECTED); @@ -137,33 +123,4 @@ public class GdbTerminalConnector extends PlatformObject implements ITerminalCon public void save(ISettingsStore arg0) { // we don't do settings } - - private class OutputReadJob extends Job { - { - setSystem(true); - } - - private InputStream fInputStream; - - OutputReadJob(InputStream inputStream) { - super("GDB CLI output Job"); //$NON-NLS-1$ - fInputStream = inputStream; - } - - @Override - protected IStatus run(IProgressMonitor monitor) { - try { - byte[] b = new byte[1024]; - int read = 0; - do { - read = fInputStream.read(b); - if (read > 0) { - fControl.getRemoteToTerminalOutputStream().write(b, 0, read); - } - } while (read >= 0); - } catch (IOException e) { - } - return Status.OK_STATUS; - } - } } diff --git a/dsf-gdb/org.eclipse.cdt.dsf.gdb.ui/src/org/eclipse/cdt/dsf/gdb/internal/ui/console/IGdbTerminalControlConnector.java b/dsf-gdb/org.eclipse.cdt.dsf.gdb.ui/src/org/eclipse/cdt/dsf/gdb/internal/ui/console/IGdbTerminalControlConnector.java new file mode 100644 index 00000000000..72cba3ad5c4 --- /dev/null +++ b/dsf-gdb/org.eclipse.cdt.dsf.gdb.ui/src/org/eclipse/cdt/dsf/gdb/internal/ui/console/IGdbTerminalControlConnector.java @@ -0,0 +1,37 @@ +/******************************************************************************* + * Copyright (c) 2016 Ericsson and others. + * All rights reserved. This program and the accompanying materials + * are made available under the terms of the Eclipse Public License v1.0 + * which accompanies this distribution, and is available at + * http://www.eclipse.org/legal/epl-v10.html + *******************************************************************************/ +package org.eclipse.cdt.dsf.gdb.internal.ui.console; + +import java.io.OutputStream; + +import org.eclipse.tm.internal.terminal.provisional.api.ITerminalControl; + +/** + * Interface to connect multiple page terminal controls with their single associated GDB process I/O. + */ +public interface IGdbTerminalControlConnector { + + /** + * Adds a terminal control which is ready to receive the process output + * + * @param terminalControl + */ + void addPageTerminalControl(ITerminalControl terminalControl); + + /** + * Removes a registered terminal control + * + * @param terminalControl + */ + void removePageTerminalControl(ITerminalControl terminalControl); + + /** + * @return The OutputStream shared among the managed terminal control instances + */ + public OutputStream getTerminalToRemoteStream(); +}