(a cherry-pick of commits
7305e5619,
0c0e4cd0b,
d6db3f005 from 'master')
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 (clear_pht_entry_from_index,
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):
If GC_manual_vdb then 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 [NACL] (GC_suspend_all): If GC_manual_vdb then
call GC_acquire_dirty_lock() and GC_release_dirty_lock() around the
code which ensures parking of threads.
* 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.
* pthread_support.c [CAN_HANDLE_FORK] (fork_prepare_proc): Call
GC_acquire_dirty_lock().
* pthread_support.c [CAN_HANDLE_FORK] (fork_parent_proc,
fork_child_proc): Call GC_release_dirty_lock().
* 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)
-# define clear_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 */
GC_wait_for_reclaim();
# endif
+ if (GC_manual_vdb) {
+ /* See the relevant comment in GC_stop_world. */
+ GC_acquire_dirty_lock();
+ }
+ /* Else do not acquire the lock as the write fault handler might */
+ /* be trying to acquire this lock too, and the suspend handler */
+ /* execution is deferred until the write fault handler completes. */
+
/* TODO: Support GC_retry_signals (not needed for TSan) */
switch (RAISE_SIGNAL(t, GC_sig_suspend)) {
/* ESRCH cannot happen as terminated threads are handled above. */
if (errno != EINTR)
ABORT("sem_wait for handler failed (suspend_self)");
}
+ if (GC_manual_vdb)
+ 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:
GC_nacl_thread_parker = pthread_self();
GC_nacl_park_threads_now = 1;
+ if (GC_manual_vdb)
+ GC_acquire_dirty_lock();
while (1) {
int num_threads_parked = 0;
struct timespec ts;
num_sleeps = 0;
}
}
+ if (GC_manual_vdb)
+ GC_release_dirty_lock();
# endif /* NACL */
return n_live_threads;
}
# 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
if (GC_parallel)
GC_acquire_mark_lock();
# endif
+ GC_acquire_dirty_lock();
}
/* Called in parent after a fork() (even if the latter failed). */
#endif
static void fork_parent_proc(void)
{
+ GC_release_dirty_lock();
# if defined(PARALLEL_MARK)
if (GC_parallel)
GC_release_mark_lock();
#endif
static void fork_child_proc(void)
{
- /* Clean up the thread table, so that just our thread is left. */
+ GC_release_dirty_lock();
# if defined(PARALLEL_MARK)
if (GC_parallel)
GC_release_mark_lock();
# endif
+ /* Clean up the thread table, so that just our thread is left. */
GC_remove_all_threads_but_me();
# ifdef PARALLEL_MARK
/* Turn off parallel marking in the child, since we are probably */
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));
}