[PATCH] [MinGW]: ConcreteProcess Refactoring and Pipe Bugfix (Updated to fix PR/12231)
Mohan Embar
gnustuff@thisiscool.com
Mon Oct 27 07:42:00 GMT 2003
Hi People,
Here is an updated version of the patch that fixes the flashing
console window issue on MinGW:
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=12231
The only change from the previous version of this patch:
http://gcc.gnu.org/ml/java-patches/2003-q4/msg00253.html
...is passing CREATE_NO_WINDOW to CreateProcess().
Ranjit had mentioned testing whether the parent process
is a GUI application and the child process is a console
application before using this flag, but upon further investigation,
this appeared unnecessary:
- this flag is ignored when spawning a GUI application
- under the JRE, spawned console applications never get a new
console anyway
(Note all usages of the term "console" in this post are in the
Win32 sense.)
This fix appears to work on WinXP and Win2K, with behavior
identical to the JRE (1.4.1_01). On Win9X, the flashing window
still appears because CREATE_NO_WINDOW is ignored. It might
be possible to mitigate this in part by using SHGetFileInfo to test
whether the child process is a console application and then use
SW_HIDE or SW_SHOWMINIMIZED in the STARTUPINFO
passed to CreateProcess(), but I don't feel like doing this. (Still
hoping Win9X will go away like a bad dream. Oops, did I say that
out loud?)
-- Mohan
http://www.thisiscool.com/
http://www.animalsong.org/
ChangeLog
2003-10-27 Mohan Embar <gnustuff@thisiscool.com>
PR libgcj/12231
* java/lang/Win32Process.java (hasExited) Changed from
public to private.
(startProcess): Likewise.
(cleanup): Likewise.
* java/lang/natWin32Process.cc (cleanup) Don't close
input, output and error streams.
(ChildProcessPipe): New helper class.
(startProcess): Refactored to use ChildProcessPipe.
Use CREATE_NO_WINDOW when launching child process.
Index: java/lang/Win32Process.java
===================================================================
RCS file: /cvs/gcc/gcc/libjava/java/lang/Win32Process.java,v
retrieving revision 1.7
diff -u -2 -r1.7 Win32Process.java
--- java/lang/Win32Process.java 27 Jul 2003 04:13:03 -0000 1.7
+++ java/lang/Win32Process.java 27 Oct 2003 07:06:44 -0000
@@ -29,6 +29,4 @@
public native void destroy ();
- public native boolean hasExited ();
-
public int exitValue ()
{
@@ -56,11 +54,4 @@
public native int waitFor () throws InterruptedException;
- public native void startProcess (String[] progarray,
- String[] envp,
- File dir)
- throws IOException;
-
- public native void cleanup ();
-
public ConcreteProcess (String[] progarray,
String[] envp,
@@ -90,3 +81,10 @@
// Exit code of the child if it has exited.
private int exitCode;
+
+ private native boolean hasExited ();
+ private native void startProcess (String[] progarray,
+ String[] envp,
+ File dir)
+ throws IOException;
+ private native void cleanup ();
}
Index: java/lang/natWin32Process.cc
===================================================================
RCS file: /cvs/gcc/gcc/libjava/java/lang/natWin32Process.cc,v
retrieving revision 1.6
diff -u -2 -r1.6 natWin32Process.cc
--- java/lang/natWin32Process.cc 19 Sep 2003 08:28:43 -0000 1.6
+++ java/lang/natWin32Process.cc 27 Oct 2003 07:06:45 -0000
@@ -30,21 +30,30 @@
java::lang::ConcreteProcess::cleanup (void)
{
- if (inputStream != NULL)
- {
- inputStream->close ();
- inputStream = NULL;
- }
-
- if (outputStream != NULL)
- {
- outputStream->close ();
- outputStream = NULL;
- }
-
- if (errorStream != NULL)
- {
- errorStream->close ();
- errorStream = NULL;
- }
+ // FIXME:
+ // We used to close the input, output and
+ // error streams here, but we can't do that
+ // because the caller also has the right
+ // to close these and FileInputStream and FileOutputStream
+ // scream if you attempt to close() them twice. Presently,
+ // we use _Jv_platform_close_on_exec, which is similar
+ // to the POSIX approach. _Jv_platform_close_on_exec
+ // doesn't do anything on Win32, but since
+ // the handles to our ends of the pipe are non-inheritable,
+ // this basically has the same effect.
+ //
+ // What I wanted to do is have private nested
+ // classes in ConcreteProcess which extend FileInputStream
+ // and FileOutputStream, respectively, but override
+ // close() to permit multiple calls to close(). This
+ // led to class header and platform configury issues
+ // that I didn't feel like dealing with. However,
+ // this approach could conceivably be a good multiplatform
+ // one since delaying the pipe close until process
+ // termination could be wasteful if many child processes
+ // are spawned within the parent process' lifetime.
+ inputStream = NULL;
+ outputStream = NULL;
+ errorStream = NULL;
+
if (procHandle)
{
@@ -130,4 +139,94 @@
}
+
+// Helper class for creating and managing the pipes
+// used for I/O redirection for child processes.
+class ChildProcessPipe
+{
+public:
+ // Indicates from the child process' point of view
+ // whether the pipe is for reading or writing.
+ enum EType {INPUT, OUTPUT};
+
+ ChildProcessPipe(EType eType);
+ ~ChildProcessPipe();
+
+ // Returns a pipe handle suitable for use by the parent process
+ HANDLE getParentHandle();
+
+ // Returns a pipe handle suitable for use by the child process.
+ HANDLE getChildHandle();
+
+private:
+ EType m_eType;
+ HANDLE m_hRead, m_hWrite;
+};
+
+ChildProcessPipe::ChildProcessPipe(EType eType):
+ m_eType(eType)
+{
+ SECURITY_ATTRIBUTES sAttrs;
+
+ // Explicitly allow the handles to the pipes to be inherited.
+ sAttrs.nLength = sizeof (SECURITY_ATTRIBUTES);
+ sAttrs.bInheritHandle = 1;
+ sAttrs.lpSecurityDescriptor = NULL;
+
+ if (CreatePipe (&m_hRead, &m_hWrite, &sAttrs, 0) == 0)
+ {
+ DWORD dwErrorCode = GetLastError ();
+ throw new java::io::IOException (
+ _Jv_WinStrError ("Error creating pipe", dwErrorCode));
+ }
+
+ // If this is the read end of the child, we need
+ // to make the parent write end non-inheritable. Similarly,
+ // if this is the write end of the child, we need to make
+ // the parent read end non-inheritable. If we didn't
+ // do this, the child would inherit these ends and we wouldn't
+ // be able to close them from our end. For full details,
+ // do a Google search on "Q190351".
+ HANDLE& rhStd = m_eType==INPUT ? m_hWrite : m_hRead;
+ HANDLE hStdNew;
+ if (DuplicateHandle (GetCurrentProcess (), rhStd,
+ GetCurrentProcess (), &hStdNew, 0, 0,
+ DUPLICATE_SAME_ACCESS) == 0)
+ {
+ DWORD dwErrorCode = GetLastError ();
+ throw new java::io::IOException (
+ _Jv_WinStrError ("Error duplicating handle", dwErrorCode));
+ }
+
+ // Close the inheritable handle before spawning
+ // the child process.
+ CloseHandle(rhStd);
+
+ // Indicate that we want the non-inheritable handle closed
+ // when the parent process exits. See the comment
+ // in cleanup() for more details.
+ _Jv_platform_close_on_exec ((jint) hStdNew);
+
+ // Only remember the non-inheritable one.
+ rhStd = hStdNew;
+}
+
+ChildProcessPipe::~ChildProcessPipe()
+{
+ // Close the parent end of the pipe. This
+ // destructor is called after the child process
+ // has been spawned.
+ CloseHandle(getChildHandle());
+}
+
+HANDLE ChildProcessPipe::getParentHandle()
+{
+ return m_eType==INPUT ? m_hWrite : m_hRead;
+}
+
+HANDLE ChildProcessPipe::getChildHandle()
+{
+ return m_eType==INPUT ? m_hRead : m_hWrite;
+}
+
void
java::lang::ConcreteProcess::startProcess (jstringArray progarray,
@@ -198,44 +297,14 @@
// We create anonymous pipes to communicate with the child
// on each of standard streams.
-
- HANDLE cldStdInRd, cldStdInWr;
- HANDLE cldStdOutRd, cldStdOutWr;
- HANDLE cldStdErrRd, cldStdErrWr;
-
- SECURITY_ATTRIBUTES sAttrs;
-
- // Explicitly allow the handles to the pipes to be inherited.
- sAttrs.nLength = sizeof (SECURITY_ATTRIBUTES);
- sAttrs.bInheritHandle = 1;
- sAttrs.lpSecurityDescriptor = NULL;
-
-
- if (CreatePipe (&cldStdInRd, &cldStdInWr, &sAttrs, 0) == 0)
- {
- DWORD dwErrorCode = GetLastError ();
- throw new IOException (_Jv_WinStrError ("Error creating stdin pipe",
- dwErrorCode));
- }
-
- if (CreatePipe (&cldStdOutRd, &cldStdOutWr, &sAttrs, 0) == 0)
- {
- DWORD dwErrorCode = GetLastError ();
- throw new IOException (_Jv_WinStrError ("Error creating stdout pipe",
- dwErrorCode));
- }
-
- if (CreatePipe (&cldStdErrRd, &cldStdErrWr, &sAttrs, 0) == 0)
- {
- DWORD dwErrorCode = GetLastError ();
- throw new IOException (_Jv_WinStrError ("Error creating stderr pipe",
- dwErrorCode));
- }
-
- outputStream = new FileOutputStream
- (new FileDescriptor ((jint) cldStdInWr));
- inputStream = new FileInputStream
- (new FileDescriptor ((jint) cldStdOutRd));
- errorStream = new FileInputStream
- (new FileDescriptor ((jint) cldStdErrRd));
+ ChildProcessPipe aChildStdIn(ChildProcessPipe::INPUT);
+ ChildProcessPipe aChildStdOut(ChildProcessPipe::OUTPUT);
+ ChildProcessPipe aChildStdErr(ChildProcessPipe::OUTPUT);
+
+ outputStream = new FileOutputStream (new FileDescriptor (
+ (jint) aChildStdIn.getParentHandle ()));
+ inputStream = new FileInputStream (new FileDescriptor (
+ (jint) aChildStdOut.getParentHandle ()));
+ errorStream = new FileInputStream (new FileDescriptor (
+ (jint) aChildStdErr.getParentHandle ()));
// Now create the child process.
@@ -251,8 +320,12 @@
si.dwFlags |= STARTF_USESTDHANDLES;
- si.hStdInput = cldStdInRd;
- si.hStdOutput = cldStdOutWr;
- si.hStdError = cldStdErrWr;
-
+ si.hStdInput = aChildStdIn.getChildHandle();
+ si.hStdOutput = aChildStdOut.getChildHandle();
+ si.hStdError = aChildStdErr.getChildHandle();
+
+ // Spawn the process. CREATE_NO_WINDOW only applies when
+ // starting a console application; it suppresses the
+ // creation of a console window. This flag is ignored on
+ // Win9X.
if (CreateProcess (NULL,
cmdLine,
@@ -260,5 +333,5 @@
NULL,
1,
- 0,
+ CREATE_NO_WINDOW,
env,
wdir,
@@ -272,9 +345,4 @@
procHandle = (jint ) pi.hProcess;
-
- // Close the wrong ends (for the parent) of the pipes.
- CloseHandle (cldStdInRd);
- CloseHandle (cldStdOutWr);
- CloseHandle (cldStdErrWr);
_Jv_Free (cmdLine);
More information about the Java-patches
mailing list