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 <alvsan09@gmail.com>
Signed-off-by: Marc Khouzam <marc.khouzam@ericsson.com>

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());

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<ITerminalControl> 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;
+	}
 }

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) {
+		fGdbPty = pty;
-			fSession = ((GdbLaunch)fLaunch).getSession();
-		} else {
-			fSession = null;
-			assert false;
-		}
 
 		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() {
@@ -140,14 +127,33 @@ public class GdbFullCliConsolePage extends Page implements IDebugContextListener
 				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.
 	 * 

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) {
+    public GdbTerminalPageConnector(IGdbTerminalControlConnector gdbTerminalCtrlConnector, PTY pty) {
-    	if (process == null) {
-    		throw new IllegalArgumentException("Invalid Process"); //$NON-NLS-1$
-    	}
-
-    	fProcess = process;
     	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
+		return fGdbTerminalCtrlConnector.getTerminalToRemoteStream();
-		// directly to GDB
-		return fProcess.getOutputStream();
 	}
 
 	@Override
@@ -63,10 +52,7 @@ public class GdbTerminalConnector extends PlatformObject implements ITerminalCon
     	}
 
 		fControl = control;
-
+		fGdbTerminalCtrlConnector.addPageTerminalControl(fControl);
-		// connect the streams
-		new OutputReadJob(fProcess.getInputStream()).schedule();
-		new OutputReadJob(fProcess.getErrorStream()).schedule();
 
 		// 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;
-        }
-    }
 }

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();
+}