Fix profile dumping race.

Fix a race that caused a non-critical assertion failure.  To trigger the
race, a thread had to be part way through initializing a new sample,
such that it was discoverable by the dumping thread, but not yet linked
into its gctx by the time a later dump phase would normally have reset
its state to 'nominal'.

Additionally, lock access to the state field during modification to
transition to the dumping state.  It's not apparent that this oversight
could have caused an actual problem due to outer locking that protects
the dumping machinery, but the added locking pedantically follows the
stated locking protocol for the state field.
This commit is contained in:
Jason Evans 2014-09-24 22:14:21 -07:00
parent eb5376ab9e
commit 6ef80d68f0
2 changed files with 10 additions and 1 deletions

View File

@ -79,6 +79,7 @@ struct prof_cnt_s {
}; };
typedef enum { typedef enum {
prof_tctx_state_initializing,
prof_tctx_state_nominal, prof_tctx_state_nominal,
prof_tctx_state_dumping, prof_tctx_state_dumping,
prof_tctx_state_purgatory /* Dumper must finish destroying. */ prof_tctx_state_purgatory /* Dumper must finish destroying. */

View File

@ -717,7 +717,7 @@ prof_lookup(tsd_t *tsd, prof_bt_t *bt)
memset(&ret.p->cnts, 0, sizeof(prof_cnt_t)); memset(&ret.p->cnts, 0, sizeof(prof_cnt_t));
ret.p->gctx = gctx; ret.p->gctx = gctx;
ret.p->prepared = true; ret.p->prepared = true;
ret.p->state = prof_tctx_state_nominal; ret.p->state = prof_tctx_state_initializing;
malloc_mutex_lock(tdata->lock); malloc_mutex_lock(tdata->lock);
error = ckh_insert(tsd, &tdata->bt2tctx, btkey, ret.v); error = ckh_insert(tsd, &tdata->bt2tctx, btkey, ret.v);
malloc_mutex_unlock(tdata->lock); malloc_mutex_unlock(tdata->lock);
@ -728,6 +728,7 @@ prof_lookup(tsd_t *tsd, prof_bt_t *bt)
return (NULL); return (NULL);
} }
malloc_mutex_lock(gctx->lock); malloc_mutex_lock(gctx->lock);
ret.p->state = prof_tctx_state_nominal;
tctx_tree_insert(&gctx->tctxs, ret.p); tctx_tree_insert(&gctx->tctxs, ret.p);
gctx->nlimbo--; gctx->nlimbo--;
malloc_mutex_unlock(gctx->lock); malloc_mutex_unlock(gctx->lock);
@ -925,8 +926,15 @@ static void
prof_tctx_merge_tdata(prof_tctx_t *tctx, prof_tdata_t *tdata) prof_tctx_merge_tdata(prof_tctx_t *tctx, prof_tdata_t *tdata)
{ {
malloc_mutex_lock(tctx->gctx->lock);
if (tctx->state == prof_tctx_state_initializing) {
malloc_mutex_unlock(tctx->gctx->lock);
return;
}
assert(tctx->state == prof_tctx_state_nominal); assert(tctx->state == prof_tctx_state_nominal);
tctx->state = prof_tctx_state_dumping; tctx->state = prof_tctx_state_dumping;
malloc_mutex_unlock(tctx->gctx->lock);
memcpy(&tctx->dump_cnts, &tctx->cnts, sizeof(prof_cnt_t)); memcpy(&tctx->dump_cnts, &tctx->cnts, sizeof(prof_cnt_t));
tdata->cnt_summed.curobjs += tctx->dump_cnts.curobjs; tdata->cnt_summed.curobjs += tctx->dump_cnts.curobjs;