mirrors/cdt
1
0
Fork 0
mirror of https://github.com/eclipse-cdt/cdt synced 2025-07-26 18:35:32 +02:00
Code Issues Wiki Activity

Bug 345164: Revert changes to Spawner and instead explicitly close streams in DSF-GDB and CDI

Browse source
This commit is contained in:
Marc Khouzam 2011-05-10 17:38:42 +00:00
parent b609ec68ba
commit 04e7974825
4 changed files with 91 additions and 24 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

81
core/org.eclipse.cdt.core/utils/org/eclipse/cdt/utils/spawner/Spawner.java
View file

@ -143,6 +143,7 @@ public class Spawner extends Process {
/**
* See java.lang.Process#getInputStream ();
* The client is responsible for closing the stream explicitly.
**/
@Override
public InputStream getInputStream() {
@ -158,6 +159,7 @@ public class Spawner extends Process {
/**
* See java.lang.Process#getOutputStream ();
* The client is responsible for closing the stream explicitly.
**/
@Override
public OutputStream getOutputStream() {
@ -173,6 +175,7 @@ public class Spawner extends Process {
/**
* See java.lang.Process#getErrorStream ();
* The client is responsible for closing the stream explicitly.
**/
@Override
public InputStream getErrorStream() {
@ -202,21 +205,28 @@ public class Spawner extends Process {
while (!isDone) {
wait();
}
// Close the streams on this side.
// If the stream was never used
// we still want to create it, so that we can close
// the pipe on the other side.
// For situations where the user does not call destroy(),
// we try to kill the streams that were not used here.
// We check for streams that were not created, we create
// them to attach to the pipes, and then we close them
// to release the pipes.
// Streams that were created by the client need to be
// closed by the client itself.
//
// Technically, we shouldn't close the streams here but
// should rely on destroy() being called properly.
// However, since we did close the streams here, we
// cannot remove this code or we would break anyone who
// does not call destroy() properly and relies on this
// code instead.
// Bug 345164
try { getErrorStream().close(); } catch (IOException e) {}
try { getInputStream().close(); } catch (IOException e) {}
try { getOutputStream().close();} catch (IOException e) {}
// But 345164
try {
if(null == err)
getErrorStream().close();
} catch (IOException e) {}
try {
if(null == in)
getInputStream().close();
} catch (IOException e) {}
try {
if(null == out)
getOutputStream().close();
} catch (IOException e) {}
return status;
}
@ -233,20 +243,47 @@ public class Spawner extends Process {
/**
* See java.lang.Process#destroy ();
*
* Clients are responsible for explicitly closing any streams
* that they have requested through
* getErrorStream(), getInputStream() or getOutputStream()
**/
@Override
public synchronized void destroy() {
// Sends the TERM
terminate();
// Close the streams on this side.
// If the stream was never used
// we still want to create it, so that we can close
// the pipe on the other side.
// Bug 345164
try { getErrorStream().close(); } catch (IOException e) {}
try { getInputStream().close(); } catch (IOException e) {}
try { getOutputStream().close();} catch (IOException e) {}
//
// We only close the streams that were
// never used by any client.
// So, if the stream was not created yet,
// we create it ourselves and close it
// right away, so as to release the pipe.
// Note that even if the stream was never
// created, the pipe has been allocated in
// native code, so we need to create the
// stream and explicitly close it.
//
// We don't close streams the clients have
// created because we don't know when the
// client will be finished using them.
// It is up to the client to close those
// streams.
//
// But 345164
try {
if(null == err)
getErrorStream().close();
} catch (IOException e) {}
try {
if(null == in)
getInputStream().close();
} catch (IOException e) {}
try {
if(null == out)
getOutputStream().close();
} catch (IOException e) {}
// Grace before using the heavy gone.
if (!isDone) {
try {

13
debug/org.eclipse.cdt.debug.mi.core/src/org/eclipse/cdt/debug/mi/core/MIProcessAdapter.java
View file

@ -172,6 +172,19 @@ public class MIProcessAdapter implements MIProcess {
}
public void destroy() {
// We are responsible for closing the streams we have used or else
// we will leak pipes.
// Bug 345164
try {
getErrorStream().close();
} catch (IOException e) {}
try {
getInputStream().close();
} catch (IOException e) {}
try {
getOutputStream().close();
} catch (IOException e) {}
fGDBProcess.destroy();
}

9
dsf-gdb/org.eclipse.cdt.dsf.gdb/src/org/eclipse/cdt/dsf/gdb/launching/LaunchUtils.java
View file

@ -301,9 +301,10 @@ public class LaunchUtils {
"Error while launching command: " + cmd, e.getCause()));//$NON-NLS-1$
}
InputStream stream = null;
StringBuilder cmdOutput = new StringBuilder(200);
try {
InputStream stream = process.getInputStream();
stream = process.getInputStream();
Reader r = new InputStreamReader(stream);
BufferedReader reader = new BufferedReader(r);
@ -316,7 +317,13 @@ public class LaunchUtils {
"Error reading GDB STDOUT after sending: " + cmd, e.getCause()));//$NON-NLS-1$
} finally {
// Cleanup to avoid leaking pipes
// Close the stream we used, and then destroy the process
// Bug 345164
if (stream != null) {
try {
stream.close();
} catch (IOException e) {}
}
process.destroy();
}

12
dsf-gdb/org.eclipse.cdt.dsf.gdb/src/org/eclipse/cdt/dsf/gdb/service/GDBBackend.java
View file

@ -442,7 +442,17 @@ public class GDBBackend extends AbstractDsfService implements IGDBBackend {
}
public void destroy() {
// destroy() should be supported even if it's not spawner.
// We are responsible for closing the streams we have used or else
// we will leak pipes.
// Bug 345164
try {
getMIOutputStream().close();
} catch (IOException e) {}
try {
getMIInputStream().close();
} catch (IOException e) {}
// destroy() should be supported even if it's not spawner.
if (getState() == State.STARTED) {
fProcess.destroy();
}