From e7ba62e0de0d0c42c3732c7b7e82a92e679648a9 Mon Sep 17 00:00:00 2001 From: Ivan Maidanski Date: Wed, 16 May 2018 11:33:02 +0300 Subject: [PATCH] Add assertions to finalize and threads support for MANUAL_VDB needs (code refactoring) * finalize.c (GC_unregister_disappearing_link_inner, GC_enqueue_all_finalizers): Remove redundant comment about held lock. * finalize.c (GC_unregister_disappearing_link_inner, GC_move_disappearing_link_inner, ITERATE_DL_HASHTBL_BEGIN, GC_finalize, GC_enqueue_all_finalizers): Add assertion that the allocation lock is held. * pthread_support.c (first_thread): Refine comment. * pthread_support.c (GC_new_thread): Add assertion that when the first thread is added then GC_threads[hv] is null (thus "next" field is set to null). * win32_threads.c (GC_new_thread): Likewise. * pthread_support.c (GC_delete_thread, GC_delete_gc_thread): Add assertion that "next" field of the first thread is not modified. * win32_threads.c (GC_delete_gc_thread_no_free, GC_delete_thread): Likewise. * win32_threads.c (GC_delete_thread): Use EXPECT() when checking that p is not the first thread. * win32_threads.c [GC_PTHREADS] (GC_pthread_start_inner): Add assertion that the current thread is not the first one (thus "status" field of the first thread is not modified). --- finalize.c | 10 +++++++--- pthread_support.c | 4 ++++ win32_threads.c | 6 +++++- 3 files changed, 16 insertions(+), 4 deletions(-) diff --git a/finalize.c b/finalize.c index cefdf601..1fc7a2ed 100644 --- a/finalize.c +++ b/finalize.c @@ -256,7 +256,6 @@ GC_API int GC_CALL GC_general_register_disappearing_link(void * * link, #endif /* Unregisters given link and returns the link entry to free. */ -/* Assume the lock is held. */ GC_INLINE struct disappearing_link *GC_unregister_disappearing_link_inner( struct dl_hashtbl_s *dl_hashtbl, void **link) { @@ -264,6 +263,7 @@ GC_INLINE struct disappearing_link *GC_unregister_disappearing_link_inner( struct disappearing_link *prev_dl = NULL; size_t index; + GC_ASSERT(I_HOLD_LOCK()); if (dl_hashtbl->log_size == -1) return NULL; /* prevent integer shift by a negative amount */ @@ -531,6 +531,7 @@ GC_API GC_await_finalize_proc GC_CALL GC_get_await_finalize_proc(void) word curr_hidden_link; word new_hidden_link; + GC_ASSERT(I_HOLD_LOCK()); if (dl_hashtbl->log_size == -1) return GC_NOT_FOUND; /* prevent integer shift by a negative amount */ @@ -921,6 +922,7 @@ GC_API void GC_CALL GC_register_finalizer_unreachable(void * obj, size_t i; \ size_t dl_size = dl_hashtbl->log_size == -1 ? 0 : \ (size_t)1 << dl_hashtbl->log_size; \ + GC_ASSERT(I_HOLD_LOCK()); \ for (i = 0; i < dl_size; i++) { \ struct disappearing_link *prev_dl = NULL; \ curr_dl = dl_hashtbl -> head[i]; \ @@ -991,6 +993,7 @@ GC_INNER void GC_finalize(void) size_t fo_size = log_fo_table_size == -1 ? 0 : (size_t)1 << log_fo_table_size; + GC_ASSERT(I_HOLD_LOCK()); # ifndef SMALL_CONFIG /* Save current GC_[dl/ll]_entries value for stats printing */ GC_old_dl_entries = GC_dl_hashtbl.entries; @@ -1140,13 +1143,14 @@ GC_INNER void GC_finalize(void) #ifndef JAVA_FINALIZATION_NOT_NEEDED - /* Enqueue all remaining finalizers to be run - Assumes lock is held. */ + /* Enqueue all remaining finalizers to be run. */ STATIC void GC_enqueue_all_finalizers(void) { struct finalizable_object * next_fo; int i; int fo_size; + GC_ASSERT(I_HOLD_LOCK()); fo_size = log_fo_table_size == -1 ? 0 : 1 << log_fo_table_size; GC_bytes_finalized = 0; for (i = 0; i < fo_size; i++) { @@ -1172,7 +1176,7 @@ GC_INNER void GC_finalize(void) GC_bytes_finalized += curr_fo -> fo_object_size + sizeof(struct finalizable_object); curr_fo = next_fo; - } + } } GC_fo_entries = 0; /* all entries deleted from the hash table */ } diff --git a/pthread_support.c b/pthread_support.c index 34b8c762..2c114319 100644 --- a/pthread_support.c +++ b/pthread_support.c @@ -522,6 +522,7 @@ void GC_push_thread_structures(void) #endif /* DEBUG_THREADS */ /* It may not be safe to allocate when we register the first thread. */ +/* As "next" and "status" fields are unused, no need to push this. */ static struct GC_Thread_Rep first_thread; /* Add a thread to GC_threads. We assume it wasn't already there. */ @@ -544,6 +545,7 @@ STATIC GC_thread GC_new_thread(pthread_t id) if (!EXPECT(first_thread_used, TRUE)) { result = &first_thread; first_thread_used = TRUE; + GC_ASSERT(NULL == GC_threads[hv]); # if defined(THREAD_SANITIZER) && defined(CPPCHECK) GC_noop1(result->dummy[0]); # endif @@ -593,6 +595,7 @@ STATIC void GC_delete_thread(pthread_t id) if (prev == 0) { GC_threads[hv] = p -> next; } else { + GC_ASSERT(prev != &first_thread); prev -> next = p -> next; } if (p != &first_thread) { @@ -622,6 +625,7 @@ STATIC void GC_delete_gc_thread(GC_thread t) if (prev == 0) { GC_threads[hv] = p -> next; } else { + GC_ASSERT(prev != &first_thread); prev -> next = p -> next; } # ifdef GC_DARWIN_THREADS diff --git a/win32_threads.c b/win32_threads.c index 58845e25..5598bd16 100644 --- a/win32_threads.c +++ b/win32_threads.c @@ -346,6 +346,7 @@ STATIC GC_thread GC_new_thread(DWORD id) if (!EXPECT(first_thread_used, TRUE)) { result = &first_thread; first_thread_used = TRUE; + GC_ASSERT(NULL == GC_threads[hv]); } else { GC_ASSERT(!GC_win32_dll_threads); result = (struct GC_Thread_Rep *) @@ -680,6 +681,7 @@ STATIC void GC_delete_gc_thread_no_free(GC_vthread t) if (prev == 0) { GC_threads[hv] = p -> tm.next; } else { + GC_ASSERT(prev != &first_thread); prev -> tm.next = p -> tm.next; } } @@ -717,9 +719,10 @@ STATIC void GC_delete_thread(DWORD id) if (prev == 0) { GC_threads[hv] = p -> tm.next; } else { + GC_ASSERT(prev != &first_thread); prev -> tm.next = p -> tm.next; } - if (p != &first_thread) { + if (EXPECT(p != &first_thread, TRUE)) { GC_INTERNAL_FREE(p); } } @@ -2649,6 +2652,7 @@ GC_INNER void GC_thr_init(void) /* we don't need to hold the allocation lock during pthread_create. */ me = GC_register_my_thread_inner(sb, thread_id); SET_PTHREAD_MAP_CACHE(pthread_id, thread_id); + GC_ASSERT(me != &first_thread); me -> pthread_id = pthread_id; if (si->detached) me -> flags |= DETACHED; UNLOCK(); -- 2.40.0