diff options
author | Ivan Maidanski <ivmai@mail.ru> | 2019-02-27 01:32:53 +0300 |
---|---|---|
committer | Ivan Maidanski <ivmai@mail.ru> | 2019-02-27 01:32:53 +0300 |
commit | 95845da3ccee911a2f86dcb5842f704478fa236e (patch) | |
tree | 8bb8b8603bca0b4f710c194e456cef86ffabb488 /reclaim.c | |
parent | 4199c59c4ceb53f00f87b6d8a4eaa45db78358cf (diff) | |
download | bdwgc-95845da3ccee911a2f86dcb5842f704478fa236e.tar.gz |
Avoid potential race in hb_sz access between realloc and reclaim_block
Issue #240 (bdwgc).
GC_realloc might be changing the block size while GC_reclaim_block
is examining it. The change to the size field is benign, i.e.
GC_reclaim would work correctly with either value, since we are not
changing the number of objects in the block. But seeing a half-updated
value (though unlikely to occur in practice) could be probably bad.
Using unordered atomic fetch of hb_sz field should solve the issue.
* reclaim.c (GC_block_nearly_full, GC_reclaim_small_nonempty_block):
Add sz argument; use sz instead of hhdr->hb_sz.
* reclaim.c (GC_reclaim_clear, GC_reclaim_uninit, GC_reclaim_check):
Skip the assertion about hhdr->hb_sz if THREADS.
* reclaim.c [ENABLE_DISCLAIM] (GC_disclaim_and_reclaim): Likewise.
* reclaim.c [AO_HAVE_load] (GC_reclaim_block): Use AO_load to access
hhdr->hb_sz; add comments.
* reclaim.c (GC_reclaim_block): Pass sz to
GC_reclaim_small_nonempty_block() and GC_block_nearly_full().
* reclaim.c (GC_continue_reclaim, GC_reclaim_all): Pass hhdr->hb_sz to
GC_reclaim_small_nonempty_block().
* reclaim.c [!EAGER_SWEEP && ENABLE_DISCLAIM]
(GC_reclaim_unconditionally_marked): Likewise.
Diffstat (limited to 'reclaim.c')
-rw-r--r-- | reclaim.c | 48 |
1 files changed, 33 insertions, 15 deletions
@@ -135,9 +135,9 @@ GC_INNER GC_bool GC_block_empty(hdr *hhdr) return (hhdr -> hb_n_marks == 0); } -STATIC GC_bool GC_block_nearly_full(hdr *hhdr) +STATIC GC_bool GC_block_nearly_full(hdr *hhdr, word sz) { - return (hhdr -> hb_n_marks > 7 * HBLK_OBJS(hhdr -> hb_sz)/8); + return hhdr -> hb_n_marks > HBLK_OBJS(sz) * 7 / 8; } /* TODO: This should perhaps again be specialized for USE_MARK_BYTES */ @@ -156,7 +156,11 @@ STATIC ptr_t GC_reclaim_clear(struct hblk *hbp, hdr *hhdr, word sz, signed_word n_bytes_found = 0; GC_ASSERT(hhdr == GC_find_header((ptr_t)hbp)); - GC_ASSERT(sz == hhdr -> hb_sz); +# ifndef THREADS + GC_ASSERT(sz == hhdr -> hb_sz); +# else + /* Skip the assertion because of a potential race with GC_realloc. */ +# endif GC_ASSERT((sz & (BYTES_PER_WORD-1)) == 0); p = (word *)(hbp->hb_body); plim = (word *)(hbp->hb_body + HBLKSIZE - sz); @@ -202,7 +206,9 @@ STATIC ptr_t GC_reclaim_uninit(struct hblk *hbp, hdr *hhdr, word sz, word *p, *plim; signed_word n_bytes_found = 0; - GC_ASSERT(sz == hhdr -> hb_sz); +# ifndef THREADS + GC_ASSERT(sz == hhdr -> hb_sz); +# endif p = (word *)(hbp->hb_body); plim = (word *)((ptr_t)hbp + HBLKSIZE - sz); @@ -233,7 +239,9 @@ STATIC ptr_t GC_reclaim_uninit(struct hblk *hbp, hdr *hhdr, word sz, struct obj_kind *ok = &GC_obj_kinds[hhdr->hb_obj_kind]; int (GC_CALLBACK *disclaim)(void *) = ok->ok_disclaim_proc; - GC_ASSERT(sz == hhdr -> hb_sz); +# ifndef THREADS + GC_ASSERT(sz == hhdr -> hb_sz); +# endif p = (word *)(hbp -> hb_body); plim = (word *)((ptr_t)p + HBLKSIZE - sz); @@ -281,8 +289,10 @@ STATIC void GC_reclaim_check(struct hblk *hbp, hdr *hhdr, word sz) { word bit_no; ptr_t p, plim; - GC_ASSERT(sz == hhdr -> hb_sz); +# ifndef THREADS + GC_ASSERT(sz == hhdr -> hb_sz); +# endif /* go through all words in block */ p = hbp->hb_body; plim = p + HBLKSIZE - sz; @@ -340,11 +350,10 @@ GC_INNER ptr_t GC_reclaim_generic(struct hblk * hbp, hdr *hhdr, size_t sz, * If entirely empty blocks are to be completely deallocated, then * caller should perform that check. */ -STATIC void GC_reclaim_small_nonempty_block(struct hblk *hbp, +STATIC void GC_reclaim_small_nonempty_block(struct hblk *hbp, word sz, GC_bool report_if_found) { hdr *hhdr = HDR(hbp); - word sz = hhdr -> hb_sz; struct obj_kind * ok = &GC_obj_kinds[hhdr -> hb_obj_kind]; void **flh = &(ok -> ok_freelist[BYTES_TO_GRANULES(sz)]); @@ -390,9 +399,16 @@ STATIC void GC_reclaim_small_nonempty_block(struct hblk *hbp, STATIC void GC_reclaim_block(struct hblk *hbp, word report_if_found) { hdr * hhdr = HDR(hbp); - word sz = hhdr -> hb_sz; /* size of objects in current block */ + word sz; /* size of objects in current block */ struct obj_kind * ok = &GC_obj_kinds[hhdr -> hb_obj_kind]; +# ifdef AO_HAVE_load + /* Atomic access is used to avoid racing with GC_realloc. */ + sz = (word)AO_load((volatile AO_t *)&hhdr->hb_sz); +# else + /* No race as GC_realloc holds the lock while updating hb_sz. */ + sz = hhdr -> hb_sz; +# endif if( sz > MAXOBJBYTES ) { /* 1 big object */ if( !mark_bit_from_hdr(hhdr, 0) ) { if (report_if_found) { @@ -440,7 +456,8 @@ STATIC void GC_reclaim_block(struct hblk *hbp, word report_if_found) GC_ASSERT(sz * hhdr -> hb_n_marks <= HBLKSIZE); # endif if (report_if_found) { - GC_reclaim_small_nonempty_block(hbp, TRUE /* report_if_found */); + GC_reclaim_small_nonempty_block(hbp, sz, + TRUE /* report_if_found */); } else if (empty) { # ifdef ENABLE_DISCLAIM if ((hhdr -> hb_flags & HAS_DISCLAIM) != 0) { @@ -451,7 +468,7 @@ STATIC void GC_reclaim_block(struct hblk *hbp, word report_if_found) GC_bytes_found += HBLKSIZE; GC_freehblk(hbp); } - } else if (GC_find_leak || !GC_block_nearly_full(hhdr)) { + } else if (GC_find_leak || !GC_block_nearly_full(hhdr, sz)) { /* group of smaller objects, enqueue the real work */ struct hblk **rlh = ok -> ok_reclaim_list; @@ -698,8 +715,9 @@ GC_INNER void GC_continue_reclaim(word sz /* granules */, int kind) while ((hbp = *rlh) != 0) { hhdr = HDR(hbp); *rlh = hhdr -> hb_next; - GC_reclaim_small_nonempty_block(hbp, FALSE); - if (*flh != 0) break; + GC_reclaim_small_nonempty_block(hbp, hhdr -> hb_sz, FALSE); + if (*flh != 0) + break; } } @@ -745,7 +763,7 @@ GC_INNER GC_bool GC_reclaim_all(GC_stop_func stop_func, GC_bool ignore_old) /* It's likely we'll need it this time, too */ /* It's been touched recently, so this */ /* shouldn't trigger paging. */ - GC_reclaim_small_nonempty_block(hbp, FALSE); + GC_reclaim_small_nonempty_block(hbp, hhdr->hb_sz, FALSE); } } } @@ -791,7 +809,7 @@ GC_INNER GC_bool GC_reclaim_all(GC_stop_func stop_func, GC_bool ignore_old) while ((hbp = *rlh) != 0) { hhdr = HDR(hbp); *rlh = hhdr->hb_next; - GC_reclaim_small_nonempty_block(hbp, FALSE); + GC_reclaim_small_nonempty_block(hbp, hhdr->hb_sz, FALSE); } } } |