Issue #235 (bdwgc).
* darwin_stop_world.c [!GC_NO_THREADS_DISCOVERY]
(GC_suspend_thread_list): Call GC_acquire_dirty_lock and
GC_release_dirty_lock around thread_suspend().
* darwin_stop_world.c (GC_stop_world): Likewise.
* include/private/gc_priv.h (set_pht_entry_from_index_safe): Remove
unused macro.
* include/private/gc_priv.h [THREADS] (GC_acquire_dirty_lock,
GC_release_dirty_lock): Define new macro; move comment from
win32_threads.c.
* include/private/gc_priv.h [THREADS && !GC_DISABLE_INCREMENTAL]
(GC_fault_handler_lock): Declare global variable (even if not
MPROTECT_VDB).
* os_dep.c [!GC_DISABLE_INCREMENTAL] (async_set_pht_entry_from_index):
Define even if not MPROTECT_VDB; reformat comments.
* os_dep.c [!GC_DISABLE_INCREMENTAL && AO_HAVE_test_and_set_acquire]
(GC_fault_handler_lock): Likewise.
* os_dep.c [THREADS && AO_HAVE_test_and_set_acquire]
(async_set_pht_entry_from_index): Use GC_acquire_dirty_lock and
GC_release_dirty_lock; remove wrong comment.
* os_dep.c [THREADS && !AO_HAVE_test_and_set_acquire]
(currently_updating, async_set_pht_entry_from_index): Remove incorrect
implementation.
* os_dep.c [!GC_DISABLE_INCREMENTAL] (GC_dirty_inner): Call
async_set_pht_entry_from_index instead of set_pht_entry_from_index;
remove FIXME.
* pthread_stop_world.c [GC_ENABLE_SUSPEND_THREAD] (GC_suspend_thread):
Refine comment for AO_store_release call; call GC_acquire_dirty_lock
and GC_release_dirty_lock around a block of RAISE_SIGNAL() and
sem_wait().
* pthread_stop_world.c [GC_OPENBSD_UTHREADS] (GC_suspend_all): Call
GC_acquire_dirty_lock and GC_release_dirty_lock around
pthread_suspend_np().
* pthread_stop_world.c [!GC_OPENBSD_UTHREADS] (GC_suspend_all): Add
comment.
* pthread_stop_world.c [!GC_OPENBSD_UTHREADS && !NACL] (GC_stop_world):
If GC_manual_vdb then call GC_acquire_dirty_lock and
GC_release_dirty_lock around a block of GC_suspend_all() and
suspend_restart_barrier(); add comment.
* win32_threads.c (GC_suspend): Use GC_acquire_dirty_lock and
GC_release_dirty_lock (regardless of MPROTECT_VDB).
# endif
/* Unconditionally suspend the thread. It will do no */
/* harm if it is already suspended by the client logic. */
+ GC_acquire_dirty_lock();
do {
kern_result = thread_suspend(thread);
} while (kern_result == KERN_ABORTED);
+ GC_release_dirty_lock();
if (kern_result != KERN_SUCCESS) {
/* The thread may have quit since the thread_threads() call we */
/* mark already suspended so it's not dealt with anymore later. */
for (p = GC_threads[i]; p != NULL; p = p->next) {
if ((p->flags & FINISHED) == 0 && !p->thread_blocked &&
p->stop_info.mach_thread != my_thread) {
-
+ GC_acquire_dirty_lock();
do {
kern_result = thread_suspend(p->stop_info.mach_thread);
} while (kern_result == KERN_ABORTED);
+ GC_release_dirty_lock();
if (kern_result != KERN_SUCCESS)
ABORT("thread_suspend failed");
if (GC_on_thread_event)
(((bl)[divWORDSZ(index)] >> modWORDSZ(index)) & 1)
# define set_pht_entry_from_index(bl, index) \
(bl)[divWORDSZ(index)] |= (word)1 << modWORDSZ(index)
-/* And a dumb but thread-safe version of set_pht_entry_from_index. */
-/* This sets (many) extra bits. */
-# define set_pht_entry_from_index_safe(bl, index) \
- (bl)[divWORDSZ(index)] = ONES
/* And, one more version for GC_add_to_black_list_normal/stack. */
/* The latter ones are invoked (indirectly) by GC_do_local_mark. */
/* protected by GC_write_cs. */
# endif
-# ifdef MPROTECT_VDB
+# if defined(GC_DISABLE_INCREMENTAL)
+# define GC_acquire_dirty_lock() (void)0
+# define GC_release_dirty_lock() (void)0
+# else
+ /* Acquire the spin lock we use to update dirty bits. */
+ /* Threads should not get stopped holding it. But we may */
+ /* acquire and release it during GC_remove_protection call. */
+# define GC_acquire_dirty_lock() \
+ do { /* empty */ \
+ } while (AO_test_and_set_acquire(&GC_fault_handler_lock) == AO_TS_SET)
+# define GC_release_dirty_lock() AO_CLEAR(&GC_fault_handler_lock)
GC_EXTERN volatile AO_TS_t GC_fault_handler_lock;
/* defined in os_dep.c */
# endif
}
#endif /* DEFAULT_VDB */
+#ifndef GC_DISABLE_INCREMENTAL
+# ifndef THREADS
+# define async_set_pht_entry_from_index(db, index) \
+ set_pht_entry_from_index(db, index)
+# elif defined(AO_HAVE_test_and_set_acquire)
+ /* We need to lock around the bitmap update (in the write fault */
+ /* handler or GC_dirty) in order to avoid the risk of losing a bit. */
+ /* We do this with a test-and-set spin lock if possible. */
+ GC_INNER volatile AO_TS_t GC_fault_handler_lock = AO_TS_INITIALIZER;
+
+ GC_ATTR_NO_SANITIZE_THREAD
+ static void async_set_pht_entry_from_index(volatile page_hash_table db,
+ size_t index)
+ {
+ GC_acquire_dirty_lock();
+ set_pht_entry_from_index(db, index);
+ GC_release_dirty_lock();
+ }
+# else
+# error No test_and_set operation: Introduces a race.
+# endif /* THREADS && !AO_HAVE_test_and_set_acquire */
+#endif /* !GC_DISABLE_INCREMENTAL */
+
#ifdef MPROTECT_VDB
/*
* This implementation maintains dirty bits itself by catching write
# endif /* !MSWIN32 */
#endif /* !DARWIN */
-#if defined(THREADS)
-/* We need to lock around the bitmap update in the write fault handler */
-/* in order to avoid the risk of losing a bit. We do this with a */
-/* test-and-set spin lock if we know how to do that. Otherwise we */
-/* check whether we are already in the handler and use the dumb but */
-/* safe fallback algorithm of setting all bits in the word. */
-/* Contention should be very rare, so we do the minimum to handle it */
-/* correctly. */
-#ifdef AO_HAVE_test_and_set_acquire
- GC_INNER volatile AO_TS_t GC_fault_handler_lock = AO_TS_INITIALIZER;
-
- GC_ATTR_NO_SANITIZE_THREAD
- static void async_set_pht_entry_from_index(volatile page_hash_table db,
- size_t index)
- {
- while (AO_test_and_set_acquire(&GC_fault_handler_lock) == AO_TS_SET) {
- /* empty */
- }
- /* 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(&GC_fault_handler_lock);
- }
-#else /* !AO_HAVE_test_and_set_acquire */
-# error No test_and_set operation: Introduces a race.
- /* THIS WOULD BE INCORRECT! */
- /* The dirty bit vector may be temporarily wrong, */
- /* just before we notice the conflict and correct it. We may end up */
- /* looking at it while it's wrong. But this requires contention */
- /* exactly when a GC is triggered, which seems far less likely to */
- /* fail than the old code, which had no reported failures. Thus we */
- /* leave it this way while we think of something better, or support */
- /* GC_test_and_set on the remaining platforms. */
- static int * volatile currently_updating = 0;
- static void async_set_pht_entry_from_index(volatile page_hash_table db,
- size_t index)
- {
- int update_dummy;
- currently_updating = &update_dummy;
- set_pht_entry_from_index(db, index);
- /* If we get contention in the 10 or so instruction window here, */
- /* and we get stopped by a GC between the two updates, we lose! */
- if (currently_updating != &update_dummy) {
- set_pht_entry_from_index_safe(db, index);
- /* We claim that if two threads concurrently try to update the */
- /* dirty bit vector, the first one to execute UPDATE_START */
- /* will see it changed when UPDATE_END is executed. (Note that */
- /* &update_dummy must differ in two distinct threads.) It */
- /* will then execute set_pht_entry_from_index_safe, thus */
- /* returning us to a safe state, though not soon enough. */
- }
- }
-#endif /* !AO_HAVE_test_and_set_acquire */
-
+#ifdef THREADS
/* This function is used only by the fault handler. Potential data */
/* race between this function and GC_install_header, GC_remove_header */
/* should not be harmful because the added or removed header should */
return HDR_INNER(addr) != NULL;
# endif
}
-
-#else /* !THREADS */
-# define async_set_pht_entry_from_index(db, index) \
- set_pht_entry_from_index(db, index)
+#else
# define is_header_found_async(addr) (HDR(addr) != NULL)
#endif /* !THREADS */
/* page unprotection. */
GC_ASSERT(GC_manual_vdb);
# endif
- set_pht_entry_from_index(GC_dirty_pages, index); /* FIXME: concurrent */
+ async_set_pht_entry_from_index(GC_dirty_pages, index);
}
/* Retrieve system dirty bits for the heap to a local buffer (unless */
}
/* Set the flag making the change visible to the signal handler. */
+ /* This also removes the protection for t object, preventing */
+ /* write faults in GC_store_stack_ptr (thus double-locking should */
+ /* not occur in async_set_pht_entry_from_index). */
AO_store_release(&t->suspended_ext, TRUE);
if (THREAD_EQUAL((pthread_t)thread, pthread_self())) {
# endif
/* TODO: Support GC_retry_signals (not needed for TSan) */
+ GC_acquire_dirty_lock();
switch (RAISE_SIGNAL(t, GC_sig_suspend)) {
/* ESRCH cannot happen as terminated threads are handled above. */
case 0:
if (errno != EINTR)
ABORT("sem_wait for handler failed (suspend_self)");
}
+ GC_release_dirty_lock();
RESTORE_CANCEL(cancel_state);
UNLOCK();
}
# ifdef GC_OPENBSD_UTHREADS
{
stack_t stack;
+
+ GC_acquire_dirty_lock();
if (pthread_suspend_np(p -> id) != 0)
ABORT("pthread_suspend_np failed");
+ GC_release_dirty_lock();
if (pthread_stackseg_np(p->id, &stack))
ABORT("pthread_stackseg_np failed");
p -> stop_info.stack_ptr = (ptr_t)stack.ss_sp - stack.ss_size;
(void *)p->id);
}
# else
+ /* The synchronization between GC_dirty (based on */
+ /* test-and-set) and the signal-based thread suspension */
+ /* is performed in GC_stop_world because */
+ /* GC_release_dirty_lock cannot be called before */
+ /* acknowledging the thread is really suspended. */
result = RAISE_SIGNAL(p, GC_sig_suspend);
switch(result) {
case ESRCH:
# else
AO_store(&GC_stop_count, (AO_t)((word)GC_stop_count + 2));
/* Only concurrent reads are possible. */
+ if (GC_manual_vdb) {
+ GC_acquire_dirty_lock();
+ /* The write fault handler cannot be called if GC_manual_vdb */
+ /* (thus double-locking should not occur in */
+ /* async_set_pht_entry_from_index based on test-and-set). */
+ }
AO_store_release(&GC_world_is_stopped, TRUE);
n_live_threads = GC_suspend_all();
if (GC_retry_signals)
n_live_threads = resend_lost_signals(n_live_threads, GC_suspend_all);
suspend_restart_barrier(n_live_threads);
+ if (GC_manual_vdb)
+ GC_release_dirty_lock(); /* cannot be done in GC_suspend_all */
# endif
# ifdef PARALLEL_MARK
return;
}
# endif
-# if defined(MPROTECT_VDB)
- /* 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_THREAD call. */
- while (AO_test_and_set_acquire(&GC_fault_handler_lock) == AO_TS_SET) {
- /* empty */
- }
-# endif
-
+ GC_acquire_dirty_lock();
# ifdef MSWINCE
/* SuspendThread() will fail if thread is running kernel code. */
while (SuspendThread(THREAD_HANDLE(t)) == (DWORD)-1)
ABORT("SuspendThread failed");
# endif /* !MSWINCE */
t -> suspended = (unsigned char)TRUE;
-# if defined(MPROTECT_VDB)
- AO_CLEAR(&GC_fault_handler_lock);
-# endif
+ GC_release_dirty_lock();
if (GC_on_thread_event)
GC_on_thread_event(GC_EVENT_THREAD_SUSPENDED, THREAD_HANDLE(t));
}
projects / gc / commitdiff