Re: Patch: initialize ProcessManager early

[David Daney <ddaney at avtrex dot com>]

Tom Tromey wrote:
> > > > > > "David" == David Daney <ddaney@avtrex.com> writes:
> > > > > >             
>
> David> New patch attached (still untested).
>
> If it works, it is fine by me.
>
> David> -	InheritableThreadLocal.newChildThread(this);
> David> +        if (!noInheritableThreadLocal)
> David> +          InheritableThreadLocal.newChildThread(this);
>
> A comment here explaining why would be useful.
> You could lift the comment from my patch.
>
>   
This is what I committed after regression testing on x86_64-pc-linux-gnu 
(FC6):

2007-02-16  David Daney  <ddaney@avtrex.com>

   * java/lang/Thread.java (Thread(ThreadGroup, Runnable, String)): Pass
   new parameter constructor.
   (Thread(ThreadGroup, Runnable, String, long)): Same.
   (Thread(String, boolean)): New constructor.
   (Thread(Thread, ThreadGroup, Runnable, String): Add parameter
   noInheritableThreadLocal, don't call
   InheritableThreadLocal.newChildThread if set.
   * java/lang/PosixProcess.java(ProcessManager()): Set
   noInheritableThreadLocal in super.
   * java/lang/natThread.cc (_Jv_AttachCurrentThread): Pass new
   parameter to Thread constructor.
   (_Jv_AttachCurrentThreadAsDaemon): Same.
   * java/lang/Thread.h: Regenerate.
   * classpath/lib/java/lang/Thread.class: Same.
   * classpath/lib/java/lang/PosixProcess$EOFInputStream.class: Same.
   * classpath/lib/java/lang/PosixProcess.class: Same.
   * classpath/lib/java/lang/Thread$State.class: Same.
   * classpath/lib/java/lang/PosixProcess$ProcessManager.class: Same.

Index: classpath/lib/java/lang/Thread.class
===================================================================
Cannot display: file marked as a binary type.
svn:mime-type = application/octet-stream
Index: classpath/lib/java/lang/PosixProcess$EOFInputStream.class
===================================================================
Cannot display: file marked as a binary type.
svn:mime-type = application/octet-stream
Index: classpath/lib/java/lang/PosixProcess.class
===================================================================
Cannot display: file marked as a binary type.
svn:mime-type = application/octet-stream
Index: classpath/lib/java/lang/Thread$State.class
===================================================================
Cannot display: file marked as a binary type.
svn:mime-type = application/octet-stream
Index: classpath/lib/java/lang/PosixProcess$ProcessManager.class
===================================================================
Cannot display: file marked as a binary type.
svn:mime-type = application/octet-stream
Index: java/lang/natThread.cc
===================================================================
--- java/lang/natThread.cc	(revision 122047)
+++ java/lang/natThread.cc	(working copy)
@@ -490,7 +490,7 @@ _Jv_AttachCurrentThread(jstring name, ja
     return thread;
   if (name == NULL)
     name = java::lang::Thread::gen_name ();
-  thread = new java::lang::Thread (NULL, group, NULL, name);
+  thread = new java::lang::Thread (NULL, group, NULL, name, false);
   _Jv_AttachCurrentThread (thread);
   _Jv_NotifyThreadStart (thread);
   return thread;
@@ -504,7 +504,7 @@ _Jv_AttachCurrentThreadAsDaemon(jstring 
     return thread;
   if (name == NULL)
     name = java::lang::Thread::gen_name ();
-  thread = new java::lang::Thread (NULL, group, NULL, name);
+  thread = new java::lang::Thread (NULL, group, NULL, name, false);
   thread->setDaemon (true);
   _Jv_AttachCurrentThread (thread);
   _Jv_NotifyThreadStart (thread);
Index: java/lang/Thread.h
===================================================================
--- java/lang/Thread.h	(revision 122047)
+++ java/lang/Thread.h	(working copy)
@@ -53,8 +53,10 @@ public:
   Thread(::java::lang::Runnable *, ::java::lang::String *);
   Thread(::java::lang::ThreadGroup *, ::java::lang::Runnable *, ::java::lang::String *);
   Thread(::java::lang::ThreadGroup *, ::java::lang::Runnable *, ::java::lang::String *, jlong);
+public: // actually package-private
+  Thread(::java::lang::String *, jboolean);
 private:
-  Thread(::java::lang::Thread *, ::java::lang::ThreadGroup *, ::java::lang::Runnable *, ::java::lang::String *);
+  Thread(::java::lang::Thread *, ::java::lang::ThreadGroup *, ::java::lang::Runnable *, ::java::lang::String *, jboolean);
 public:
   static jint activeCount();
   virtual void checkAccess();
Index: java/lang/PosixProcess.java
===================================================================
--- java/lang/PosixProcess.java	(revision 122047)
+++ java/lang/PosixProcess.java	(working copy)
@@ -42,7 +42,12 @@ final class PosixProcess extends Process
 
     ProcessManager()
     {
-      super("ProcessManager");
+      // Use package private Thread constructor to place us in the
+      // root ThreadGroup with no InheritableThreadLocal.  If the
+      // InheritableThreadLocals were allowed to initialize, they could
+      // cause a Runtime.exec() to be called causing infinite
+      // recursion.
+      super("ProcessManager", true);
       // Don't keep the (main) process from exiting on our account.
       this.setDaemon(true);
     }
Index: java/lang/Thread.java
===================================================================
--- java/lang/Thread.java	(revision 122047)
+++ java/lang/Thread.java	(working copy)
@@ -355,7 +355,7 @@ public class Thread implements Runnable
    */
   public Thread(ThreadGroup group, Runnable target, String name)
   {
-    this(currentThread(), group, target, name);
+    this(currentThread(), group, target, name, false);
   }
 
   /**
@@ -381,10 +381,26 @@ public class Thread implements Runnable
   public Thread(ThreadGroup group, Runnable target, String name, long size)
   {
     // Just ignore stackSize for now.
-    this(currentThread(), group, target, name);
+    this(currentThread(), group, target, name, false);
   }
 
-  private Thread (Thread current, ThreadGroup g, Runnable r, String n)
+  /**
+   * Allocate a new Thread object for threads used internally to the
+   * run time.  Runtime threads should not be members of an
+   * application ThreadGroup, nor should they execute arbitrary user
+   * code as part of the InheritableThreadLocal protocol.
+   *
+   * @param name the name for the Thread
+   * @param noInheritableThreadLocal if true, do not initialize
+   * InheritableThreadLocal variables for this thread.
+   * @throws IllegalThreadStateException if group is destroyed
+   */
+  Thread(String name, boolean noInheritableThreadLocal)
+  {
+    this(null, null, null, name, noInheritableThreadLocal);
+  }
+  
+  private Thread (Thread current, ThreadGroup g, Runnable r, String n, boolean noInheritableThreadLocal)
   {
     // Make sure the current thread may create a new thread.
     checkAccess();
@@ -424,7 +440,10 @@ public class Thread implements Runnable
 	int pri = current.getPriority();
 	priority = (gmax < pri ? gmax : pri);
 	contextClassLoader = current.contextClassLoader;
-	InheritableThreadLocal.newChildThread(this);
+        // InheritableThreadLocal allows arbitrary user code to be
+        // executed, only do this if our caller desires it.
+        if (!noInheritableThreadLocal)
+          InheritableThreadLocal.newChildThread(this);
       }
     else
       {