From e9d878edaeeb595b9cc8d0d2033c7389c617646d Mon Sep 17 00:00:00 2001 From: hoiji09 Date: Sun, 5 Oct 2014 21:15:24 +0530 Subject: [PATCH] Simplified Locking in MultList This was done by moving the mutex from EntList to MultList. --- src/clstepcore/complexSupport.h | 27 ++++++++++++--------------- src/clstepcore/match-ors.cc | 4 ++-- src/clstepcore/multlist.cc | 20 +++----------------- src/clstepcore/orlist.cc | 6 +++--- src/clstepcore/trynext.cc | 12 ++++++------ 5 files changed, 26 insertions(+), 43 deletions(-) diff --git a/src/clstepcore/complexSupport.h b/src/clstepcore/complexSupport.h index fc6222ee..36e89de3 100644 --- a/src/clstepcore/complexSupport.h +++ b/src/clstepcore/complexSupport.h @@ -142,12 +142,6 @@ class SC_CORE_EXPORT EntNode { }; class SC_CORE_EXPORT EntList { - // NOTE: The locking methodology used here assumes that a node (i.e a - // EntList object) can neither be removed, nor can be inserted - // between two existing nodes. If such APIs are introduced in - // future the EntList class (or its subclasses) will no longer be - // thread safe and can crash / deadlock under multithreaded environment. - friend class MultList; friend class JoinList; friend class OrList; @@ -203,10 +197,6 @@ class SC_CORE_EXPORT EntList { return ( join != SIMPLE ); } EntList * next, *prev; - sc_mutex mtx; - /** \var mtx - * Currently only being used by the subclass MultList - */ protected: MatchType viable; @@ -269,7 +259,7 @@ class SC_CORE_EXPORT MultList : public EntList { public: MultList( JoinType j ) : EntList( j ), supertype( 0 ), numchildren( 0 ), - childList( 0 ) {} + childList( 0 ), mtxP( new sc_mutex() ) {;} ~MultList(); void setLevel( int ); bool contains( char * ); @@ -301,6 +291,13 @@ class SC_CORE_EXPORT MultList : public EntList { * The children may be SIMPLE EntLists (contain entity names) or may * themselves be And-, Or-, or AndOrLists. */ + + mutable sc_mutex * mtxP; + /** \var mtxP + * Protects childList, numchildren and the counters in subclass + * OrList. NOTE: The locking methodology assumes that a childList + * will have a unique parent. + */ }; /** \class JoinList @@ -346,17 +343,17 @@ class SC_CORE_EXPORT OrList : public MultList { void unmarkAll( EntNode * ); bool acceptChoice( EntNode * ); bool acceptNextChoice( EntNode * ents ) { - mtx.lock(); + mtxP->lock(); choice++; - mtx.unlock(); + mtxP->unlock(); return ( acceptChoice( ents ) ); } void reset() { - mtx.lock(); + mtxP->lock(); choice = -1; choice1 = -2; choiceCount = 0; - mtx.unlock(); + mtxP->unlock(); MultList::reset(); } diff --git a/src/clstepcore/match-ors.cc b/src/clstepcore/match-ors.cc index ea914a96..34182d5d 100644 --- a/src/clstepcore/match-ors.cc +++ b/src/clstepcore/match-ors.cc @@ -121,9 +121,9 @@ MatchType OrList::matchORs( EntNode * ents ) { if( choice == -1 ) { choice1 = choice = count; } - mtx.lock();// protects choiceCount + mtxP->lock();// protects choiceCount choiceCount++; - mtx.unlock(); + mtxP->unlock(); if( viable < retval ) { viable = retval; diff --git a/src/clstepcore/multlist.cc b/src/clstepcore/multlist.cc index 37d7a0f5..da08c511 100644 --- a/src/clstepcore/multlist.cc +++ b/src/clstepcore/multlist.cc @@ -27,6 +27,7 @@ MultList::~MultList() { delete child; child = cnext; } + delete mtxP; } /** @@ -104,29 +105,16 @@ EntList * MultList::getChild( int num ) { */ void MultList::appendList( EntList * ent ) { EntList * eprev; - - mtx.lock(); // Protects numchildren, childList + mtxP->lock(); // Protects numchildren, childList if( numchildren == 0 ) { childList = ent; } else { eprev = getLast(); - eprev->mtx.lock(); - // In a multithreaded environment It is possible that before locking - // more elements got added into the existing list and eprev is no - // longer the last element. This while loop takes care of that. - while( eprev->next ) { - eprev->mtx.unlock(); - eprev = eprev->next; - eprev->mtx.lock(); - } - // eprev lock is acquired eprev->next = ent; - eprev->mtx.unlock(); - ent->prev = eprev; // ent locking not required } numchildren += ent->siblings(); - mtx.unlock(); + mtxP->unlock(); } /** @@ -150,9 +138,7 @@ EntList * MultList::copyList( EntList * ent ) { newlist = new AndOrList; break; }; - ent->mtx.lock(); // appendList doesn't lock the EntNode appendList( newlist ); - ent->mtx.unlock(); if( ent->multiple() ) { // For the multlists, we must recurse for all their children: diff --git a/src/clstepcore/orlist.cc b/src/clstepcore/orlist.cc index 58309e2a..653b2a21 100644 --- a/src/clstepcore/orlist.cc +++ b/src/clstepcore/orlist.cc @@ -65,7 +65,7 @@ void OrList::unmarkAll( EntNode * ents ) { bool OrList::acceptChoice( EntNode * ents ) { EntList * child; - mtx.lock(); // to proctect choice + mtxP->lock(); // to proctect choice if( choice == LISTEND ) { choice = choice1; } @@ -73,7 +73,7 @@ bool OrList::acceptChoice( EntNode * ents ) { while( child ) { if( child->viable >= MATCHSOME && child->acceptChoice( ents ) ) { // acceptChoice() returns true if we marked something. - mtx.unlock(); + mtxP->unlock(); return true; } child = child->next; @@ -82,6 +82,6 @@ bool OrList::acceptChoice( EntNode * ents ) { // If we got here, we must have gotten to the end of the childList without // finding a choice which marks something. choice = LISTEND; - mtx.unlock(); + mtxP->unlock(); return false; } diff --git a/src/clstepcore/trynext.cc b/src/clstepcore/trynext.cc index e091901d..63146aa5 100644 --- a/src/clstepcore/trynext.cc +++ b/src/clstepcore/trynext.cc @@ -103,10 +103,10 @@ MatchType OrList::tryNext( EntNode * ents ) { EntList * child; MatchType retval; - mtx.lock(); // For choice + mtxP->lock(); // For choice if( choice == LISTEND ) { // if we've already exhausted all the choices in this OR, - mtx.unlock(); + mtxP->unlock(); return NOMORE; } @@ -120,7 +120,7 @@ MatchType OrList::tryNext( EntNode * ents ) { // our descendants before moving on. retval = ( ( MultList * )child )->tryNext( ents ); if( retval == MATCHALL ) { - mtx.unlock(); + mtxP->unlock(); ents->sharedMtxP->unlock(); return MATCHALL; } @@ -129,7 +129,7 @@ MatchType OrList::tryNext( EntNode * ents ) { // EntLists on the higher levels (if there are) can retry all the // later choices with the new choice we just found. Otherwise, // we'll continue below looking into our next choice. - mtx.unlock(); + mtxP->unlock(); ents->sharedMtxP->unlock(); return NEWCHOICE; } @@ -143,11 +143,11 @@ MatchType OrList::tryNext( EntNode * ents ) { // (Also, it's nec. to unmark now, as we did above before returning and // before the calling tryNext() tries earlier OR's - see notes, 11/12.) choice = LISTEND; - mtx.unlock(); + mtxP->unlock(); ents->sharedMtxP->unlock(); return NOMORE; } - mtx.unlock(); + mtxP->unlock(); // Otherwise, try our next: if( acceptNextChoice( ents ) ) {