Bug #541237 (mantis-192) “Concurrency bug in channel access to l...” : Bugs : EPICS Base

Revision history for this message

Jeff Hill (johill-lanl) wrote on 2005-04-12:

#1

Download full text (9.6 KiB)

I commited this fix

cvs diff: Diffing .
Index: dbCAC.h
===================================================================
RCS file: /net/phoebus/epicsmgr/cvsroot/epics/base/src/db/dbCAC.h,v
retrieving revision 1.39.2.5
diff -c -r1.39.2.5 dbCAC.h
*** dbCAC.h 23 Jan 2004 20:15:35 -0000 1.39.2.5
--- dbCAC.h 12 Apr 2005 16:38:59 -0000
***************
*** 126,145 ****
dbContextPrivateListOfIO & operator = ( const dbContextPrivateListOfIO & );
};

! // allow only one thread at a time to use the cache, but do not hold
! // lock when calling the callback
  class dbContextReadNotifyCache {
  public:
      dbContextReadNotifyCache ( epicsMutex & );
- ~dbContextReadNotifyCache ();
      void callReadNotify ( epicsGuard < epicsMutex > &,
          struct dbAddr & addr, unsigned type, unsigned long count,
              cacReadNotify & notify );
      void show ( epicsGuard < epicsMutex > &, unsigned level ) const;
  private:
! unsigned long readNotifyCacheSize;
! epicsMutex & mutex;
! char * pReadNotifyCache;
   dbContextReadNotifyCache ( const dbContextReadNotifyCache & );
   dbContextReadNotifyCache & operator = ( const dbContextReadNotifyCache & );
  };
--- 126,159 ----
   dbContextPrivateListOfIO & operator = ( const dbContextPrivateListOfIO & );
  };

! class dbContextReadNotifyCacheAllocator {
! public:
! dbContextReadNotifyCacheAllocator ();
! ~dbContextReadNotifyCacheAllocator ();
! char * alloc ( unsigned long size );
! void free ( char * pFree );
! void show ( unsigned level ) const;
! private:
! struct cacheElem_t {
! struct cacheElem_t * pNext;
! };
! unsigned long _readNotifyCacheSize;
! cacheElem_t * _pReadNotifyCache;
! void reclaimAllCacheEntries ();
! dbContextReadNotifyCacheAllocator ( const dbContextReadNotifyCacheAllocator & );
! dbContextReadNotifyCacheAllocator & operator = ( const dbContextReadNotifyCacheAllocator & );
! };
!
  class dbContextReadNotifyCache {
  public:
      dbContextReadNotifyCache ( epicsMutex & );
      void callReadNotify ( epicsGuard < epicsMutex > &,
          struct dbAddr & addr, unsigned type, unsigned long count,
              cacReadNotify & notify );
      void show ( epicsGuard < epicsMutex > &, unsigned level ) const;
  private:
! dbContextReadNotifyCacheAllocator _allocator;
! epicsMutex & _mutex;
   dbContextReadNotifyCache ( const dbContextReadNotifyCache & );
   dbContextReadNotifyCache & operator = ( const dbContextReadNotifyCache & );
  };
Index: dbContextReadNotifyCache.cpp
===================================================================
RCS file: /net/phoebus/epicsmgr/cvsroot/epics/base/src/db/dbContextReadNotifyCache.cpp,v
retrieving revision 1.1.2.1
diff -c -r1.1.2.1 dbContextReadNotifyCache.cpp
*** dbContextReadNotifyCache.cpp 9 Jan 2004 00:42:13 -0000 1.1.2.1
--- dbContextReadNotifyCache.cpp 12 Apr 2005 16:52:08 -0000
***************
*** 14,20 ****
   */

#include "epicsMutex.h"
- #include "tsFreeList.h"

#include "cadef.h" // this can be eliminated when the callbacks use the new interface
#include "db_access.h" // should be eliminated here in the future
--- 14,19 ----
***************
...

I commited this fix

cvs diff: Diffing .
Index: dbCAC.h
===================================================================
RCS file: /net/phoebus/epicsmgr/cvsroot/epics/base/src/db/dbCAC.h,v
retrieving revision 1.39.2.5
diff -c -r1.39.2.5 dbCAC.h
*** dbCAC.h	23 Jan 2004 20:15:35 -0000	1.39.2.5
--- dbCAC.h	12 Apr 2005 16:38:59 -0000
***************
*** 126,145 ****
  	dbContextPrivateListOfIO & operator = ( const dbContextPrivateListOfIO & );
  };
  
! // allow only one thread at a time to use the cache, but do not hold
! // lock when calling the callback
  class dbContextReadNotifyCache  {
  public:
      dbContextReadNotifyCache ( epicsMutex & );
-     ~dbContextReadNotifyCache ();
      void callReadNotify ( epicsGuard < epicsMutex > &, 
          struct dbAddr & addr, unsigned type, unsigned long count, 
              cacReadNotify & notify );
      void show ( epicsGuard < epicsMutex > &, unsigned level ) const;
  private:
!     unsigned long readNotifyCacheSize;
!     epicsMutex & mutex;
!     char * pReadNotifyCache;
  	dbContextReadNotifyCache ( const dbContextReadNotifyCache & );
  	dbContextReadNotifyCache & operator = ( const dbContextReadNotifyCache & );
  };
--- 126,159 ----
  	dbContextPrivateListOfIO & operator = ( const dbContextPrivateListOfIO & );
  };
  
! class dbContextReadNotifyCacheAllocator  {
! public:
!     dbContextReadNotifyCacheAllocator ();
!     ~dbContextReadNotifyCacheAllocator ();
!     char * alloc ( unsigned long size );
!     void free ( char * pFree );
!     void show ( unsigned level ) const;
! private:
!     struct cacheElem_t {
!         struct cacheElem_t * pNext;
!     };
!     unsigned long _readNotifyCacheSize;
!     cacheElem_t * _pReadNotifyCache;
!     void reclaimAllCacheEntries ();
! 	dbContextReadNotifyCacheAllocator ( const dbContextReadNotifyCacheAllocator & );
! 	dbContextReadNotifyCacheAllocator & operator = ( const dbContextReadNotifyCacheAllocator & );
! };
! 
  class dbContextReadNotifyCache  {
  public:
      dbContextReadNotifyCache ( epicsMutex & );
      void callReadNotify ( epicsGuard < epicsMutex > &, 
          struct dbAddr & addr, unsigned type, unsigned long count, 
              cacReadNotify & notify );
      void show ( epicsGuard < epicsMutex > &, unsigned level ) const;
  private:
!     dbContextReadNotifyCacheAllocator _allocator;
!     epicsMutex & _mutex;
  	dbContextReadNotifyCache ( const dbContextReadNotifyCache & );
  	dbContextReadNotifyCache & operator = ( const dbContextReadNotifyCache & );
  };
Index: dbContextReadNotifyCache.cpp
===================================================================
RCS file: /net/phoebus/epicsmgr/cvsroot/epics/base/src/db/dbContextReadNotifyCache.cpp,v
retrieving revision 1.1.2.1
diff -c -r1.1.2.1 dbContextReadNotifyCache.cpp
*** dbContextReadNotifyCache.cpp	9 Jan 2004 00:42:13 -0000	1.1.2.1
--- dbContextReadNotifyCache.cpp	12 Apr 2005 16:52:08 -0000
***************
*** 14,20 ****
   */
  
  #include "epicsMutex.h"
- #include "tsFreeList.h"
  
  #include "cadef.h" // this can be eliminated when the callbacks use the new interface
  #include "db_access.h" // should be eliminated here in the future
--- 14,19 ----
***************
*** 25,59 ****
  #include "dbCAC.h"
  
  dbContextReadNotifyCache::dbContextReadNotifyCache ( epicsMutex & mutexIn ) :
!     readNotifyCacheSize ( 0 ), mutex ( mutexIn ), pReadNotifyCache ( 0 )
  {
  }
  
! dbContextReadNotifyCache::~dbContextReadNotifyCache ()
! {
! 	delete this->pReadNotifyCache;
! }
! 
! void dbContextReadNotifyCache::show ( 
!     epicsGuard < epicsMutex > & guard, unsigned level ) const
! {
!     guard.assertIdenticalMutex ( this->mutex );
  
!     if ( level > 0 ) {
!         printf ( "\tget call back cache location %p, and its size %lu\n", 
!             static_cast <void *> ( this->pReadNotifyCache ), 
!             this->readNotifyCacheSize );
!     }
! }
! 
! // extra effort taken here to not hold the lock when caslling the callback
  void dbContextReadNotifyCache::callReadNotify ( 
      epicsGuard < epicsMutex > & guard, struct dbAddr & addr, 
  	unsigned type, unsigned long count, cacReadNotify & notify )
  {
!     guard.assertIdenticalMutex ( this->mutex );
! 
!     unsigned long size = dbr_size_n ( type, count );
  
      if ( type > INT_MAX ) {
          notify.exception ( guard, ECA_BADTYPE, 
--- 24,53 ----
  #include "dbCAC.h"
  
  dbContextReadNotifyCache::dbContextReadNotifyCache ( epicsMutex & mutexIn ) :
!     _mutex ( mutexIn )
  {
  }
  
! class privateAutoDestroyPtr {
! public:
!     privateAutoDestroyPtr ( 
!         dbContextReadNotifyCacheAllocator & allocator, unsigned long size ) :
!         _allocator ( allocator ), _p ( allocator.alloc ( size ) ) {}
!     ~privateAutoDestroyPtr () { _allocator.free ( _p ); }
! 	char * get () const { return _p; }              
! private:
!     dbContextReadNotifyCacheAllocator & _allocator;
!     char * _p;
! 	privateAutoDestroyPtr ( const privateAutoDestroyPtr & );
! 	privateAutoDestroyPtr & operator = ( const privateAutoDestroyPtr & );
! };
  
! // extra effort taken here to not hold the lock when calling the callback
  void dbContextReadNotifyCache::callReadNotify ( 
      epicsGuard < epicsMutex > & guard, struct dbAddr & addr, 
  	unsigned type, unsigned long count, cacReadNotify & notify )
  {
!     guard.assertIdenticalMutex ( _mutex );
  
      if ( type > INT_MAX ) {
          notify.exception ( guard, ECA_BADTYPE, 
***************
*** 62,99 ****
          return;
      }
  
!     if ( count > static_cast < unsigned > ( INT_MAX ) || 
! 		count > static_cast < unsigned > ( addr.no_elements ) ) {
          notify.exception ( guard, ECA_BADCOUNT, 
!             "element count out of range (high side)",
              type, count);
          return;
      }
  
!     if ( this->readNotifyCacheSize < size) {
!         char * pTmp = new char [size];
!         if ( ! pTmp ) {
!             notify.exception ( guard, ECA_ALLOCMEM, 
!                 "unable to allocate callback cache",
!                 type, count );
!             return;
!         }
!         delete [] this->pReadNotifyCache;
!         this->pReadNotifyCache = pTmp;
!         this->readNotifyCacheSize = size;
      }
      int status;
      {
          epicsGuardRelease < epicsMutex > unguard ( guard );
          status = db_get_field ( &addr, static_cast <int> ( type ), 
!                     this->pReadNotifyCache, static_cast <int> ( count ), 0 );
      }
      if ( status ) {
          notify.exception ( guard, ECA_GETFAIL, 
              "db_get_field() completed unsuccessfuly",
!             type, count);
      }
      else { 
!         notify.completion ( guard, type, count, this->pReadNotifyCache );
      }
  }
--- 56,163 ----
          return;
      }
  
!     if ( addr.no_elements < 0 ) {
          notify.exception ( guard, ECA_BADCOUNT, 
!             "database has negetive element count",
              type, count);
          return;
      }
  
!     if ( count > static_cast < unsigned > ( addr.no_elements ) ) {
!         notify.exception ( guard, ECA_BADCOUNT, 
!             "element count out of range (high side)",
!             type, count);
!         return;
      }
+ 
+     unsigned long size = dbr_size_n ( type, count );
+     privateAutoDestroyPtr ptr ( _allocator, size );
      int status;
      {
          epicsGuardRelease < epicsMutex > unguard ( guard );
          status = db_get_field ( &addr, static_cast <int> ( type ), 
!                     ptr.get (), static_cast <int> ( count ), 0 );
      }
      if ( status ) {
          notify.exception ( guard, ECA_GETFAIL, 
              "db_get_field() completed unsuccessfuly",
!             type, count );
      }
      else { 
!         notify.completion ( 
!             guard, type, count, ptr.get () );
!     }
! }
! 
! void dbContextReadNotifyCache::show ( 
!     epicsGuard < epicsMutex > & guard, unsigned level ) const
! {
!     guard.assertIdenticalMutex ( _mutex );
! 
!     printf ( "dbContextReadNotifyCache\n" );
!     if ( level > 0 ) {
!         this->_allocator.show ( level - 1 );
      }
  }
+ 
+ dbContextReadNotifyCacheAllocator::dbContextReadNotifyCacheAllocator () :
+     _readNotifyCacheSize ( 0 ), _pReadNotifyCache ( 0 )
+ {
+ }
+ 
+ dbContextReadNotifyCacheAllocator::~dbContextReadNotifyCacheAllocator ()
+ {
+     this->reclaimAllCacheEntries ();
+ }
+ 
+ void dbContextReadNotifyCacheAllocator::reclaimAllCacheEntries ()
+ {
+ 
+     while ( _pReadNotifyCache ) {
+         cacheElem_t * pNext = _pReadNotifyCache->pNext;
+         delete [] _pReadNotifyCache;
+         _pReadNotifyCache = pNext;
+     }
+ }
+ 
+ char * dbContextReadNotifyCacheAllocator::alloc ( unsigned long size )
+ {
+     if ( size > _readNotifyCacheSize ) {
+         this->reclaimAllCacheEntries ();
+         _readNotifyCacheSize = size;
+     }
+ 
+     cacheElem_t * pAlloc = _pReadNotifyCache;
+     if ( pAlloc ) {
+         _pReadNotifyCache = pAlloc->pNext;
+     }
+     else {
+         size_t nElem = _readNotifyCacheSize / sizeof ( cacheElem_t );
+         pAlloc = new cacheElem_t [ nElem + 1 ];
+     }
+     return reinterpret_cast < char * > ( pAlloc );
+ }
+ 
+ void dbContextReadNotifyCacheAllocator::free ( char * pFree )
+ {
+     cacheElem_t * pAlloc = reinterpret_cast < cacheElem_t * > ( pFree );
+     pAlloc->pNext = _pReadNotifyCache;
+     _pReadNotifyCache = pAlloc;
+ }
+ 
+ void dbContextReadNotifyCacheAllocator::show ( unsigned level ) const
+ {
+     printf ( "dbContextReadNotifyCacheAlocator\n" );
+     if ( level > 0 ) {
+         size_t count =0;
+         cacheElem_t * pNext = _pReadNotifyCache;
+         while ( pNext ) {
+             pNext = _pReadNotifyCache->pNext;
+             count++;
+         }
+         printf ( "\tcount %u and size %lu\n", 
+             count, _readNotifyCacheSize );
+     }
+ }
+ 
+

Revision history for this message

Jeff Hill (johill-lanl) wrote on 2005-04-12:

#2

ran CA regression tests against a local db channel

Revision history for this message

Jeff Hill (johill-lanl) wrote on 2005-04-12:

#3

fixed in R3.14.8

Revision history for this message

Jeff Hill (johill-lanl) wrote on 2005-04-12:

#4

PS: allocating and freeing on every call would probably introduce unacceptable
overhead so a buffer pool was implemented.

Revision history for this message

Jeff Hill (johill-lanl) wrote on 2005-04-13:

#5

fixed in R3.14.8

Revision history for this message

Andrew Johnson (anj) wrote on 2005-11-22:

#6

R3.14.8 Release.

EPICS Base

Concurrency bug in channel access to local DB

Bug Description

Other bug subscribers

Remote bug watches