Skip to content

Commit bf40641

Browse files
committed
Fix a prof_tctx_t destruction race.
1 parent 155bfa7 commit bf40641

File tree

1 file changed

+32
-18
lines changed

1 file changed

+32
-18
lines changed

src/prof.c

Lines changed: 32 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -609,7 +609,7 @@ prof_tctx_destroy(tsd_t *tsd, prof_tctx_t *tctx)
609609
{
610610
prof_tdata_t *tdata = tctx->tdata;
611611
prof_gctx_t *gctx = tctx->gctx;
612-
bool destroy_tdata, destroy_gctx;
612+
bool destroy_tdata, destroy_tctx, destroy_gctx;
613613

614614
assert(tctx->cnts.curobjs == 0);
615615
assert(tctx->cnts.curbytes == 0);
@@ -622,33 +622,47 @@ prof_tctx_destroy(tsd_t *tsd, prof_tctx_t *tctx)
622622
malloc_mutex_unlock(tdata->lock);
623623

624624
malloc_mutex_lock(gctx->lock);
625-
tctx_tree_remove(&gctx->tctxs, tctx);
626-
if (prof_gctx_should_destroy(gctx)) {
625+
if (tctx->state != prof_tctx_state_dumping) {
626+
tctx_tree_remove(&gctx->tctxs, tctx);
627+
destroy_tctx = true;
628+
if (prof_gctx_should_destroy(gctx)) {
629+
/*
630+
* Increment gctx->nlimbo in order to keep another
631+
* thread from winning the race to destroy gctx while
632+
* this one has gctx->lock dropped. Without this, it
633+
* would be possible for another thread to:
634+
*
635+
* 1) Sample an allocation associated with gctx.
636+
* 2) Deallocate the sampled object.
637+
* 3) Successfully prof_gctx_try_destroy(gctx).
638+
*
639+
* The result would be that gctx no longer exists by the
640+
* time this thread accesses it in
641+
* prof_gctx_try_destroy().
642+
*/
643+
gctx->nlimbo++;
644+
destroy_gctx = true;
645+
} else
646+
destroy_gctx = false;
647+
} else {
627648
/*
628-
* Increment gctx->nlimbo in order to keep another thread from
629-
* winning the race to destroy gctx while this one has
630-
* gctx->lock dropped. Without this, it would be possible for
631-
* another thread to:
632-
*
633-
* 1) Sample an allocation associated with gctx.
634-
* 2) Deallocate the sampled object.
635-
* 3) Successfully prof_gctx_try_destroy(gctx).
636-
*
637-
* The result would be that gctx no longer exists by the time
638-
* this thread accesses it in prof_gctx_try_destroy().
649+
* A dumping thread needs tctx to remain valid until dumping
650+
* has finished. Change state such that the dumping thread will
651+
* complete destruction during a late dump iteration phase.
639652
*/
640-
gctx->nlimbo++;
641-
destroy_gctx = true;
642-
} else
653+
tctx->state = prof_tctx_state_purgatory;
654+
destroy_tctx = false;
643655
destroy_gctx = false;
656+
}
644657
malloc_mutex_unlock(gctx->lock);
645658
if (destroy_gctx)
646659
prof_gctx_try_destroy(tsd, gctx, tdata);
647660

648661
if (destroy_tdata)
649662
prof_tdata_destroy(tsd, tdata, false);
650663

651-
idalloc(tsd, tctx);
664+
if (destroy_tctx)
665+
idalloc(tsd, tctx);
652666
}
653667

654668
static bool

0 commit comments

Comments
 (0)