From: ivmai Date: Wed, 16 Sep 2009 10:27:22 +0000 (+0000) Subject: 2009-09-16 Ivan Maidanski X-Git-Tag: gc7_2alpha4~127 X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=048b93ba0fc96aa54293b5e82b3c973c3eb28144;p=gc 2009-09-16 Ivan Maidanski (ivmai128.diff - superseding diff62, diff66 partly) * finalize.c (GC_general_register_disappearing_link): Return GC_SUCCESS, GC_DUPLICATE, GC_NO_MEMORY (instead of 0, 1 and 2, respectively). * include/gc.h (GC_NO_MEMORY): New macro (defined as 2). * include/gc.h (GC_register_disappearing_link, GC_general_register_disappearing_link): Update the comment. * typd_mlc.c (GC_calloc_explicitly_typed): Use GC_NO_MEMORY macro. * finalize.c (GC_general_register_disappearing_link, GC_register_finalizer_inner): Recalculate the hash table index after GC_oom_fn succeeded (since the table may grow while not holding the lock) and check again that the entry is still not in the table (free the unused entry otherwise unless DBG_HDRS_ALL). * finalize.c (GC_register_finalizer_inner): Initialize "hhdr" local variable (to prevent a compiler warning). * finalize.c (GC_register_finalizer_inner): Don't modify the data pointed by "ocd" and "ofn" in GC_register_finalizer_inner() failed (due to out of memory). --- diff --git a/ChangeLog b/ChangeLog index 26d52caa..fcf30e8a 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,3 +1,24 @@ +2009-09-16 Ivan Maidanski + (ivmai128.diff - superseding diff62, diff66 partly) + + * finalize.c (GC_general_register_disappearing_link): Return + GC_SUCCESS, GC_DUPLICATE, GC_NO_MEMORY (instead of 0, 1 and 2, + respectively). + * include/gc.h (GC_NO_MEMORY): New macro (defined as 2). + * include/gc.h (GC_register_disappearing_link, + GC_general_register_disappearing_link): Update the comment. + * typd_mlc.c (GC_calloc_explicitly_typed): Use GC_NO_MEMORY macro. + * finalize.c (GC_general_register_disappearing_link, + GC_register_finalizer_inner): Recalculate the hash table index + after GC_oom_fn succeeded (since the table may grow while not + holding the lock) and check again that the entry is still not in + the table (free the unused entry otherwise unless DBG_HDRS_ALL). + * finalize.c (GC_register_finalizer_inner): Initialize "hhdr" + local variable (to prevent a compiler warning). + * finalize.c (GC_register_finalizer_inner): Don't modify the data + pointed by "ocd" and "ofn" in GC_register_finalizer_inner() failed + (due to out of memory). + 2009-09-16 Ivan Maidanski (ivmai124.diff - superseding diff67 partly) diff --git a/finalize.c b/finalize.c index 287030a1..607e8bdd 100644 --- a/finalize.c +++ b/finalize.c @@ -170,7 +170,7 @@ GC_API int GC_CALL GC_general_register_disappearing_link(void * * link, if (curr_dl -> dl_hidden_link == HIDE_POINTER(link)) { curr_dl -> dl_hidden_obj = HIDE_POINTER(obj); UNLOCK(); - return(1); + return GC_DUPLICATE; } } new_dl = (struct disappearing_link *) @@ -181,10 +181,24 @@ GC_API int GC_CALL GC_general_register_disappearing_link(void * * link, new_dl = (struct disappearing_link *) (*oom_fn)(sizeof(struct disappearing_link)); if (0 == new_dl) { - return(2); + return GC_NO_MEMORY; } /* It's not likely we'll make it here, but ... */ LOCK(); + /* Recalculate index since the table may grow. */ + index = HASH2(link, log_dl_table_size); + /* Check again that our disappearing link not in the table. */ + for (curr_dl = dl_head[index]; curr_dl != 0; curr_dl = dl_next(curr_dl)) { + if (curr_dl -> dl_hidden_link == HIDE_POINTER(link)) { + curr_dl -> dl_hidden_obj = HIDE_POINTER(obj); + UNLOCK(); +# ifndef DBG_HDRS_ALL + /* Free unused new_dl returned by GC_oom_fn() */ + GC_free((void *)new_dl); +# endif + return GC_DUPLICATE; + } + } } new_dl -> dl_hidden_obj = HIDE_POINTER(obj); new_dl -> dl_hidden_link = HIDE_POINTER(link); @@ -192,7 +206,7 @@ GC_API int GC_CALL GC_general_register_disappearing_link(void * * link, dl_head[index] = new_dl; GC_dl_entries++; UNLOCK(); - return(0); + return GC_SUCCESS; } GC_API int GC_CALL GC_unregister_disappearing_link(void * * link) @@ -297,8 +311,8 @@ STATIC void GC_register_finalizer_inner(void * obj, ptr_t base; struct finalizable_object * curr_fo, * prev_fo; size_t index; - struct finalizable_object *new_fo; - hdr *hhdr; + struct finalizable_object *new_fo = 0; + hdr *hhdr = NULL; /* initialized to prevent warning. */ GC_oom_func oom_fn; DCL_LOCK_STATE; @@ -314,75 +328,93 @@ STATIC void GC_register_finalizer_inner(void * obj, } /* in the THREADS case we hold allocation lock. */ base = (ptr_t)obj; - index = HASH2(base, log_fo_table_size); - prev_fo = 0; curr_fo = fo_head[index]; - while (curr_fo != 0) { - GC_ASSERT(GC_size(curr_fo) >= sizeof(struct finalizable_object)); - if (curr_fo -> fo_hidden_base == HIDE_POINTER(base)) { - /* Interruption by a signal in the middle of this */ - /* should be safe. The client may see only *ocd */ - /* updated, but we'll declare that to be his */ - /* problem. */ - if (ocd) *ocd = (void *) (curr_fo -> fo_client_data); - if (ofn) *ofn = curr_fo -> fo_fn; - /* Delete the structure for base. */ - if (prev_fo == 0) { - fo_head[index] = fo_next(curr_fo); - } else { - fo_set_next(prev_fo, fo_next(curr_fo)); - } - if (fn == 0) { - GC_fo_entries--; - /* May not happen if we get a signal. But a high */ - /* estimate will only make the table larger than */ - /* necessary. */ -# if !defined(THREADS) && !defined(DBG_HDRS_ALL) - GC_free((void *)curr_fo); -# endif - } else { - curr_fo -> fo_fn = fn; - curr_fo -> fo_client_data = (ptr_t)cd; - curr_fo -> fo_mark_proc = mp; - /* Reinsert it. We deleted it first to maintain */ - /* consistency in the event of a signal. */ - if (prev_fo == 0) { - fo_head[index] = curr_fo; - } else { - fo_set_next(prev_fo, curr_fo); - } - } - UNLOCK(); - return; - } - prev_fo = curr_fo; - curr_fo = fo_next(curr_fo); - } - if (ofn) *ofn = 0; - if (ocd) *ocd = 0; - if (fn == 0) { + for (;;) { + index = HASH2(base, log_fo_table_size); + prev_fo = 0; curr_fo = fo_head[index]; + while (curr_fo != 0) { + GC_ASSERT(GC_size(curr_fo) >= sizeof(struct finalizable_object)); + if (curr_fo -> fo_hidden_base == HIDE_POINTER(base)) { + /* Interruption by a signal in the middle of this */ + /* should be safe. The client may see only *ocd */ + /* updated, but we'll declare that to be his problem. */ + if (ocd) *ocd = (void *) (curr_fo -> fo_client_data); + if (ofn) *ofn = curr_fo -> fo_fn; + /* Delete the structure for base. */ + if (prev_fo == 0) { + fo_head[index] = fo_next(curr_fo); + } else { + fo_set_next(prev_fo, fo_next(curr_fo)); + } + if (fn == 0) { + GC_fo_entries--; + /* May not happen if we get a signal. But a high */ + /* estimate will only make the table larger than */ + /* necessary. */ +# if !defined(THREADS) && !defined(DBG_HDRS_ALL) + GC_free((void *)curr_fo); +# endif + } else { + curr_fo -> fo_fn = fn; + curr_fo -> fo_client_data = (ptr_t)cd; + curr_fo -> fo_mark_proc = mp; + /* Reinsert it. We deleted it first to maintain */ + /* consistency in the event of a signal. */ + if (prev_fo == 0) { + fo_head[index] = curr_fo; + } else { + fo_set_next(prev_fo, curr_fo); + } + } + UNLOCK(); +# ifndef DBG_HDRS_ALL + if (EXPECT(new_fo != 0, FALSE)) { + /* Free unused new_fo returned by GC_oom_fn() */ + GC_free((void *)new_fo); + } +# endif + return; + } + prev_fo = curr_fo; + curr_fo = fo_next(curr_fo); + } + if (EXPECT(new_fo != 0, FALSE)) { + /* new_fo is returned GC_oom_fn(), so fn != 0 and hhdr != 0. */ + break; + } + if (fn == 0) { + if (ocd) *ocd = 0; + if (ofn) *ofn = 0; UNLOCK(); - return; - } - GET_HDR(base, hhdr); - if (0 == hhdr) { - /* We won't collect it, hence finalizer wouldn't be run. */ - UNLOCK(); - return; - } - new_fo = (struct finalizable_object *) + return; + } + GET_HDR(base, hhdr); + if (EXPECT(0 == hhdr, FALSE)) { + /* We won't collect it, hence finalizer wouldn't be run. */ + if (ocd) *ocd = 0; + if (ofn) *ofn = 0; + UNLOCK(); + return; + } + new_fo = (struct finalizable_object *) GC_INTERNAL_MALLOC(sizeof(struct finalizable_object),NORMAL); - if (EXPECT(0 == new_fo, FALSE)) { + if (EXPECT(new_fo != 0, TRUE)) + break; oom_fn = GC_oom_fn; UNLOCK(); new_fo = (struct finalizable_object *) (*oom_fn)(sizeof(struct finalizable_object)); if (0 == new_fo) { + /* No enough memory. *ocd and *ofn remains unchanged. */ return; } /* It's not likely we'll make it here, but ... */ LOCK(); + /* Recalculate index since the table may grow and */ + /* check again that our finalizer is not in the table. */ } GC_ASSERT(GC_size(new_fo) >= sizeof(struct finalizable_object)); + if (ocd) *ocd = 0; + if (ofn) *ofn = 0; new_fo -> fo_hidden_base = (word)HIDE_POINTER(base); new_fo -> fo_fn = fn; new_fo -> fo_client_data = (ptr_t)cd; diff --git a/include/gc.h b/include/gc.h index 7fcd9647..57a6be52 100644 --- a/include/gc.h +++ b/include/gc.h @@ -845,6 +845,8 @@ GC_API void GC_CALL GC_debug_register_finalizer_unreachable(void * /* obj */, GC_finalization_proc /* fn */, void * /* cd */, GC_finalization_proc * /* ofn */, void ** /* ocd */); +#define GC_NO_MEMORY 2 /* Failure due to lack of memory. */ + /* The following routine may be used to break cycles between */ /* finalizable objects, thus causing cyclic finalizable */ /* objects to be finalized in the correct order. Standard */ @@ -867,9 +869,9 @@ GC_API int GC_CALL GC_register_disappearing_link(void ** /* link */); /* There's an argument that an arbitrary action should */ /* be allowed here, instead of just clearing a pointer. */ /* But this causes problems if that action alters, or */ - /* examines connectivity. */ - /* Returns 1 if link was already registered, 0 if */ - /* registration succeeded, 2 if it failed for lack of */ + /* examines connectivity. Returns GC_DUPLICATE if link */ + /* was already registered, GC_SUCCESS if registration */ + /* succeeded, GC_NO_MEMORY if it failed for lack of */ /* memory, and GC_oom_fn did not handle the problem. */ /* Only exists for backward compatibility. See below: */ @@ -900,7 +902,11 @@ GC_API int GC_CALL GC_general_register_disappearing_link(void ** /* link */, /* pointer is accessed. Otherwise a strong pointer */ /* could be recreated between the time the collector */ /* decides to reclaim the object and the link is */ - /* cleared. */ + /* cleared. Returns GC_SUCCESS if registration */ + /* succeeded (a new link is registered), GC_DUPLICATE */ + /* if link was already registered (with some object), */ + /* GC_NO_MEMORY if registration failed for lack of */ + /* memory (and GC_oom_fn did not handle the problem). */ GC_API int GC_CALL GC_unregister_disappearing_link(void ** /* link */); /* Undoes a registration by either of the above two */ diff --git a/typd_mlc.c b/typd_mlc.c index ded8b2d0..048fd726 100644 --- a/typd_mlc.c +++ b/typd_mlc.c @@ -721,7 +721,7 @@ DCL_LOCK_STATE; /* Make sure the descriptor is cleared once there is any danger */ /* it may have been collected. */ if (GC_general_register_disappearing_link((void * *)((word *)op+lw-1), - op) == 2) { + op) == GC_NO_MEMORY) { /* Couldn't register it due to lack of memory. Punt. */ /* This will probably fail too, but gives the recovery code */ /* a chance. */