niklas/cyclonedds
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

fix race: delete reader & delete writer (#159)

Browse source 
Adding and removing reader/writer matches can be done by multiple
threads, and this can result in two threads simultaneously trying to do
this on a single reader/writer pair.  The code therefore always checks
first whether the pair is (not) matched before proceeding.

However, removing a reader from a proxy writer had part of the code
outside this check.  Therefore, if both entities are being deleted
simultanously, there is a risk that local_reader_ary_remove is called
twice for the same argument, and in that case, it asserts in one of them
because the reader can no longer be found.  The counting of the number
of matched reliable readers suffers from the same race condition.

This commit eliminates these race conditions by moving these operations
into the block guarded by the aforementioned check.

Signed-off-by: Erik Boasson <eb@ilities.com>
This commit is contained in:
Erik Boasson 2019-04-20 10:28:33 +02:00 • committed by eboasson
parent 0b106cc186
commit 62a71a870f
1 changed files with 4 additions and 8 deletions
Download patch file Download diff file Expand all files Collapse all files

8
src/core/ddsi/src/q_entity.c
View file

@ -1371,8 +1371,8 @@ static void writer_drop_local_connection (const struct nn_guid *wr_guid, struct
if ((m = ut_avlLookup (&wr_local_readers_treedef, &wr->local_readers, &rd->e.guid)) != NULL) if ((m = ut_avlLookup (&wr_local_readers_treedef, &wr->local_readers, &rd->e.guid)) != NULL)
{ {
ut_avlDelete (&wr_local_readers_treedef, &wr->local_readers, m); ut_avlDelete (&wr_local_readers_treedef, &wr->local_readers, m);
}
local_reader_ary_remove (&wr->rdary, rd); local_reader_ary_remove (&wr->rdary, rd);
}
ddsrt_mutex_unlock (&wr->e.lock); ddsrt_mutex_unlock (&wr->e.lock);
if (m != NULL && wr->status_cb) if (m != NULL && wr->status_cb)
{ {
@ -1490,15 +1490,11 @@ static void proxy_writer_drop_connection (const struct nn_guid *pwr_guid, struct
{ {
ut_avlDelete (&pwr_readers_treedef, &pwr->readers, m); ut_avlDelete (&pwr_readers_treedef, &pwr->readers, m);
if (m->in_sync != PRMSS_SYNC) if (m->in_sync != PRMSS_SYNC)
{
pwr->n_readers_out_of_sync--; pwr->n_readers_out_of_sync--;
}
}
if (rd->reliable) if (rd->reliable)
{
pwr->n_reliable_readers--; pwr->n_reliable_readers--;
}
local_reader_ary_remove (&pwr->rdary, rd); local_reader_ary_remove (&pwr->rdary, rd);
}
ddsrt_mutex_unlock (&pwr->e.lock); ddsrt_mutex_unlock (&pwr->e.lock);
if (m != NULL) if (m != NULL)
{ {