2009-05-21 Hans Boehm <Hans.Boehm@hp.com> (really Ivan Maidanski)

	* win32_threads.c (GC_new_thread): Make first_thread
	visible to the whole file.
	(UNPROTECT): New macro.
	(GC_push_stack_for, GC_suspend, GC_start_world): unprotect
	thread structures before writing.
	(GC_suspend): Acquire GC_fault_handler_lock before suspending
	thread.
	* os_dep.c: export GC_fault_handler_lock.
	(GC_remove_protection): Check if already unprotected.

diff --git a/ChangeLog b/ChangeLog
index d01f3ac1aa6638e68c45dd5d4b767c76b196f23f..7969f42ff9e3049ad2f8e411ddf479482f787f2a 100644 (file)
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,14 @@
+2009-05-21  Hans Boehm <Hans.Boehm@hp.com> (really Ivan Maidanski)
+       * win32_threads.c (GC_new_thread): Make first_thread
+       visible to the whole file.
+       (UNPROTECT): New macro.
+       (GC_push_stack_for, GC_suspend, GC_start_world): unprotect
+       thread structures before writing.
+       (GC_suspend): Acquire GC_fault_handler_lock before suspending
+       thread.
+       * os_dep.c: export GC_fault_handler_lock.
+       (GC_remove_protection): Check if already unprotected.
+
 2009-05-20  Hans Boehm <Hans.Boehm@hp.com> (really Ivan Maidanski)
        * doc/README.win32: Add OpenWatcom warning.
        * include/private/gcconfig.h: Really check it in.

diff --git a/os_dep.c b/os_dep.c
index c0c03294764ad3c4579f7e16293690fee0edf724..97c272e3facc10c888c486dbec897b58e9d8459b 100644 (file)
--- a/os_dep.c
+++ b/os_dep.c
@@ -2594,13 +2594,13 @@ STATIC GC_bool GC_old_segv_handler_used_si;
 /* Contention should be very rare, so we do the minimum to handle it   */
 /* correctly.                                                          */
 #ifdef AO_HAVE_test_and_set_acquire
-  static volatile AO_TS_t fault_handler_lock = 0;
+  volatile AO_TS_t GC_fault_handler_lock = 0;
   void async_set_pht_entry_from_index(volatile page_hash_table db, size_t index) {
-    while (AO_test_and_set_acquire(&fault_handler_lock) == AO_TS_SET) {}
+    while (AO_test_and_set_acquire(&GC_fault_handler_lock) == AO_TS_SET) {}
     /* Could also revert to set_pht_entry_from_index_safe if initial   */
     /* GC_test_and_set fails.                                          */
     set_pht_entry_from_index(db, index);
-    AO_CLEAR(&fault_handler_lock);
+    AO_CLEAR(&GC_fault_handler_lock);
   }
 #else /* !AO_have_test_and_set_acquire */
 # error No test_and_set operation: Introduces a race.
@@ -2804,9 +2804,13 @@ void GC_remove_protection(struct hblk *h, word nblocks, GC_bool is_ptrfree)
     h_trunc = (struct hblk *)((word)h & ~(GC_page_size-1));
     h_end = (struct hblk *)(((word)(h + nblocks) + GC_page_size-1)
                            & ~(GC_page_size-1));
+    if (h_end == h_trunc + 1 &&
+        get_pht_entry_from_index(GC_dirty_pages, PHT_HASH(h_trunc))) {
+       /* already marked dirty, and hence unprotected. */
+       return;
+    }
     for (current = h_trunc; current < h_end; ++current) {
-        size_t index = PHT_HASH(current);
-            
+        size_t index = PHT_HASH(h_trunc);
         if (!is_ptrfree || current < h || current >= h + nblocks) {
             async_set_pht_entry_from_index(GC_dirty_pages, index);
         }

diff --git a/win32_threads.c b/win32_threads.c
index 19318144f9c97893acdf1dd07d9b9540689df5da..1bbbb6ac458382ef52346610570c87abef387822 100644 (file)
--- a/win32_threads.c
+++ b/win32_threads.c
@@ -279,6 +279,10 @@ GC_bool GC_started_thread_while_stopped(void)
 #endif
   STATIC GC_thread GC_threads[THREAD_TABLE_SZ];
   
+  /* It may not be safe to allocate when we register the first thread. */
+  /* Thus we allocated one statically.                                 */
+  static struct GC_Thread_Rep first_thread;
+  static GC_bool first_thread_used = FALSE;
 
 /* Add a thread to GC_threads.  We assume it wasn't already there.     */
 /* Caller holds allocation lock.                                       */
@@ -287,9 +291,6 @@ STATIC GC_thread GC_new_thread(DWORD id)
 {
     word hv = ((word)id) % THREAD_TABLE_SZ;
     GC_thread result;
-    /* It may not be safe to allocate when we register the first thread. */
-    static struct GC_Thread_Rep first_thread;
-    static GC_bool first_thread_used = FALSE;
     
     GC_ASSERT(I_HOLD_LOCK());
     if (!first_thread_used) {
@@ -501,6 +502,22 @@ static GC_thread GC_lookup_thread(DWORD thread_id)
   }
 }
 
+/* Make sure thread descriptor t is not protected by the VDB           */
+/* implementation.                                                     */
+/* Used to prevent write faults when the world is (partially) stopped, */
+/* since it may have been stopped with a system lock held, and that    */
+/* lock may be required for fault handling.                            */
+# if defined(MPROTECT_VDB) && !defined(MSWINCE)
+#    define UNPROTECT(t) \
+       if (GC_dirty_maintained && !GC_win32_dll_threads && \
+           t != &first_thread) { \
+         GC_ASSERT(SMALL_OBJ(GC_size(t))); \
+         GC_remove_protection(HBLKPTR(t), 1, FALSE); \
+       }
+# else
+#    define UNPROTECT(p)
+# endif
+
 /* If a thread has been joined, but we have not yet            */
 /* been notified, then there may be more than one thread       */
 /* in the table with the same win32 id.                                */
@@ -709,6 +726,8 @@ void GC_push_thread_structures(void)
 # endif
 }
 
+extern volatile AO_TS_t GC_fault_handler_lock;  /* from os_dep.c */
+
 /* Suspend the given thread, if it's still active.     */
 void GC_suspend(GC_thread t)
 {
@@ -723,6 +742,8 @@ void GC_suspend(GC_thread t)
     /* This reduces the probability of that event, though it still     */
     /* appears there's a race here.                                    */
     DWORD exitCode; 
+
+    UNPROTECT(t);
     if (GetExitCodeThread(t -> handle, &exitCode) &&
         exitCode != STILL_ACTIVE) {
 #     ifdef GC_PTHREADS
@@ -735,10 +756,16 @@ void GC_suspend(GC_thread t)
 #     endif
       return;
     }
+    /* Acquire the spin lock we use to update dirty bits.      */
+    /* Threads shouldn't get stopped holding it.  But we may   */
+    /* acquire and release it in the UNPROTECT call.           */
+      while (AO_test_and_set_acquire(&GC_fault_handler_lock) == AO_TS_SET) {}
+
     if (SuspendThread(t -> handle) == (DWORD)-1)
       ABORT("SuspendThread failed");
 # endif
    t -> suspended = TRUE;
+   AO_CLEAR(&GC_fault_handler_lock);
 }
 
 /* Defined in misc.c */
@@ -829,6 +856,7 @@ void GC_start_world(void)
            && t -> id != thread_id) {
           if (ResumeThread(t -> handle) == (DWORD)-1)
            ABORT("ResumeThread failed");
+         UNPROTECT(t);
           t -> suspended = FALSE;
         }
       }
@@ -950,6 +978,7 @@ STATIC void GC_push_stack_for(GC_thread thread)
       /* of large stacks.                                              */
       if (thread -> last_stack_min == ADDR_LIMIT) {
        stack_min = GC_get_stack_min(thread -> stack_base);
+        UNPROTECT(thread);
         thread -> last_stack_min = stack_min;
       } else {
        if (sp < thread -> stack_base && sp >= thread -> last_stack_min) {
@@ -965,6 +994,7 @@ STATIC void GC_push_stack_for(GC_thread thread)
              stack_min = GC_get_stack_min(thread -> stack_base);
            }
 #        endif
+          UNPROTECT(thread);
           thread -> last_stack_min = stack_min;
        }
       }