From 62a71a870f3e37669a301a727153c51929626ac1 Mon Sep 17 00:00:00 2001 From: Erik Boasson Date: Sat, 20 Apr 2019 10:28:33 +0200 Subject: [PATCH] fix race: delete reader & delete writer (#159) 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 --- src/core/ddsi/src/q_entity.c | 12 ++++-------- 1 file changed, 4 insertions(+), 8 deletions(-) diff --git a/src/core/ddsi/src/q_entity.c b/src/core/ddsi/src/q_entity.c index 0b0c8fa..c832a9b 100644 --- a/src/core/ddsi/src/q_entity.c +++ b/src/core/ddsi/src/q_entity.c @@ -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) { 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); 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); if (m->in_sync != PRMSS_SYNC) - { pwr->n_readers_out_of_sync--; - } + if (rd->reliable) + pwr->n_reliable_readers--; + local_reader_ary_remove (&pwr->rdary, rd); } - if (rd->reliable) - { - pwr->n_reliable_readers--; - } - local_reader_ary_remove (&pwr->rdary, rd); ddsrt_mutex_unlock (&pwr->e.lock); if (m != NULL) {