Hi Canh Ack. I have looked at this some more and can see that this is actually used in only one thread meaning that my comment about thread safety is not valid. I have not tested.
Thanks Lennart > -----Original Message----- > From: Lennart Lund > Sent: den 7 december 2017 13:22 > To: 'Canh Van Truong' <[email protected]>; Vu Minh Nguyen > <[email protected]>; [email protected] > Cc: [email protected]; Canh Van Truong > <[email protected]> > Subject: RE: [PATCH 1/1] log: fix coredump when stop cluster [#2700] > > Hi Canh > > I am not sure about how you think here but I can see a "standard" problem > with handling of a global resource, in this case a C++ map and the problem is > simply that it is not thread safe and is handled in more than one thread. For > a > container there are two problems: > 1. > If an iterator is used for reading the content in one thread and some changes > are done in another thread when this is ongoing the iterator will be invalid > probably resulting in a segv. This can be handled by copying the container > and then create an iterator for the copy when the content shall be read. > However the copying itself is not thread safe and must be protected > 2. > If the content is changed in one thread while it is read in another may (and > probably) will create some sync problem or race condition even if the > problem with the iterator is solved by copying the container as above. > > I cannot see that the fix is solving any of these problems? > > Thanks > Lennart > > > > -----Original Message----- > > From: Canh Van Truong [mailto:[email protected]] > > Sent: den 23 november 2017 11:47 > > To: Lennart Lund <[email protected]>; Vu Minh Nguyen > > <[email protected]>; [email protected] > > Cc: [email protected]; Canh Van Truong > > <[email protected]> > > Subject: [PATCH 1/1] log: fix coredump when stop cluster [#2700] > > > > The problem happen because the iterator after erasing the element will be > > invalid. > > > > The patch fix to increase the iterator before delete client > > --- > > src/log/logd/lgs_evt.cc | 6 +++--- > > 1 file changed, 3 insertions(+), 3 deletions(-) > > > > diff --git a/src/log/logd/lgs_evt.cc b/src/log/logd/lgs_evt.cc > > index 94a94b646..c01680c49 100644 > > --- a/src/log/logd/lgs_evt.cc > > +++ b/src/log/logd/lgs_evt.cc > > @@ -333,9 +333,9 @@ int lgs_client_delete_by_mds_dest(MDS_DEST > > mds_dest, time_t *closetime_ptr) { > > TRACE_ENTER2("mds_dest %" PRIx64, mds_dest); > > /* Loop through Client DB */ > > ClientMap *clientMap(reinterpret_cast<ClientMap *>(client_db)); > > - for (const auto &value : *clientMap) { > > - rp = value.second; > > - > > + for (auto it = clientMap->begin(); it != clientMap->end(); ) { > > + rp = it->second; > > + it++; > > if (m_NCS_MDS_DEST_EQUAL(&rp->mds_dest, &mds_dest)) > > rc = lgs_client_delete(rp->client_id, closetime_ptr); > > } > > -- > > 2.13.0 ------------------------------------------------------------------------------ Check out the vibrant tech community on one of the world's most engaging tech sites, Slashdot.org! http://sdm.link/slashdot _______________________________________________ Opensaf-devel mailing list [email protected] https://lists.sourceforge.net/lists/listinfo/opensaf-devel
