re PR libgcj/29324 (add wait handling hook)

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.

From-SVN: r124638

libjava/ChangeLog

@@ -1,3 +1,46 @@
+2007-05-12  David Daney  <ddaney@avtrex.com>
+
+	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  <iant@google.com>
 
 	PR java/31842

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.
 //

libjava/java/lang/PosixProcess$ProcessManager.h

@@ -7,31 +7,37 @@
 #pragma interface
 
 #include <java/lang/Thread.h>
+extern "Java"
+{
+  namespace gnu
+  {
+    namespace gcj
+    {
+        class RawDataManaged;
+    }
+  }
+}
 
 class java::lang::PosixProcess$ProcessManager : public ::java::lang::Thread
 {
 
 public: // actually package-private
   PosixProcess$ProcessManager();
-private:
+  void addToLiveProcesses(::java::lang::PosixProcess *);
-  ::java::lang::PosixProcess * removeProcessFromMap(jlong);
+  void startExecuting(::java::lang::PosixProcess *);
-public: // actually package-private
+  void waitUntilReady();
-  virtual void addProcessToMap(::java::lang::PosixProcess *);
-  virtual void startExecuting(::java::lang::PosixProcess *);
-  virtual 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::LinkedList * __attribute__((aligned(__alignof__( ::java::lang::Thread)))) queue;
-  ::java::util::List * __attribute__((aligned(__alignof__( ::java::lang::Thread)))) queue;
+  ::java::util::LinkedList * liveProcesses;
-private:
-  ::java::util::Map * pidToProcess;
   jboolean ready;
-  jlong reaperPID;
+public: // actually package-private
+  static ::gnu::gcj::RawDataManaged * nativeData;
 public:
   static ::java::lang::Class class$;
 };

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$0();
-  static ::java::lang::Object * access$1();
+  static void access$1(::java::lang::PosixProcess$ProcessManager *);
-  static void access$2(::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;

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 <tromey@cygnus.com>
@@ -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 LinkedList<PosixProcess> queue = new LinkedList<PosixProcess>();
-    private Map pidToProcess = new HashMap();
+    private LinkedList<PosixProcess> liveProcesses =
+      new LinkedList<PosixProcess>();
     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
+     * Add a process to the list of running processes.  This must only
-     * remove it from the map.  This method is called from the
+     * be called with the queueLock held.
-     * 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.
      *
      * @param p The PosixProcess.
      */
-    void addProcessToMap(PosixProcess p)
+    void addToLiveProcesses(PosixProcess p)
     {
-      pidToProcess.put(new Long(p.pid), p);
+      liveProcesses.add(p);
     }
 
     /**
@@ -132,12 +120,19 @@ final class PosixProcess extends Process
             {
               synchronized (queueLock)
                 {
-		  boolean haveMoreChildren = reap();
+                  Iterator<PosixProcess> processIterator =
-		  if (! haveMoreChildren && queue.size() == 0)
+                    liveProcesses.iterator();
+                  while (processIterator.hasNext())
                     {
-		      // This reaper thread could exit, but we
+                      boolean reaped = reap(processIterator.next());
-		      // keep it alive for a while in case
+                      if (reaped)
-		      // someone wants to start more Processes.
+                        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);
@@ -155,16 +150,15 @@ final class PosixProcess extends Process
                     }
                   while (queue.size() > 0)
                     {
-		      PosixProcess p = (PosixProcess) queue.remove(0);
+                      PosixProcess p = queue.remove(0);
                       p.spawn(this);
                     }
                 }
 
-	      // Wait for a SIGCHLD from either an exiting
+              // Wait for a SIGCHLD from either an exiting process or
-	      // process or the startExecuting() method.  This
+              // the startExecuting() method.  This is done outside of
-	      // is done outside of the synchronized block to
+              // the synchronized block to allow other threads to
-	      // allow other threads to enter and submit more
+              // enter and submit more jobs.
-	      // jobs.
               waitForSignal();
             }
           catch (Exception ex)
@@ -176,7 +170,6 @@ final class PosixProcess extends Process
 
     /**
      * 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.
@@ -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

libjava/java/lang/natPosixProcess.cc

@@ -30,6 +30,8 @@ details.  */
 
 #include <gcj/cni.h>
 #include <jvm.h>
+#include <posix.h>
+#include <posix-threads.h>
 
 #include <java/lang/PosixProcess$ProcessManager.h>
 #include <java/lang/PosixProcess.h>
@@ -48,6 +50,7 @@ details.  */
 #include <java/lang/PosixProcess$EOFInputStream.h>
 
 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;
+  // The nativeData is static to avoid races installing the signal
-  // Remenber our PID so other threads can kill us.
+  // handler in the case that it is chained.
-  reaperPID = (jlong) pthread_self ();
+  if (nativeData == NULL )
+    {
+      ProcessManagerInternal *pmi =
+        (ProcessManagerInternal *)JvAllocBytes(sizeof(ProcessManagerInternal));
+
+      if (0 != ::pipe(pmi->pipe_ends))
+        goto error;
+
+      // 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);
+
+      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_handler = sigchld_handler;
+      sa.sa_sigaction = sigchld_handler;
       // We only want signals when the things exit.
-  sa.sa_flags = SA_NOCLDSTOP;
+      sa.sa_flags = SA_NOCLDSTOP | SA_SIGINFO;
 
-  if (-1 == sigaction (SIGCHLD, &sa, NULL))
+      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;
+  _Jv_UnBlockSigchld();
-  pthread_sigmask (0, NULL, &mask);
+  ProcessManagerInternal *pmi = (ProcessManagerInternal *)nativeData;
-  sigdelset (&mask, SIGCHLD);
 
-  // Use sigsuspend() instead of sigwait() as sigwait() doesn't play
+  // Try to read multiple (64) notifications in one go.
-  // nicely with the GC's use of signals.
+  char c[64];
-  sigsuspend (&mask);
+ ::read(pmi->pipe_ends[0], c, sizeof (c));
 
-  // Do not check sigsuspend return value.  The only legitimate return
+  _Jv_BlockSigchld();
-  // 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.
 
-  // 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.
-
-  for (;;)
-    {
-      // Get the return code from a dead child process.
   int status;
-      pid = waitpid ((pid_t) - 1, &status, WNOHANG);
+  rv = ::waitpid ((pid_t)p->pid, &status, WNOHANG);
-      if (pid == -1)
+  if (rv == -1)
-	{
-	  if (errno == ECHILD)
-	    return false;
-	  else
-	    goto error;
-	}
-
-      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)));
+
+  if (rv == 0)
+    return false;   // No children to wait for.
+
+  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);
+  ProcessManagerInternal *pmi = (ProcessManagerInternal *)nativeData;
-  if (c == 0)
+  char c = 0;
-    return;
+  ::write(pmi->pipe_ends[1], &c, 1);
-  // pthread_kill() failed.
+  // Ignore errors.  If EPIPE the reaper has already exited.
-  throw new InternalError (JvNewStringUTF (strerror (c)));
 }
 
 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.

libjava/posix-threads.cc

@@ -14,6 +14,7 @@ details.  */
 #include <config.h>
 
 #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
+void
-block_sigchld()
+_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();