diff --git a/libjava/ChangeLog b/libjava/ChangeLog index 385e26d8cdd..dce155806cf 100644 --- a/libjava/ChangeLog +++ b/libjava/ChangeLog @@ -1,3 +1,46 @@ +2007-05-12 David Daney + + PR libgcj/29324 + * include/posix-threads.h (_Jv_BlockSigchld): Declare. + (_Jv_UnBlockSigchld): Same. + * posix-threads.cc: Include posix-threads.h. + (block_sigchld) Rename to... + (_Jv_BlockSigchld) ... this. + (_Jv_UnBlockSigchld): New function. + (_Jv_InitThreads): Call _Jv_BlockSigchld in place of block_sigchld. + (_Jv_ThreadStart): Same. + * java/lang/PosixProcess$ProcessManager.h: Regenerate. + * java/lang/PosixProcess.java: Clean up imports. + (ProcessManager): Make final. + (ProcessManager.queue): Genericise and make private. + (ProcessManager.pidToProcess): Remove. + (ProcessManager.liveProcesses): New field. + (ProcessManager.reaperPID): Remove. + (ProcessManager.nativeData): New field. + (ProcessManager.removeProcessFromMap): Remove. + (ProcessManager.addProcessToMap):Remove. + (ProcessManager.addToLiveProcesses): New method. + (ProcessManager.run): Rewritten. + (ProcessManager.reap): Change method signature, + (getErrorStream): Correct formatting. + (getInputStream): Same. + (spawn): Add process to liveProcesses list. + (pid): Make package private. + * java/lang/PosixProcess.h: Regenerate. + * java/lang/natPosixProcess.cc: Include posix.h and posix-threads.h. + Add useing namespace java::lang. + (ProcessManagerInternal): New struct. + (sigchld_handler): Rewritten. + (init): Rewritten. + (waitForSignal): Same. + (reap): Same. + (signalReaper): Same. + (nativeDestroy): Call kill as ::kill. + (nativeSpawn): Correct formatting. + * classpath/lib/java/lang/PosixProcess$EOFInputStream.class: Regenerate. + * classpath/lib/java/lang/PosixProcess.class: Same. + * classpath/lib/java/lang/PosixProcess$ProcessManager.class: Same. + 2007-05-07 Ian Lance Taylor PR java/31842 diff --git a/libjava/classpath/lib/java/lang/PosixProcess$EOFInputStream.class b/libjava/classpath/lib/java/lang/PosixProcess$EOFInputStream.class index a7103f81a1c..c4cc6c37ccb 100644 Binary files a/libjava/classpath/lib/java/lang/PosixProcess$EOFInputStream.class and b/libjava/classpath/lib/java/lang/PosixProcess$EOFInputStream.class differ diff --git a/libjava/classpath/lib/java/lang/PosixProcess$ProcessManager.class b/libjava/classpath/lib/java/lang/PosixProcess$ProcessManager.class index 2e260c35813..1d80a2d4cc1 100644 Binary files a/libjava/classpath/lib/java/lang/PosixProcess$ProcessManager.class and b/libjava/classpath/lib/java/lang/PosixProcess$ProcessManager.class differ diff --git a/libjava/classpath/lib/java/lang/PosixProcess.class b/libjava/classpath/lib/java/lang/PosixProcess.class index 0227add131e..0241ae74f9e 100644 Binary files a/libjava/classpath/lib/java/lang/PosixProcess.class and b/libjava/classpath/lib/java/lang/PosixProcess.class differ diff --git a/libjava/include/posix-threads.h b/libjava/include/posix-threads.h index d5f64957c22..806ee556011 100644 --- a/libjava/include/posix-threads.h +++ b/libjava/include/posix-threads.h @@ -91,6 +91,14 @@ _Jv_GetPlatformThreadID(_Jv_Thread_t *t) return t->thread; } +// +// Signal helpers. +// + +void _Jv_BlockSigchld(); +void _Jv_UnBlockSigchld(); + + // // Condition variables. // diff --git a/libjava/java/lang/PosixProcess$ProcessManager.h b/libjava/java/lang/PosixProcess$ProcessManager.h index d8d0d594232..4b3b62e13dd 100644 --- a/libjava/java/lang/PosixProcess$ProcessManager.h +++ b/libjava/java/lang/PosixProcess$ProcessManager.h @@ -7,31 +7,37 @@ #pragma interface #include +extern "Java" +{ + namespace gnu + { + namespace gcj + { + class RawDataManaged; + } + } +} class java::lang::PosixProcess$ProcessManager : public ::java::lang::Thread { public: // actually package-private PosixProcess$ProcessManager(); -private: - ::java::lang::PosixProcess * removeProcessFromMap(jlong); -public: // actually package-private - virtual void addProcessToMap(::java::lang::PosixProcess *); - virtual void startExecuting(::java::lang::PosixProcess *); - virtual void waitUntilReady(); + void addToLiveProcesses(::java::lang::PosixProcess *); + void startExecuting(::java::lang::PosixProcess *); + void waitUntilReady(); public: - virtual void run(); + void run(); private: void init(); void waitForSignal(); - jboolean reap(); + jboolean reap(::java::lang::PosixProcess *); void signalReaper(); -public: // actually package-private - ::java::util::List * __attribute__((aligned(__alignof__( ::java::lang::Thread)))) queue; -private: - ::java::util::Map * pidToProcess; + ::java::util::LinkedList * __attribute__((aligned(__alignof__( ::java::lang::Thread)))) queue; + ::java::util::LinkedList * liveProcesses; jboolean ready; - jlong reaperPID; +public: // actually package-private + static ::gnu::gcj::RawDataManaged * nativeData; public: static ::java::lang::Class class$; }; diff --git a/libjava/java/lang/PosixProcess.h b/libjava/java/lang/PosixProcess.h index a43fa0739d5..3254f5224ce 100644 --- a/libjava/java/lang/PosixProcess.h +++ b/libjava/java/lang/PosixProcess.h @@ -32,17 +32,16 @@ private: void nativeSpawn(); public: // actually package-private PosixProcess(JArray< ::java::lang::String * > *, JArray< ::java::lang::String * > *, ::java::io::File *, jboolean); - static jlong access$0(::java::lang::PosixProcess *); - static ::java::lang::Object * access$1(); - static void access$2(::java::lang::PosixProcess$ProcessManager *); + static ::java::lang::Object * access$0(); + static void access$1(::java::lang::PosixProcess$ProcessManager *); private: JArray< ::java::lang::String * > * __attribute__((aligned(__alignof__( ::java::lang::Process)))) progarray; JArray< ::java::lang::String * > * envp; ::java::io::File * dir; jboolean redirect; ::java::lang::Throwable * exception; - jlong pid; public: // actually package-private + jlong pid; static const jint STATE_WAITING_TO_START = 0; static const jint STATE_RUNNING = 1; static const jint STATE_TERMINATED = 2; diff --git a/libjava/java/lang/PosixProcess.java b/libjava/java/lang/PosixProcess.java index 6b1496426dd..dd59e7b93a1 100644 --- a/libjava/java/lang/PosixProcess.java +++ b/libjava/java/lang/PosixProcess.java @@ -13,11 +13,10 @@ import java.io.File; import java.io.IOException; import java.io.InputStream; import java.io.OutputStream; -import java.util.HashMap; +import java.util.Iterator; import java.util.LinkedList; -import java.util.List; -import java.util.Map; +import gnu.gcj.RawDataManaged; /** * @author Tom Tromey @@ -27,7 +26,7 @@ import java.util.Map; */ final class PosixProcess extends Process { - static class ProcessManager extends Thread + static final class ProcessManager extends Thread { /** * A list of {@link PosixProcess PosixProcesses} to be @@ -35,10 +34,12 @@ final class PosixProcess extends Process * for all process related operations. To avoid dead lock * ensure queueLock is obtained before PosixProcess. */ - List queue = new LinkedList(); - private Map pidToProcess = new HashMap(); + private LinkedList queue = new LinkedList(); + private LinkedList liveProcesses = + new LinkedList(); private boolean ready = false; - private long reaperPID; + + static RawDataManaged nativeData; ProcessManager() { @@ -53,27 +54,14 @@ final class PosixProcess extends Process } /** - * Get the PosixProcess object with the given pid and - * remove it from the map. This method is called from the - * native code for {@link #reap()). The mapping is removed so - * the PosixProcesses can be GCed after they terminate. - * - * @param p The pid of the process. - */ - private PosixProcess removeProcessFromMap(long p) - { - return (PosixProcess) pidToProcess.remove(new Long(p)); - } - - /** - * Put the given PosixProcess in the map using the Long - * value of its pid as the key. + * Add a process to the list of running processes. This must only + * be called with the queueLock held. * * @param p The PosixProcess. */ - void addProcessToMap(PosixProcess p) + void addToLiveProcesses(PosixProcess p) { - pidToProcess.put(new Long(p.pid), p); + liveProcesses.add(p); } /** @@ -122,61 +110,66 @@ final class PosixProcess extends Process // Now ready to accept requests. synchronized (this) { - ready = true; - this.notifyAll(); + ready = true; + this.notifyAll(); } for (;;) { - try - { - synchronized (queueLock) - { - boolean haveMoreChildren = reap(); - if (! haveMoreChildren && queue.size() == 0) - { - // This reaper thread could exit, but we - // keep it alive for a while in case - // someone wants to start more Processes. - try - { - queueLock.wait(1000L); - if (queue.size() == 0) - { - processManager = null; - return; // Timed out. - } - } - catch (InterruptedException ie) - { - // Ignore and exit the thread. - return; - } - } - while (queue.size() > 0) - { - PosixProcess p = (PosixProcess) queue.remove(0); - p.spawn(this); - } - } + try + { + synchronized (queueLock) + { + Iterator processIterator = + liveProcesses.iterator(); + while (processIterator.hasNext()) + { + boolean reaped = reap(processIterator.next()); + if (reaped) + processIterator.remove(); + } + if (liveProcesses.size() == 0 && queue.size() == 0) + { + // This reaper thread could exit, but we keep it + // alive for a while in case someone wants to + // start more Processes. + try + { + queueLock.wait(1000L); + if (queue.size() == 0) + { + processManager = null; + return; // Timed out. + } + } + catch (InterruptedException ie) + { + // Ignore and exit the thread. + return; + } + } + while (queue.size() > 0) + { + PosixProcess p = queue.remove(0); + p.spawn(this); + } + } - // Wait for a SIGCHLD from either an exiting - // process or the startExecuting() method. This - // is done outside of the synchronized block to - // allow other threads to enter and submit more - // jobs. - waitForSignal(); - } - catch (Exception ex) - { - ex.printStackTrace(System.err); - } + // Wait for a SIGCHLD from either an exiting process or + // the startExecuting() method. This is done outside of + // the synchronized block to allow other threads to + // enter and submit more jobs. + waitForSignal(); + } + catch (Exception ex) + { + ex.printStackTrace(System.err); + } } } /** * Setup native signal handlers and other housekeeping things. - * */ private native void init(); @@ -187,12 +180,14 @@ final class PosixProcess extends Process private native void waitForSignal(); /** - * Try to reap as many children as possible without blocking. + * Try to reap the specified child without blocking. * - * @return true if more live children exist. + * @param p the process to try to reap. + * + * @return true if the process terminated. * */ - private native boolean reap(); + private native boolean reap(PosixProcess p); /** * Send SIGCHLD to the reaper thread. @@ -295,7 +290,7 @@ final class PosixProcess extends Process returnedErrorStream = EOFInputStream.instance; else returnedErrorStream = errorStream; - + return returnedErrorStream; } @@ -308,7 +303,7 @@ final class PosixProcess extends Process returnedInputStream = EOFInputStream.instance; else returnedInputStream = inputStream; - + return returnedInputStream; } @@ -329,7 +324,7 @@ final class PosixProcess extends Process /** * Start this process running. This should only be called by the - * ProcessManager. + * ProcessManager with the queueLock held. * * @param pm The ProcessManager that made the call. */ @@ -342,7 +337,7 @@ final class PosixProcess extends Process // There is no race with reap() in the pidToProcess map // because this is always called from the same thread // doing the reaping. - pm.addProcessToMap(this); + pm.addToLiveProcesses(this); state = STATE_RUNNING; // Notify anybody waiting on state change. this.notifyAll(); @@ -426,7 +421,7 @@ final class PosixProcess extends Process private Throwable exception; /** The process id. This is cast to a pid_t on the native side. */ - private long pid; + long pid; // FIXME: Why doesn't the friend declaration in PosixProcess.h // allow PosixProcess$ProcessManager native code access these diff --git a/libjava/java/lang/natPosixProcess.cc b/libjava/java/lang/natPosixProcess.cc index 252da6e80ab..c7b8b6e5eb0 100644 --- a/libjava/java/lang/natPosixProcess.cc +++ b/libjava/java/lang/natPosixProcess.cc @@ -30,6 +30,8 @@ details. */ #include #include +#include +#include #include #include @@ -48,6 +50,7 @@ details. */ #include using gnu::java::nio::channels::FileChannelImpl; +using namespace java::lang; extern char **environ; @@ -91,13 +94,37 @@ myclose (int &fd) fd = -1; } +namespace +{ + struct ProcessManagerInternal + { + int pipe_ends[2]; + struct sigaction old_sigaction; + }; +} + + // There has to be a signal handler in order to be able to // sigwait() on SIGCHLD. The information passed is ignored as it // will be recovered by the waitpid() call. static void -sigchld_handler (int) +sigchld_handler (int sig, siginfo_t *si, void *third) { - // Ignore. + if (PosixProcess$ProcessManager::nativeData != NULL) + { + ProcessManagerInternal *pmi = + (ProcessManagerInternal *)PosixProcess$ProcessManager::nativeData; + char c = 0; + ::write(pmi->pipe_ends[1], &c, 1); + if (pmi->old_sigaction.sa_handler != SIG_DFL + && pmi->old_sigaction.sa_handler != SIG_IGN) + { + if ((pmi->old_sigaction.sa_flags & SA_SIGINFO) != 0) + pmi->old_sigaction.sa_sigaction(sig, si, third); + else + (*pmi->old_sigaction.sa_handler)(sig); + } + } } @@ -105,22 +132,35 @@ sigchld_handler (int) void java::lang::PosixProcess$ProcessManager::init () { - using namespace java::lang; - // Remenber our PID so other threads can kill us. - reaperPID = (jlong) pthread_self (); + // The nativeData is static to avoid races installing the signal + // handler in the case that it is chained. + if (nativeData == NULL ) + { + ProcessManagerInternal *pmi = + (ProcessManagerInternal *)JvAllocBytes(sizeof(ProcessManagerInternal)); - // SIGCHLD is blocked in all threads in posix-threads.cc. - // Setup the SIGCHLD handler. - struct sigaction sa; - memset (&sa, 0, sizeof (sa)); + if (0 != ::pipe(pmi->pipe_ends)) + goto error; - sa.sa_handler = sigchld_handler; - // We only want signals when the things exit. - sa.sa_flags = SA_NOCLDSTOP; + // Make writing non-blocking so that the signal handler will + // never block. + int fl = ::fcntl(pmi->pipe_ends[1], F_GETFL); + ::fcntl(pmi->pipe_ends[1], F_SETFL, fl | O_NONBLOCK); - if (-1 == sigaction (SIGCHLD, &sa, NULL)) - goto error; + nativeData = (::gnu::gcj::RawDataManaged *)pmi; + // SIGCHLD is blocked in all threads in posix-threads.cc. + // Setup the SIGCHLD handler. + struct sigaction sa; + memset (&sa, 0, sizeof (sa)); + + sa.sa_sigaction = sigchld_handler; + // We only want signals when the things exit. + sa.sa_flags = SA_NOCLDSTOP | SA_SIGINFO; + + if (-1 == sigaction (SIGCHLD, &sa, &pmi->old_sigaction)) + goto error; + } // All OK. return; @@ -132,79 +172,52 @@ void java::lang::PosixProcess$ProcessManager::waitForSignal () { // Wait for SIGCHLD - sigset_t mask; - pthread_sigmask (0, NULL, &mask); - sigdelset (&mask, SIGCHLD); + _Jv_UnBlockSigchld(); + ProcessManagerInternal *pmi = (ProcessManagerInternal *)nativeData; - // Use sigsuspend() instead of sigwait() as sigwait() doesn't play - // nicely with the GC's use of signals. - sigsuspend (&mask); + // Try to read multiple (64) notifications in one go. + char c[64]; + ::read(pmi->pipe_ends[0], c, sizeof (c)); - // Do not check sigsuspend return value. The only legitimate return - // is EINTR, but there is a known kernel bug affecting alpha-linux - // wrt sigsuspend+handler+sigreturn that can result in a return value - // of __NR_sigsuspend and errno unset. Don't fail unnecessarily on - // older kernel versions. + _Jv_BlockSigchld(); - // All OK. return; } -jboolean java::lang::PosixProcess$ProcessManager::reap () +jboolean java::lang::PosixProcess$ProcessManager::reap (PosixProcess *p) { - using namespace java::lang; + pid_t rv; - pid_t pid; + // Try to get the return code from the child process. + int status; + rv = ::waitpid ((pid_t)p->pid, &status, WNOHANG); + if (rv == -1) + throw new InternalError (JvNewStringUTF (strerror (errno))); - for (;;) - { - // Get the return code from a dead child process. - int status; - pid = waitpid ((pid_t) - 1, &status, WNOHANG); - if (pid == -1) - { - if (errno == ECHILD) - return false; - else - goto error; - } + if (rv == 0) + return false; // No children to wait for. - if (pid == 0) - return true; // No children to wait for. - - // Look up the process in our pid map. - PosixProcess * process = removeProcessFromMap ((jlong) pid); - - // Note that if process==NULL, then we have an unknown child. - // This is not common, but can happen, and isn't an error. - if (process) - { - JvSynchronize sync (process); - process->status = WIFEXITED (status) ? WEXITSTATUS (status) : -1; - process->state = PosixProcess::STATE_TERMINATED; - process->processTerminationCleanup(); - process->notifyAll (); - } - } - -error: - throw new InternalError (JvNewStringUTF (strerror (errno))); + JvSynchronize sync (p); + p->status = WIFEXITED (status) ? WEXITSTATUS (status) : -1; + p->state = PosixProcess::STATE_TERMINATED; + p->processTerminationCleanup(); + p->notifyAll (); + return true; } void java::lang::PosixProcess$ProcessManager::signalReaper () { - int c = pthread_kill ((pthread_t) reaperPID, SIGCHLD); - if (c == 0) - return; - // pthread_kill() failed. - throw new InternalError (JvNewStringUTF (strerror (c))); + ProcessManagerInternal *pmi = (ProcessManagerInternal *)nativeData; + char c = 0; + ::write(pmi->pipe_ends[1], &c, 1); + // Ignore errors. If EPIPE the reaper has already exited. } void java::lang::PosixProcess::nativeDestroy () { - int c = kill ((pid_t) pid, SIGKILL); + int c = ::kill ((pid_t) pid, SIGKILL); if (c == 0) return; // kill() failed. @@ -427,9 +440,9 @@ java::lang::PosixProcess::nativeSpawn () char c; int r = read (msgp[0], &c, 1); if (r == -1) - throw new IOException (JvNewStringUTF (strerror (errno))); + throw new IOException (JvNewStringUTF (strerror (errno))); else if (r != 0) - throw new IOException (JvNewStringUTF (strerror (c))); + throw new IOException (JvNewStringUTF (strerror (c))); } catch (java::lang::Throwable *thrown) { diff --git a/libjava/posix-threads.cc b/libjava/posix-threads.cc index 6ea724b3be8..287d6b79a6f 100644 --- a/libjava/posix-threads.cc +++ b/libjava/posix-threads.cc @@ -14,6 +14,7 @@ details. */ #include #include "posix.h" +#include "posix-threads.h" // If we're using the Boehm GC, then we need to override some of the // thread primitives. This is fairly gross. @@ -472,8 +473,8 @@ handle_intr (int) // Do nothing. } -static void -block_sigchld() +void +_Jv_BlockSigchld() { sigset_t mask; sigemptyset (&mask); @@ -483,6 +484,17 @@ block_sigchld() JvFail (strerror (c)); } +void +_Jv_UnBlockSigchld() +{ + sigset_t mask; + sigemptyset (&mask); + sigaddset (&mask, SIGCHLD); + int c = pthread_sigmask (SIG_UNBLOCK, &mask, NULL); + if (c != 0) + JvFail (strerror (c)); +} + void _Jv_InitThreads (void) { @@ -501,7 +513,7 @@ _Jv_InitThreads (void) // Block SIGCHLD here to ensure that any non-Java threads inherit the new // signal mask. - block_sigchld(); + _Jv_BlockSigchld(); // Check/set the thread stack size. size_t min_ss = 32 * 1024; @@ -581,7 +593,7 @@ _Jv_ThreadRegister (_Jv_Thread_t *data) } # endif // Block SIGCHLD which is used in natPosixProcess.cc. - block_sigchld(); + _Jv_BlockSigchld(); } void @@ -629,7 +641,7 @@ _Jv_ThreadStart (java::lang::Thread *thread, _Jv_Thread_t *data, // Block SIGCHLD which is used in natPosixProcess.cc. // The current mask is inherited by the child thread. - block_sigchld(); + _Jv_BlockSigchld(); param.sched_priority = thread->getPriority();