Re: [Fwd: Possible bug in Thread.start()]

Chris Pickett wrote:
> Etienne Gagnon wrote:
>> -------- Original Message --------
>> Subject: Possible bug in Thread.start()
>> Date: Mon, 13 Dec 2004 17:38:47 -0600
>> From: Archie Cobbs <[email protected]>
>> To: old mailing-list
>>
>> Hello Sablevm folks,
>>
>> I recently discovered a bug in JC that may also apply to SableVM,
>> because JC's thread handling is based on SableVM.
>>
>> The bug is a race condition between thread #1 stopping the world
>> and thread #2 creating a new thread via Thread.start().
> 
> Thanks Archie, I've chased bugs related to thread startup in the GC 
> stuff before (threads will reach the "impossible control flow" case), 
> but never managed to nail it.

Well hopefully this is it. By the way, here is the test program
that I was using to trigger the bug in JC. Maybe you can try it
with SableVM "before and after" the bug fix.

Also, the actual fix I used was different from what I said in the
email, which was incorrect. You need to call halt_if_requested()
right before creating the new thread and adding it to the list of
live threads, while holding the mutex, not in _svmf_thread_start().
See attached patch.

The point is to avoid adding new threads in the RUNNING_JAVA state
to the linked list until you're sure that no other thread is trying
to halt the world.

-Archie

__________________________________________________________________________
Archie Cobbs      *        CTO, Awarix        *      http://www.awarix.com

public class memtest extends Thread {
         public static int NTHDS = 5;
         static memtest[] all = new memtest[NTHDS];
         byte[] a1;
         byte[] a2;
         byte[] a3;
         Object[] o1;
         Random r;
         boolean doit;
         public memtest(boolean doit) {
                 this.doit = doit;
                 r = new Random();
                 a1 = new byte[(r.nextInt() & 0x7fffffff) % 5000];
                 if (r.nextBoolean())
                         a2 = new byte[(r.nextInt() & 0x7fffffff) % 100];
                 a3 = new byte[(r.nextInt() & 0x7fffffff) % 7];
                 if (r.nextBoolean())
                         o1 = new Object[] { r, getClass().getClassLoader() };
         }
         public void run() {
                 while (doit) {
                         try {
                                 Thread.sleep((r.nextInt() & 0x7fffffff) % 500);
                                 if (r.nextBoolean()) {
                                     int i = (r.nextInt() & 0x7fffffff) % NTHDS;
                                     if (all[i] != null)
                                             all[i].interrupt();
                                 } else
                                         new memtest(false).start();
                         } catch (InterruptedException e) {
                         }
                 }
         }
     /*
         protected void finalize() {
                 if (r.nextBoolean())
                         o1 = new Object[] { this };
         }
     */
         public static void main(String[] args) throws Exception {
                 for (int i = 0; i < NTHDS; i++)
                         (all[i] = new memtest(true)).start();
         }
         public static native long foobar(boolean x, String[] y);
}

_______________________________________________
SableVM-devel mailing list
[email protected]
http://sablevm.org/lists/control/listinfo/sablevm-devel

Index: java_lang_VMThread.c
===================================================================
--- java_lang_VMThread.c	(revision 3166)
+++ java_lang_VMThread.c	(working copy)
@@ -98,6 +98,8 @@
 
       _svmm_mutex_lock (vm->global_mutex);
 
+      _svmf_halt_if_requested (env);
+
       if (vm->threads.free_list != NULL)
 	{
 	  new_env = vm->threads.free_list;