Thanks Lennart,

The actual issue here is the iterator that refer or point to the removed
element after erasing will be invalid. So I need to increase the reading
iterator to point next element before it is invalid.

Regards
Canh


-----Original Message-----
From: Lennart Lund [mailto:[email protected]] 
Sent: Thursday, December 7, 2017 8:25 PM
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

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

Reply via email to