> -----Original Message----- > From: Anders Björnerstedt > Sent: den 29 augusti 2014 10:10 > To: Hans Feldt > Cc: [email protected] > Subject: RE: [PATCH 1 of 1] imma: add missing lock when traversing client db > from mds thread [#1033] > > Nack. > > Just returning on failure to obtain the lock does not seem like a good > solution. [Hans] Yeah just did some copy paste meaning it is wrong in many places around this location.
> The stale marking of the clients will then just silently be skipped. > This will cause unpredicatable behavior towards the user. > > Instead you need a retry loop with small delay and if *that* fails then I > suggest > Go ahead and stale mark anyway. > > The alternative is to assert and crash the user process. [Hans] Yes I will do that, this is a should not fail situation. In the core library there are now some lock_ordie utilities that Anders W did. Using them would be a better solution but that requires the use of pthread mutexes and not leap ones. > > /AndersBj > > -----Original Message----- > From: Hans Feldt > Sent: den 29 augusti 2014 10:02 > To: Anders Björnerstedt > Cc: [email protected] > Subject: [PATCH 1 of 1] imma: add missing lock when traversing client db from > mds thread [#1033] > > osaf/libs/agents/saf/imma/imma_db.c | 8 +++++++- > 1 files changed, 7 insertions(+), 1 deletions(-) > > > diff --git a/osaf/libs/agents/saf/imma/imma_db.c > b/osaf/libs/agents/saf/imma/imma_db.c > --- a/osaf/libs/agents/saf/imma/imma_db.c > +++ b/osaf/libs/agents/saf/imma/imma_db.c > @@ -676,13 +676,17 @@ void imma_mark_clients_stale(IMMA_CB *cb > ******************************************************************************/ > void imma_process_stale_clients(IMMA_CB *cb) { > - /* We are LOCKED already */ > IMMA_CLIENT_NODE * clnode; > SaImmHandleT *temp_ptr=0; > SaImmHandleT temp_hdl=0; > > TRACE_ENTER(); > > + if (m_NCS_LOCK(&cb->cb_lock, NCS_LOCK_WRITE) != NCSCC_RC_SUCCESS) { > + TRACE_3("Lock failure"); > + return; > + } > + > /* scan the entire handle db & check each record */ > while ((clnode = (IMMA_CLIENT_NODE *) > ncs_patricia_tree_getnext(&cb->client_tree, (uint8_t > *)temp_ptr))) @@ -707,6 +711,8 @@ void > imma_process_stale_clients(IMMA_CB > } > } > > + m_NCS_UNLOCK(&cb->cb_lock, NCS_LOCK_WRITE); > + > TRACE_LEAVE(); > } > ------------------------------------------------------------------------------ Slashdot TV. Video for Nerds. Stuff that matters. http://tv.slashdot.org/ _______________________________________________ Opensaf-devel mailing list [email protected] https://lists.sourceforge.net/lists/listinfo/opensaf-devel
