Hi, I made some changes in the code to remove dynamic initialize leaks. They only appeared in the SipXportLib so I only made an init function for this library. I added two files to the SipXportLib project. portlibInit.cpp(placed in \sipXportLib\src\) and portlibInit.h (placed in \sipXportLib\include\). Files and patches are attached. What do you think?
Index:
D:/TEMP/SIPXTapiPreCompliledRelease-Debug_21-feb/SIPXTapiPreCompliledRelease-Debug/sipXmediaLib/src/mp/NetInTask.cpp
===================================================================
---
D:/TEMP/SIPXTapiPreCompliledRelease-Debug_21-feb/SIPXTapiPreCompliledRelease-Debug/sipXmediaLib/src/mp/NetInTask.cpp
(revision 9483)
+++
D:/TEMP/SIPXTapiPreCompliledRelease-Debug_21-feb/SIPXTapiPreCompliledRelease-Debug/sipXmediaLib/src/mp/NetInTask.cpp
(working copy)
@@ -193,6 +193,12 @@
pHelper->start();
}
pNotify->wait();
+ pHelper->requestShutdown();
+ for(int wait = 100; wait > 0 ; wait--){
+ if( pHelper->isShutDown() )
+ break;
+ OsTask::delay(1);
+ }
delete pHelper;
delete pNotify;
}
@@ -432,13 +438,11 @@
// osPrintf("NetInTask: calling accept\n");
mpReadSocket = pBindSocket->accept();
// osPrintf("NetInTask: accept returned, closing server socket\n");
- pBindSocket->close();
- delete pBindSocket;
+ pBindSocket->close();
if (NULL == mpReadSocket) {
Zprintf(" *** NetInTask: accept() failed!\n", 0,0,0,0,0,0);
return 0;
}
-
for (i=0, ppr=pairs; i<NET_TASK_MAX_FD_PAIRS; i++) {
ppr->pRtpSocket = NULL;
ppr->pRtcpSocket = NULL;
@@ -660,6 +664,8 @@
ppr++;
}
}
+ delete mpReadSocket;
+ delete pBindSocket;
return 0;
}
Index:
D:/TEMP/SIPXTapiPreCompliledRelease-Debug_21-feb/SIPXTapiPreCompliledRelease-Debug/sipXportLib/src/os/OsNameDb.cpp
===================================================================
---
D:/TEMP/SIPXTapiPreCompliledRelease-Debug_21-feb/SIPXTapiPreCompliledRelease-Debug/sipXportLib/src/os/OsNameDb.cpp
(revision 9483)
+++
D:/TEMP/SIPXTapiPreCompliledRelease-Debug_21-feb/SIPXTapiPreCompliledRelease-Debug/sipXportLib/src/os/OsNameDb.cpp
(working copy)
@@ -28,7 +28,7 @@
// STATIC VARIABLE INITIALIZATIONS
OsNameDb* OsNameDb::spInstance = NULL;
-OsBSem* OsNameDb::spLock = new OsBSem(OsBSem::Q_PRIORITY, OsBSem::FULL);
+OsBSem* OsNameDb::spLock;
/* //////////////////////////// PUBLIC ////////////////////////////////////
*/
Index:
D:/TEMP/SIPXTapiPreCompliledRelease-Debug_21-feb/SIPXTapiPreCompliledRelease-Debug/sipXportLib/src/os/OsSysLog.cpp
===================================================================
---
D:/TEMP/SIPXTapiPreCompliledRelease-Debug_21-feb/SIPXTapiPreCompliledRelease-Debug/sipXportLib/src/os/OsSysLog.cpp
(revision 9483)
+++
D:/TEMP/SIPXTapiPreCompliledRelease-Debug_21-feb/SIPXTapiPreCompliledRelease-Debug/sipXportLib/src/os/OsSysLog.cpp
(working copy)
@@ -60,7 +60,7 @@
unsigned long OsSysLog::sEventCount = 0;
UtlString OsSysLog::sProcessId = "" ;
UtlString OsSysLog::sHostname = "" ;
-OsSysLogPriority* OsSysLog::spPriorities = new
OsSysLogPriority[FAC_MAX_FACILITY] ;
+OsSysLogPriority* OsSysLog::spPriorities;
// Initial logging level is PRI_ERR.
OsSysLogPriority OsSysLog::sLoggingPriority = PRI_ERR ;
UtlBoolean OsSysLog::bPrioritiesInitialized = FALSE ;
@@ -970,4 +970,4 @@
OsTask::getCurrentTaskId(mTid);
OsSysLog::add(mFacility, mPriority, "ENTER FUNC (tid=%d) %s\n",
mTid, mMethodName.data());
-}
\ No newline at end of file
+}
Index:
D:/TEMP/SIPXTapiPreCompliledRelease-Debug_21-feb/SIPXTapiPreCompliledRelease-Debug/sipXportLib/src/os/OsTimerTask.cpp
===================================================================
---
D:/TEMP/SIPXTapiPreCompliledRelease-Debug_21-feb/SIPXTapiPreCompliledRelease-Debug/sipXportLib/src/os/OsTimerTask.cpp
(revision 9483)
+++
D:/TEMP/SIPXTapiPreCompliledRelease-Debug_21-feb/SIPXTapiPreCompliledRelease-Debug/sipXportLib/src/os/OsTimerTask.cpp
(working copy)
@@ -20,7 +20,7 @@
#include "os/OsTimer.h"
#include "os/OsTimerMsg.h"
#include "os/OsTimerTask.h"
-#include "os/OsLock.h"
+#include "portlibInit.h"
// EXTERNAL FUNCTIONS
// EXTERNAL VARIABLES
@@ -33,8 +33,7 @@
// Create a semaphore at run time rather than declaring a static semaphore,
// so that the shut-down code does not try to destroy the semaphore,
// which can lead to problems with the ordering of destructors.
-OsBSem* OsTimerTask::sLock =
- new OsBSem(OsBSem::Q_PRIORITY, OsBSem::FULL);
+
const int OsTimerTask::TIMER_MAX_REQUEST_MSGS = 10000;
/* //////////////////////////// PUBLIC ////////////////////////////////////
*/
@@ -50,20 +49,20 @@
{
// If the task does not yet exist or hasn't been started, then acquire
// the lock to ensure that only one instance of the task is started
- sLock->acquire();
+ portlib::OsTimerTaskLock->acquire();
// Test again, as the previous test was not interlocked against other
// threads.
if (spInstance == NULL)
- {
+ {
spInstance = new OsTimerTask();
assert( spInstance );
// Have to cast spInstance to remove volatile, according to C++
// rules.
UtlBoolean isStarted = ((OsTimerTask*) spInstance)->start();
- assert(isStarted);
- }
- sLock->release();
+ assert(isStarted);
+ }
+ portlib::OsTimerTaskLock->release();
OsSysLog::add(FAC_KERNEL, PRI_DEBUG,
"OsTimerTask::getTimerTask OsTimerTask started");
}
@@ -76,13 +75,13 @@
{
OsSysLog::add(FAC_KERNEL, PRI_DEBUG,
"OsTimerTask::destroyTimerTask entered");
- sLock->acquire();
+ portlib::OsTimerTaskLock->acquire();
if (spInstance)
{
delete spInstance ;
spInstance = NULL ;
}
- sLock->release();
+ portlib::OsTimerTaskLock->release();
}
// Destructor
@@ -99,7 +98,7 @@
// Since this code is locked by sLock, no (few) messages will have
// been added to the incoming queue while we were waiting for the
// OS_TIMER_SHUTDOWN message to get through the queue, as getTimerTask would
- // have waited for sLock.
+ // have waited for sLock
}
/* ============================ MANIPULATORS ==============================
*/
@@ -119,6 +118,7 @@
),
mTimerQueue(0)
{
+
}
// The entry point for the task.
Index:
D:/TEMP/SIPXTapiPreCompliledRelease-Debug_21-feb/SIPXTapiPreCompliledRelease-Debug/sipXportLib/src/utl/UtlContainer.cpp
===================================================================
---
D:/TEMP/SIPXTapiPreCompliledRelease-Debug_21-feb/SIPXTapiPreCompliledRelease-Debug/sipXportLib/src/utl/UtlContainer.cpp
(revision 9483)
+++
D:/TEMP/SIPXTapiPreCompliledRelease-Debug_21-feb/SIPXTapiPreCompliledRelease-Debug/sipXportLib/src/utl/UtlContainer.cpp
(working copy)
@@ -29,7 +29,7 @@
* between modules, and this lock needs to exist until all container and
iterator
* destructors have been run - so we deliberately leak it.
*/
-OsBSem* UtlContainer::spIteratorConnectionLock = new
OsBSem(OsBSem::Q_PRIORITY, OsBSem::FULL);
+OsBSem* UtlContainer::spIteratorConnectionLock;
/* //////////////////////////// PUBLIC ////////////////////////////////////
*/
Index:
D:/TEMP/SIPXTapiPreCompliledRelease-Debug_21-feb/SIPXTapiPreCompliledRelease-Debug/sipXportLib/src/utl/UtlLink.cpp
===================================================================
---
D:/TEMP/SIPXTapiPreCompliledRelease-Debug_21-feb/SIPXTapiPreCompliledRelease-Debug/sipXportLib/src/utl/UtlLink.cpp
(revision 9483)
+++
D:/TEMP/SIPXTapiPreCompliledRelease-Debug_21-feb/SIPXTapiPreCompliledRelease-Debug/sipXportLib/src/utl/UtlLink.cpp
(working copy)
@@ -3,14 +3,10 @@
//
//////////////////////////////////////////////////////////////////////////////
-// SYSTEM INCLUDES
-#include "os/OsDefs.h"
-#include "os/OsBSem.h"
-#include "os/OsLock.h"
-#include "assert.h"
+//// SYSTEM INCLUDES
// APPLICATION INCLUDES
-#include "utl/UtlLink.h"
+#include "portlibInit.h"
// DEFINES
// MACROS
@@ -103,117 +99,8 @@
}
-/// Pool of available objects derived from UtlChain.
-/**
- * This avoids excessive heap operations; rather than delete unused
UtlChains, they are
- * stored on the mPool here. To limit the heap overhead associated with
allocating
- * UtlChain, they are allocated in mBlockSize blocks, which are chained on
- * mBlocks.
- *
- * The actual allocation of the blocks and initial chaining is done by the
allocator
- * function supplied by the UtlChain subclass.
- */
-class UtlChainPool
-{
-public:
-
-protected:
- friend class UtlLink;
- friend class UtlPair;
-
- /// Allocate blocksize instances of the subclass and chain them into the
pool.
- typedef void allocator(size_t blocksize, ///< number of instances to
allocate
- UtlChain* blockList, ///< list header for first instance
- UtlChain* pool ///< list header for others
- );
- /**<
- * This function is supplied by the subclass to the UtlChainPool
constructor.
- * It is responsible for allocating a block of blocksize instances of its
subclass.
- * The first instance in each block is added to the blockList, so that the
UtlChainPool
- * destructor can delete the block. The remaining (blocksize-1) instances
are
- * chained onto the pool list header.
- */
-
- /// Create a UtlChainPool that uses blockAllocator to create UtlChain
derived objects.
- UtlChainPool(allocator* blockAllocator, size_t blockSize) :
- mLock(OsBSem::Q_PRIORITY, OsBSem::FULL),
- mBlockSize(blockSize),
- mAllocations(0),
- mAllocator(blockAllocator)
- {
- }
-
- /// Get a UtlLink with chain pointers NULL
- UtlChain* get()
- {
- UtlChain* newChain;
- { // critical section for member variables
- OsLock poolLock(mLock);
-
- if (mPool.isUnLinked()) // are there available objects in the pool?
- {
- // no - get the subclass to allocate some more
- mAllocator(mBlockSize, &mBlocks, &mPool);
- mAllocations++;
- }
-
- // pull the first UtlChain off the mPool
- newChain = mPool.listHead ();
- if (newChain)
- {
- newChain->detachFromList(&mPool);
- }
- } // end of critical section
- return newChain;
- }
-
- /// Return freeLink to the pool of available UtlLinks.
- void release(UtlChain* freeChain)
- {
- OsLock poolLock(mLock);
-
- // put this freed object on the tail of the pool list
- freeChain->listBefore(&mPool, NULL);
- }
-
- /// Returns the total number of subclasses instances allocated by this
pool.
- /**
- * The returned count does not include the 1 instance in each allocation
that is
- * consumed to manage the pool.
- */
- size_t totalAllocated()
- {
- return mAllocations * (mBlockSize-1); // one per block is overhead
- }
-
-private:
-
- /// Release all dynamic memory used by the UtlLinkPool.
- ~UtlChainPool()
- {
- OsLock poolLock(mLock);
-
- UtlChain* block;
- while (!mBlocks.isUnLinked()) // blocks still on block list
- {
- block = mBlocks.listHead ()->detachFromList(&mBlocks);
- delete[] block;
- }
- }
-
- OsBSem mLock; ///< lock for all the other member variables
- size_t mBlockSize;
- size_t mAllocations;
- allocator* mAllocator;
- UtlChain mPool; ///< list of available UtlLinks.
- UtlChain mBlocks; /**< list of memory blocks allocated by the mAllocator.
- * Each block is an mBlockSize array of objects derived from
- * UtlChain. The 0th element is used to form the linked list
- * of blocks. The rest are made a part of the mPool.*/
-};
-
// The pool of available UtlLinks
-UtlChainPool* UtlLink::spLinkPool = new UtlChainPool(UtlLink::allocate,
UTLLINK_BLOCK_SIZE);
+UtlChainPool* UtlLink::spLinkPool;
UtlContainable* UtlLink::unlink()
{
@@ -348,7 +235,7 @@
// The pool of available UtlPairs
-UtlChainPool* UtlPair::spPairPool = new UtlChainPool(UtlPair::allocate,
UTLLINK_BLOCK_SIZE);
+UtlChainPool* UtlPair::spPairPool;// = new UtlChainPool(UtlPair::allocate,
UTLLINK_BLOCK_SIZE);
void UtlPair::allocate(size_t blocksize, ///< number of instances to
allocate
UtlChain* blockList, ///< list header for first instance
Index:
D:/TEMP/SIPXTapiPreCompliledRelease-Debug_21-feb/SIPXTapiPreCompliledRelease-Debug/sipXportLib/src/utl/UtlListIterator.cpp
===================================================================
---
D:/TEMP/SIPXTapiPreCompliledRelease-Debug_21-feb/SIPXTapiPreCompliledRelease-Debug/sipXportLib/src/utl/UtlListIterator.cpp
(revision 9483)
+++
D:/TEMP/SIPXTapiPreCompliledRelease-Debug_21-feb/SIPXTapiPreCompliledRelease-Debug/sipXportLib/src/utl/UtlListIterator.cpp
(working copy)
@@ -28,7 +28,7 @@
* the iterator would return the first element on the list, because NULL
* designates the state "before the first element".
*/
-const UtlLink* UtlListIterator::NOWHERE = new UtlLink;
+const UtlLink* UtlListIterator::NOWHERE;
UtlLink const* UtlListIterator::OFF_LIST_END = NOWHERE;
/* //////////////////////////// PUBLIC ////////////////////////////////////
*/
Index:
D:/TEMP/SIPXTapiPreCompliledRelease-Debug_21-feb/SIPXTapiPreCompliledRelease-Debug/sipXportLib/include/os/OsNameDb.h
===================================================================
---
D:/TEMP/SIPXTapiPreCompliledRelease-Debug_21-feb/SIPXTapiPreCompliledRelease-Debug/sipXportLib/include/os/OsNameDb.h
(revision 9483)
+++
D:/TEMP/SIPXTapiPreCompliledRelease-Debug_21-feb/SIPXTapiPreCompliledRelease-Debug/sipXportLib/include/os/OsNameDb.h
(working copy)
@@ -78,6 +78,9 @@
UtlBoolean isEmpty(void);
//:Return TRUE if the name database is empty
+ static OsBSem* spLock; // semaphore used to ensure that there is
+ // only one instance of this class
+
/* //////////////////////////// PROTECTED /////////////////////////////////
*/
protected:
@@ -97,8 +100,6 @@
private:
static OsNameDb* spInstance; // pointer to the single instance of the
// OsNameDb class
- static OsBSem* spLock; // semaphore used to ensure that there is
- // only one instance of this class
UtlHashMap mDict; // hash table used to store the name/value
// mappings
OsRWMutex mRWLock; // R/W mutex used to coordinate access to
Index:
D:/TEMP/SIPXTapiPreCompliledRelease-Debug_21-feb/SIPXTapiPreCompliledRelease-Debug/sipXportLib/include/os/OsSysLog.h
===================================================================
---
D:/TEMP/SIPXTapiPreCompliledRelease-Debug_21-feb/SIPXTapiPreCompliledRelease-Debug/sipXportLib/include/os/OsSysLog.h
(revision 9483)
+++
D:/TEMP/SIPXTapiPreCompliledRelease-Debug_21-feb/SIPXTapiPreCompliledRelease-Debug/sipXportLib/include/os/OsSysLog.h
(working copy)
@@ -177,6 +177,7 @@
{
/* //////////////////////////// PUBLIC ////////////////////////////////////
*/
public:
+ static OsSysLogPriority* spPriorities;
static const char* sPriorityNames[] ;
//:List of Priority Names orders in the same order as OsSysLogPriority.
@@ -441,7 +442,6 @@
static OsSysLogTask* spOsSysLogTask;
static unsigned long sEventCount;
- static OsSysLogPriority* spPriorities;
static OsSysLogPriority sLoggingPriority;
static UtlString sProcessId;
static UtlString sHostname;
Index:
D:/TEMP/SIPXTapiPreCompliledRelease-Debug_21-feb/SIPXTapiPreCompliledRelease-Debug/sipXportLib/include/utl/UtlContainer.h
===================================================================
---
D:/TEMP/SIPXTapiPreCompliledRelease-Debug_21-feb/SIPXTapiPreCompliledRelease-Debug/sipXportLib/include/utl/UtlContainer.h
(revision 9483)
+++
D:/TEMP/SIPXTapiPreCompliledRelease-Debug_21-feb/SIPXTapiPreCompliledRelease-Debug/sipXportLib/include/utl/UtlContainer.h
(working copy)
@@ -147,6 +147,9 @@
/// Unlock the linkage between containers and iterators
static void releaseIteratorConnectionLock();
+ /// This lock prevent container/iterator deadlocks
+ static OsBSem* spIteratorConnectionLock;
+
/* //////////////////////////// PROTECTED /////////////////////////////////
*/
protected:
friend class UtlIterator;
@@ -202,8 +205,6 @@
/* //////////////////////////// PRIVATE ///////////////////////////////////
*/
private:
- /// This lock prevent container/iterator deadlocks
- static OsBSem* spIteratorConnectionLock;
/**<
* UtlContainer/UtlIterator locking strategy
*
Index:
D:/TEMP/SIPXTapiPreCompliledRelease-Debug_21-feb/SIPXTapiPreCompliledRelease-Debug/sipXportLib/include/utl/UtlListIterator.h
===================================================================
---
D:/TEMP/SIPXTapiPreCompliledRelease-Debug_21-feb/SIPXTapiPreCompliledRelease-Debug/sipXportLib/include/utl/UtlListIterator.h
(revision 9483)
+++
D:/TEMP/SIPXTapiPreCompliledRelease-Debug_21-feb/SIPXTapiPreCompliledRelease-Debug/sipXportLib/include/utl/UtlListIterator.h
(working copy)
@@ -91,6 +91,8 @@
* Is the iterator positioned at the last element?
*/
UtlBoolean atLast() const ;
+
+ static const UtlLink* NOWHERE;
/* //////////////////////////// PROTECTED /////////////////////////////////
*/
protected:
@@ -106,7 +108,6 @@
virtual void removing(const UtlLink* node);
- static const UtlLink* NOWHERE;
static UtlLink const* OFF_LIST_END;
/* //////////////////////////// PRIVATE ///////////////////////////////////
*/
2007/4/26, [EMAIL PROTECTED] <[EMAIL PROTECTED]>:
ymoona wrote: > > I tried making it static, but then I get an access violation when I try to > acquire a lock. > I made it static like this: OsBSem* OsTimerTask::sLock; > > But then the constructor is not called causing an access violation > > when I put a break point print on the original > initialisation, I see that it is called twice. > Function: `dynamic initializer for 'OsTimerTask::sLock''(void), Thread: > 0xA80 OsFileBase::touch, Caller _initterm, Address 0x101F7740 > Function: `dynamic initializer for 'OsTimerTask::sLock''(void), Thread: > 0xA80 OsFileBase::touch, Caller _initterm, Address 0x00449580 > > How do I make sLock static? > I had a look at OsBSem* OsTimerTask::sLock again and it cannot be made "static" static member because it is derived from OsBSemWnt, which has in acquire(const OsTime& rTimeout = OsTime::OS_INFINITY); OsTime::OS_INFINITY which is also a static member and might be uninitialized during sLock uninitialization. so having OsBSem OsTimerTask::sLock; in cpp file will not work. You cant have just OsBSem* OsTimerTask::sLock; in cpp file as its just a static pointer without an object, so when u access it you get access violation. That's why sLock is initialized using new, and never freed. It's actually easy to get rid of this initialziation problem in sipxportlib without any compiler tricks, but across libraries its more dificult. The solution is : - introduce init/shutdown methods for each library, init will create singletons/static members and shutdown delete them. Each library would have a counter in init/shutdown so it only initializes once and also shutdowns only once (in case we get init, init, shutdown, shutdown). Init and shutdown would make sure singletons are initialized and deleted in the right order. Then init function would simply call static methods Initialize and Shutdown of all classes that have some static members. This is the cleanest solution. We would only call sipxInitialize: 1.)sipxInitialize (would also initialize any global dynamic variables if counter is 0, it would initialize all required libraries, increase reference counter) 2.)SipXTackLib::Init() (if counter is 0, initialize all required libraries - portlib, then initialize static members and singletons, increase counter) 3.) SipXPortLib::Init() (called from tacklib init, if counter is 0, initialize all static members and singletons, increase counter) 4.) SipXCallLib::Init() (it would initialize the same portlib again, but this time singletons are already created and nothing new would be created) 5.) SipXPortLib::Init() (called from calllib) 6.) SipXMediaLib::Init() (it would initialize the same portlib again) 7.) SipXPortLib::Init() (called from medialib) ... sipxUnInitialize (decrease counter, would delete global dynamic objects if referece counter reaches 0, and also shutdown libraries) 8.)SipXTackLib::Shutdown() (decrease counter, if 0 then shutdown portlib) 9.)SipXPortLib::Shutdown() (called from tacklib, decrease counter, not 0, dont shutdown) 10.)SipXCallLib::Shutdown() (decrease counter, if 0 then shutdown portlib) 11.)SipXPortLib::Shutdown() (called from calllib, decrease counter, not 0, dont shutdown) 12.)SipXMediaLib::Shutdown() (decrease counter, if 0 then shutdown portlib) 13.)SipXPortLib::Shutdown() (called from media lib, decrease counter, this time counter is 0, its safe to delete static members and singletons) sipxsdplib and mediaadapterlib would also be initialized/uninitialized Initialize and Shutdown would call public static methods Initialize and Shutdown of all classes that have static members or singleton. What do you guys think? Jaroslav Libak
portlibInit.h
Description: Binary data
portlibInit.cpp
Description: Binary data
_______________________________________________ sipxtapi-dev mailing list [email protected] List Archive: http://list.sipfoundry.org/archive/sipxtapi-dev/
