From 315282c01a556f39883e9f2940271e1894e460ec Mon Sep 17 00:00:00 2001 From: "henrike@webrtc.org" Date: Fri, 9 Dec 2011 17:46:20 +0000 Subject: [PATCH] Fixes a compiler warning related to dynamically allocated static memory. the fix is to leak the memory since the OS will clean it up anyways. This will not add noise to memory tools so it's ok. The issue is reported here: http://code.google.com/p/webrtc/issues/detail?id=147. Review URL: http://webrtc-codereview.appspot.com/267023 git-svn-id: http://webrtc.googlecode.com/svn/trunk@1150 4adac7df-926f-26a2-2b94-8c16560cd09d --- src/modules/rtp_rtcp/source/ssrc_database.cc | 103 +----------- src/modules/rtp_rtcp/source/ssrc_database.h | 32 ++-- .../source/udp_socket2_manager_windows.cc | 67 ++++---- .../source/udp_socket2_manager_windows.h | 8 +- .../source/udp_socket_manager_posix.cc | 37 ++-- .../source/udp_socket_manager_posix.h | 8 +- .../source/udp_socket_manager_windows.cc | 22 ++- .../source/udp_socket_manager_windows.h | 6 +- .../source/udp_socket_manager_wrapper.cc | 158 ++---------------- .../source/udp_socket_manager_wrapper.h | 43 +++-- .../interface/static_instance.h | 156 +++++++++++++++++ src/system_wrappers/source/trace_impl.cc | 154 ++--------------- src/system_wrappers/source/trace_impl.h | 24 +-- 13 files changed, 325 insertions(+), 493 deletions(-) create mode 100644 src/system_wrappers/interface/static_instance.h diff --git a/src/modules/rtp_rtcp/source/ssrc_database.cc b/src/modules/rtp_rtcp/source/ssrc_database.cc index 4db32eeaf..befc5eea8 100644 --- a/src/modules/rtp_rtcp/source/ssrc_database.cc +++ b/src/modules/rtp_rtcp/source/ssrc_database.cc @@ -20,122 +20,35 @@ #include #include //timeGetTime +// TODO(hellner): investigate if it is necessary to disable these warnings. #pragma warning(disable:4311) #pragma warning(disable:4312) - - // Platform SDK fixes when building with /Wp64 for a 32 bits target. - #if !defined(_WIN64) && defined(_Wp64) - #ifdef InterlockedExchangePointer - #undef InterlockedExchangePointer - // The problem is that the macro provided for InterlockedExchangePointer() is - // doing a (LONG) C-style cast that triggers invariably the warning C4312 when - // building on 32 bits. - inline void* InterlockedExchangePointer(void* volatile* target, void* value) - { - return reinterpret_cast(static_cast(InterlockedExchange( - reinterpret_cast(target), - static_cast(reinterpret_cast(value))))); - } - #endif // #ifdef InterlockedExchangePointer - #endif //!defined(_WIN64) && defined(_Wp64) - #else #include #include #include #include - #include // definition of auto_ptr #endif namespace webrtc { -// Construct On First Use idiom. Avoids "static initialization order fiasco" (JFGI). -SSRCDatabase*& -SSRCDatabase::StaticInstance(SsrcDatabaseCount inc) +SSRCDatabase* +SSRCDatabase::StaticInstance(CountOperation count_operation) { - static volatile long theSSRCDatabaseCount = 0; // this needs to be long due to Windows, not an issue due to its usage - static SSRCDatabase* theSSRCDatabase = NULL; - - SsrcDatabaseCreate state = kSsrcDbExist; - -#ifndef _WIN32 - static std::auto_ptr crtiSect = std::auto_ptr(CriticalSectionWrapper::CreateCriticalSection()); - CriticalSectionScoped lock(*crtiSect); - - if(inc == kSsrcDbInc) - { - theSSRCDatabaseCount++; - if(theSSRCDatabaseCount == 1) - { - state = kSsrcDbCreate; - } - } else - { - theSSRCDatabaseCount--; - if(theSSRCDatabaseCount == 0) - { - state = kSsrcDbDestroy; - } - } - if(state == kSsrcDbCreate) - { - theSSRCDatabase = new SSRCDatabase(); - - }else if(state == kSsrcDbDestroy) - { - SSRCDatabase* oldValue = theSSRCDatabase; - theSSRCDatabase = NULL; - if(oldValue) - { - delete oldValue; - } - return theSSRCDatabase; - } -#else - // Windows - if(inc == kSsrcDbInc) - { - if(1 == InterlockedIncrement(&theSSRCDatabaseCount)) - { - state = kSsrcDbCreate; - } - } else - { - int newValue = InterlockedDecrement(&theSSRCDatabaseCount); - if(newValue == 0) - { - state = kSsrcDbDestroy; - } - } - if(state == kSsrcDbCreate) - { - SSRCDatabase* newValue = new SSRCDatabase(); - SSRCDatabase* oldValue = (SSRCDatabase*)InterlockedExchangePointer(reinterpret_cast(&theSSRCDatabase), newValue); - assert(oldValue == NULL); - - }else if(state == kSsrcDbDestroy) - { - SSRCDatabase* oldValue = (SSRCDatabase*)InterlockedExchangePointer(reinterpret_cast(&theSSRCDatabase), NULL); - if(oldValue) - { - delete oldValue; - } - return theSSRCDatabase; - } -#endif - assert(theSSRCDatabase); - return theSSRCDatabase; + SSRCDatabase* impl = + GetStaticInstance(count_operation); + return impl; } SSRCDatabase* SSRCDatabase::GetSSRCDatabase() { - return StaticInstance(kSsrcDbInc); + return StaticInstance(kAddRef); } void SSRCDatabase::ReturnSSRCDatabase() { - StaticInstance(kSsrcDbDec); + StaticInstance(kRelease); } WebRtc_UWord32 diff --git a/src/modules/rtp_rtcp/source/ssrc_database.h b/src/modules/rtp_rtcp/source/ssrc_database.h index 73ad21845..370e549e4 100644 --- a/src/modules/rtp_rtcp/source/ssrc_database.h +++ b/src/modules/rtp_rtcp/source/ssrc_database.h @@ -11,30 +11,18 @@ #ifndef WEBRTC_MODULES_RTP_RTCP_SOURCE_SSRC_DATABASE_H_ #define WEBRTC_MODULES_RTP_RTCP_SOURCE_SSRC_DATABASE_H_ -#include "typedefs.h" - #ifndef WEBRTC_NO_STL #include #endif +#include "system_wrappers/interface/static_instance.h" +#include "typedefs.h" + namespace webrtc { class CriticalSectionWrapper; class SSRCDatabase { - enum SsrcDatabaseCount - { - kSsrcDbDec = 0, - kSsrcDbInc = 1 - }; - - enum SsrcDatabaseCreate - { - kSsrcDbExist = 0, - kSsrcDbCreate = 1, - kSsrcDbDestroy = 2 - }; - public: static SSRCDatabase* GetSSRCDatabase(); static void ReturnSSRCDatabase(); @@ -43,13 +31,19 @@ public: WebRtc_Word32 RegisterSSRC(const WebRtc_UWord32 ssrc); WebRtc_Word32 ReturnSSRC(const WebRtc_UWord32 ssrc); -private: - static SSRCDatabase*& StaticInstance(SsrcDatabaseCount inc); - -private: +protected: SSRCDatabase(); virtual ~SSRCDatabase(); + static SSRCDatabase* CreateInstance() { return new SSRCDatabase(); } + +private: + // Friend function to allow the SSRC destructor to be accessed from the + // template class. + friend SSRCDatabase* GetStaticInstance( + CountOperation count_operation); + static SSRCDatabase* StaticInstance(CountOperation count_operation); + WebRtc_UWord32 GenerateRandom(); #ifdef WEBRTC_NO_STL diff --git a/src/modules/udp_transport/source/udp_socket2_manager_windows.cc b/src/modules/udp_transport/source/udp_socket2_manager_windows.cc index af9eac1bc..979c783b2 100644 --- a/src/modules/udp_transport/source/udp_socket2_manager_windows.cc +++ b/src/modules/udp_transport/source/udp_socket2_manager_windows.cc @@ -20,24 +20,17 @@ namespace webrtc { WebRtc_UWord32 UdpSocket2ManagerWindows::_numOfActiveManagers = 0; bool UdpSocket2ManagerWindows::_wsaInit = false; -UdpSocket2ManagerWindows::UdpSocket2ManagerWindows( - const WebRtc_Word32 id, - WebRtc_UWord8& numOfWorkThreads) - : UdpSocketManager(id, numOfWorkThreads), - _id(id), - _stoped(false), +UdpSocket2ManagerWindows::UdpSocket2ManagerWindows() + : UdpSocketManager(), + _id(-1), + _stopped(false), _init(false), _pCrit(NULL), _ioCompletionHandle(NULL), - _numActiveSockets(0), - _numOfWorkThreads(numOfWorkThreads) + _numActiveSockets(0) { _managerNumber = _numOfActiveManagers++; - WEBRTC_TRACE(kTraceDebug, kTraceTransport, _id, - "UdpSocket2ManagerWindows(%d)::UdpSocket2ManagerWindows()", - _managerNumber); - if(_numOfActiveManagers == 1) { WORD wVersionRequested = MAKEWORD(2, 2); @@ -105,6 +98,19 @@ UdpSocket2ManagerWindows::~UdpSocket2ManagerWindows() } } +bool UdpSocket2ManagerWindows::Init(WebRtc_Word32 id, + WebRtc_UWord8& numOfWorkThreads) { + CriticalSectionScoped cs(*_pCrit); + if ((_id != -1) || (_numOfWorkThreads != 0)) { + assert(_id != -1); + assert(_numOfWorkThreads != 0); + return false; + } + _id = id; + _numOfWorkThreads = numOfWorkThreads; + return true; +} + WebRtc_Word32 UdpSocket2ManagerWindows::ChangeUniqueId(const WebRtc_Word32 id) { _id = id; @@ -117,7 +123,7 @@ bool UdpSocket2ManagerWindows::Start() "UdpSocket2ManagerWindows(%d)::Start()",_managerNumber); if(!_init) { - Init(); + StartWorkerThreads(); } if(!_init) @@ -126,7 +132,7 @@ bool UdpSocket2ManagerWindows::Start() } _pCrit->Enter(); // Start worker threads. - _stoped = false; + _stopped = false; WebRtc_Word32 i = 0; WebRtc_Word32 error = 0; ListItem* pItem = _workerThreadsList.First(); @@ -154,14 +160,14 @@ bool UdpSocket2ManagerWindows::Start() return true; } -WebRtc_Word32 UdpSocket2ManagerWindows::Init() +bool UdpSocket2ManagerWindows::StartWorkerThreads() { if(!_init) { _pCrit = CriticalSectionWrapper::CreateCriticalSection(); if(_pCrit == NULL) { - return -1; + return false; } _pCrit->Enter(); @@ -174,11 +180,11 @@ WebRtc_Word32 UdpSocket2ManagerWindows::Init() kTraceError, kTraceTransport, _id, - "UdpSocket2ManagerWindows(%d)::Init()\ - _ioCompletioHandle == NULL: error:%d", + "UdpSocket2ManagerWindows(%d)::StartWorkerThreads()" + "_ioCompletioHandle == NULL: error:%d", _managerNumber,error); _pCrit->Leave(); - return -1; + return false; } // Create worker threads. @@ -208,8 +214,8 @@ WebRtc_Word32 UdpSocket2ManagerWindows::Init() kTraceError, kTraceTransport, _id, - "UdpSocket2ManagerWindows(%d)::Init() error creating work\ - threads", + "UdpSocket2ManagerWindows(%d)::StartWorkerThreads() error " + "creating work threads", _managerNumber); // Delete worker threads. ListItem* pItem = NULL; @@ -221,7 +227,7 @@ WebRtc_Word32 UdpSocket2ManagerWindows::Init() _workerThreadsList.PopFront(); } _pCrit->Leave(); - return -1; + return false; } if(_ioContextPool.Init()) { @@ -229,23 +235,23 @@ WebRtc_Word32 UdpSocket2ManagerWindows::Init() kTraceError, kTraceTransport, _id, - "UdpSocket2ManagerWindows(%d)::Init() error initiating\ - _ioContextPool", + "UdpSocket2ManagerWindows(%d)::StartWorkerThreads() error " + "initiating _ioContextPool", _managerNumber); _pCrit->Leave(); - return -1; + return false; } _init = true; WEBRTC_TRACE( kTraceDebug, kTraceTransport, _id, - "UdpSocket2ManagerWindows::Init() %d number of work threads\ - created and init", + "UdpSocket2ManagerWindows::StartWorkerThreads %d number of work " + "threads created and initialized", _numOfWorkThreads); _pCrit->Leave(); } - return 0; + return true; } bool UdpSocket2ManagerWindows::Stop() @@ -258,7 +264,7 @@ bool UdpSocket2ManagerWindows::Stop() return false; } _pCrit->Enter(); - _stoped = true; + _stopped = true; if(_numActiveSockets) { WEBRTC_TRACE( @@ -276,6 +282,7 @@ bool UdpSocket2ManagerWindows::Stop() _pCrit->Leave(); return result; } + bool UdpSocket2ManagerWindows::StopWorkerThreads() { WebRtc_Word32 error = 0; @@ -421,7 +428,7 @@ PerIoContext* UdpSocket2ManagerWindows::PopIoContext() } PerIoContext* pIoC = NULL; - if(!_stoped) + if(!_stopped) { pIoC = _ioContextPool.PopIoContext(); }else diff --git a/src/modules/udp_transport/source/udp_socket2_manager_windows.h b/src/modules/udp_transport/source/udp_socket2_manager_windows.h index 3e10aad2c..f8619d6bc 100644 --- a/src/modules/udp_transport/source/udp_socket2_manager_windows.h +++ b/src/modules/udp_transport/source/udp_socket2_manager_windows.h @@ -96,10 +96,10 @@ private: class UdpSocket2ManagerWindows : public UdpSocketManager { public: - UdpSocket2ManagerWindows(const WebRtc_Word32 id, - WebRtc_UWord8& numOfWorkThreads); + UdpSocket2ManagerWindows(); virtual ~UdpSocket2ManagerWindows(); + virtual bool Init(WebRtc_Word32 id, WebRtc_UWord8& numOfWorkThreads); virtual WebRtc_Word32 ChangeUniqueId(const WebRtc_Word32 id); virtual bool Start(); @@ -117,7 +117,7 @@ public: private: bool StopWorkerThreads(); - WebRtc_Word32 Init(); + bool StartWorkerThreads(); bool AddSocketPrv(UdpSocket2Windows* s); bool RemoveSocketPrv(UdpSocket2Windows* s); @@ -127,7 +127,7 @@ private: WebRtc_Word32 _id; CriticalSectionWrapper* _pCrit; WebRtc_Word32 _managerNumber; - volatile bool _stoped; + volatile bool _stopped; bool _init; WebRtc_Word32 _numActiveSockets; ListWrapper _workerThreadsList; diff --git a/src/modules/udp_transport/source/udp_socket_manager_posix.cc b/src/modules/udp_transport/source/udp_socket_manager_posix.cc index 72a4ee6ef..9c629d9db 100644 --- a/src/modules/udp_transport/source/udp_socket_manager_posix.cc +++ b/src/modules/udp_transport/source/udp_socket_manager_posix.cc @@ -20,16 +20,30 @@ #include "udp_socket_posix.h" namespace webrtc { -UdpSocketManagerPosix::UdpSocketManagerPosix(const WebRtc_Word32 id, - WebRtc_UWord8& numOfWorkThreads) - : UdpSocketManager(id, numOfWorkThreads), - _id(id), +UdpSocketManagerPosix::UdpSocketManagerPosix() + : UdpSocketManager(), + _id(-1), _critSect(CriticalSectionWrapper::CreateCriticalSection()), - _numberOfSocketMgr(numOfWorkThreads), + _numberOfSocketMgr(-1), _incSocketMgrNextTime(0), _nextSocketMgrToAssign(0), _socketMgr() { +} + +bool UdpSocketManagerPosix::Init(WebRtc_Word32 id, + WebRtc_UWord8& numOfWorkThreads) { + CriticalSectionScoped cs(*_critSect); + if ((_id != -1) || (_numOfWorkThreads != 0)) { + assert(_id != -1); + assert(_numOfWorkThreads != 0); + return false; + } + + _id = id; + _numberOfSocketMgr = numOfWorkThreads; + _numOfWorkThreads = numOfWorkThreads; + if(MAX_NUMBER_OF_SOCKET_MANAGERS_LINUX < _numberOfSocketMgr) { _numberOfSocketMgr = MAX_NUMBER_OF_SOCKET_MANAGERS_LINUX; @@ -38,14 +52,13 @@ UdpSocketManagerPosix::UdpSocketManagerPosix(const WebRtc_Word32 id, { _socketMgr[i] = new UdpSocketManagerPosixImpl(); } - - WEBRTC_TRACE(kTraceDebug, kTraceTransport, _id, - "UdpSocketManagerPosix(%d)::UdpSocketManagerPosix()", - _numberOfSocketMgr); + return true; } + UdpSocketManagerPosix::~UdpSocketManagerPosix() { + Stop(); WEBRTC_TRACE(kTraceDebug, kTraceTransport, _id, "UdpSocketManagerPosix(%d)::UdpSocketManagerPosix()", _numberOfSocketMgr); @@ -95,7 +108,7 @@ bool UdpSocketManagerPosix::Stop() _critSect->Enter(); bool retVal = true; - for(int i = 0;i < _numberOfSocketMgr && retVal; i++) + for(int i = 0; i < _numberOfSocketMgr && retVal; i++) { retVal = _socketMgr[i]->Stop(); } @@ -105,8 +118,8 @@ bool UdpSocketManagerPosix::Stop() kTraceError, kTraceTransport, _id, - "UdpSocketManagerPosix(%d)::Stop() there are still active socket\ - managers", + "UdpSocketManagerPosix(%d)::Stop() there are still active socket " + "managers", _numberOfSocketMgr); } _critSect->Leave(); diff --git a/src/modules/udp_transport/source/udp_socket_manager_posix.h b/src/modules/udp_transport/source/udp_socket_manager_posix.h index 11f2a4411..c89aa134f 100644 --- a/src/modules/udp_transport/source/udp_socket_manager_posix.h +++ b/src/modules/udp_transport/source/udp_socket_manager_posix.h @@ -24,15 +24,19 @@ #define MAX_NUMBER_OF_SOCKET_MANAGERS_LINUX 8 namespace webrtc { + +class ConditionVariableWrapper; class UdpSocketManagerPosixImpl; class UdpSocketManagerPosix : public UdpSocketManager { public: - UdpSocketManagerPosix(const WebRtc_Word32 id, - WebRtc_UWord8& numOfWorkThreads); + UdpSocketManagerPosix(); virtual ~UdpSocketManagerPosix(); + virtual bool Init(WebRtc_Word32 id, + WebRtc_UWord8& numOfWorkThreads); + virtual WebRtc_Word32 ChangeUniqueId(const WebRtc_Word32 id); virtual bool Start(); diff --git a/src/modules/udp_transport/source/udp_socket_manager_windows.cc b/src/modules/udp_transport/source/udp_socket_manager_windows.cc index 970b0e2dd..d10ac7625 100644 --- a/src/modules/udp_transport/source/udp_socket_manager_windows.cc +++ b/src/modules/udp_transport/source/udp_socket_manager_windows.cc @@ -14,11 +14,9 @@ namespace webrtc { WebRtc_UWord32 UdpSocketManagerWindows::_numOfActiveManagers = 0; -UdpSocketManagerWindows::UdpSocketManagerWindows( - const WebRtc_Word32 id, - WebRtc_UWord8& numOfWorkThreads) - : _id(id), - UdpSocketManager(id, numOfWorkThreads) +UdpSocketManagerWindows::UdpSocketManagerWindows() + : UdpSocketManager(), + _id(-1) { const WebRtc_Word8* threadName = "UdpSocketManagerWindows_Thread"; _critSectList = CriticalSectionWrapper::CreateCriticalSection(); @@ -30,8 +28,22 @@ UdpSocketManagerWindows::UdpSocketManagerWindows( _numOfActiveManagers++; } +bool UdpSocketManagerWindows::Init(WebRtc_Word32 id, + WebRtc_UWord8& numOfWorkThreads) { + CriticalSectionScoped cs(*_critSectList); + if ((_id != -1) || (_numOfWorkThreads != 0)) { + assert(_id == -1); + assert(_numOfWorkThreads == 0); + return false; + } + _id = id; + _numOfWorkThreads = numOfWorkThreads; + return true; +} + UdpSocketManagerWindows::~UdpSocketManagerWindows() { + Stop(); if(_thread != NULL) { delete _thread; diff --git a/src/modules/udp_transport/source/udp_socket_manager_windows.h b/src/modules/udp_transport/source/udp_socket_manager_windows.h index 143cb5e54..06d905aed 100644 --- a/src/modules/udp_transport/source/udp_socket_manager_windows.h +++ b/src/modules/udp_transport/source/udp_socket_manager_windows.h @@ -29,10 +29,12 @@ class UdpSocketWindows; class UdpSocketManagerWindows : public UdpSocketManager { public: - UdpSocketManagerWindows(const WebRtc_Word32 id, - WebRtc_UWord8& numOfWorkThreads); + UdpSocketManagerWindows(); virtual ~UdpSocketManagerWindows(); + virtual bool Init(WebRtc_Word32 id, + WebRtc_UWord8& numOfWorkThreads); + virtual WebRtc_Word32 ChangeUniqueId(const WebRtc_Word32 id); virtual bool Start(); diff --git a/src/modules/udp_transport/source/udp_socket_manager_wrapper.cc b/src/modules/udp_transport/source/udp_socket_manager_wrapper.cc index 2800797ea..4113c7ea8 100644 --- a/src/modules/udp_transport/source/udp_socket_manager_wrapper.cc +++ b/src/modules/udp_transport/source/udp_socket_manager_wrapper.cc @@ -20,183 +20,53 @@ #include "udp_socket_manager_posix.h" #endif -#ifndef _WIN32 -#include -#endif - namespace webrtc { -UdpSocketManager* UdpSocketManager::CreateSocketManager( - const WebRtc_Word32 id, - WebRtc_UWord8& numOfWorkThreads) +UdpSocketManager* UdpSocketManager::CreateInstance() { #if defined(_WIN32) #if (defined(USE_WINSOCK2)) return static_cast( - new UdpSocket2ManagerWindows(id, numOfWorkThreads)); + new UdpSocket2ManagerWindows()); #else numOfWorkThreads = 1; return static_cast( - new UdpSocketManagerWindows(id, numOfWorkThreads)); + new UdpSocketManagerWindows()); #endif #else - return new UdpSocketManagerPosix(id, numOfWorkThreads); + return new UdpSocketManagerPosix(); #endif } -// TODO (hellner): more or less the same code is used in trace_impl.cc. -// Should be possible to avoid duplication here. -// Construct On First Use idiom. Avoids "static initialization order fiasco". UdpSocketManager* UdpSocketManager::StaticInstance( - const UdpSocketManagerCount inc, + CountOperation count_operation, const WebRtc_Word32 id, WebRtc_UWord8& numOfWorkThreads) { - // TODO (hellner): use atomic wrapper instead. - static volatile long theUdpSocketManagerCount = 0; - static UdpSocketManager* volatile theUdpSocketManager = NULL; - - UdpSocketManagerState state = kUdpSocketManagerExist; - -#ifndef _WIN32 - - static std::auto_ptr crtiSect = - std::auto_ptr( - CriticalSectionWrapper::CreateCriticalSection()); - CriticalSectionScoped lock(*crtiSect); - - if(inc == kUdpSocketManagerInc) - { - theUdpSocketManagerCount++; - if(theUdpSocketManagerCount == 1) - { - state = kUdpSocketManagerCreate; - } - } else - { - theUdpSocketManagerCount--; - if(theUdpSocketManagerCount == 0) - { - state = kUdpSocketManagerDestroy; + UdpSocketManager* impl = + GetStaticInstance(count_operation); + if (count_operation == kAddRef && impl != NULL) { + if (impl->Init(id, numOfWorkThreads)) { + impl->Start(); } } - if(state == kUdpSocketManagerCreate) - { - theUdpSocketManager = - UdpSocketManager::CreateSocketManager(id, numOfWorkThreads); - theUdpSocketManager->Start(); - assert(theUdpSocketManager); - return theUdpSocketManager; - - }else if(state == kUdpSocketManagerDestroy) - { - UdpSocketManager* oldValue = theUdpSocketManager; - theUdpSocketManager = NULL; - if(oldValue) - { - if(oldValue->Stop()) - { - delete static_cast(oldValue); - } - } - return NULL; - } else - { - if(theUdpSocketManager) - { - numOfWorkThreads = theUdpSocketManager->WorkThreads(); - } - } -#else // _WIN32 - if(inc == kUdpSocketManagerInc) - { - if(theUdpSocketManagerCount == 0) - { - state = kUdpSocketManagerCreate; - }else { - if(1 == InterlockedIncrement(&theUdpSocketManagerCount)) - { - // The instance has been destroyed by some other thread. - // Rollback. - InterlockedDecrement(&theUdpSocketManagerCount); - state = kUdpSocketManagerCreate; - } - } - } else - { - WebRtc_Word32 newValue = InterlockedDecrement( - &theUdpSocketManagerCount); - if(newValue == 0) - { - state = kUdpSocketManagerDestroy; - } - } - if(state == kUdpSocketManagerCreate) - { - // Create instance and let whichever thread finishes first assign its - // local copy to the global instance. All other threads reclaim their - // local copy. - UdpSocketManager* newSocketMgr= - UdpSocketManager::CreateSocketManager(id, numOfWorkThreads); - if(1 == InterlockedIncrement(&theUdpSocketManagerCount)) - { - UdpSocketManager* oldValue = (UdpSocketManager*) - InterlockedExchangePointer( - reinterpret_cast(&theUdpSocketManager), - newSocketMgr); - newSocketMgr->Start(); - assert(oldValue == NULL); - assert(theUdpSocketManager); - return newSocketMgr; - - } - InterlockedDecrement(&theUdpSocketManagerCount); - if(newSocketMgr) - { - delete static_cast(newSocketMgr); - } - return NULL; - } else if(state == kUdpSocketManagerDestroy) - { - UdpSocketManager* oldValue = (UdpSocketManager*) - InterlockedExchangePointer( - reinterpret_cast(&theUdpSocketManager), - NULL); - if(oldValue) - { - if(oldValue->Stop()) - { - delete static_cast(oldValue); - } - } - return NULL; - } else - { - if(theUdpSocketManager) - { - numOfWorkThreads = theUdpSocketManager->WorkThreads(); - } - } -#endif // #ifndef _WIN32 - return theUdpSocketManager; + return impl; } UdpSocketManager* UdpSocketManager::Create(const WebRtc_Word32 id, WebRtc_UWord8& numOfWorkThreads) { - return UdpSocketManager::StaticInstance(kUdpSocketManagerInc, id, + return UdpSocketManager::StaticInstance(kAddRef, id, numOfWorkThreads); } void UdpSocketManager::Return() { WebRtc_UWord8 numOfWorkThreads = 0; - UdpSocketManager::StaticInstance(kUdpSocketManagerDec, -1, + UdpSocketManager::StaticInstance(kRelease, -1, numOfWorkThreads); } -UdpSocketManager::UdpSocketManager(const WebRtc_Word32 /*id*/, - WebRtc_UWord8& numOfWorkThreads) - : _numOfWorkThreads(numOfWorkThreads) +UdpSocketManager::UdpSocketManager() : _numOfWorkThreads(0) { } diff --git a/src/modules/udp_transport/source/udp_socket_manager_wrapper.h b/src/modules/udp_transport/source/udp_socket_manager_wrapper.h index 31828a137..e7bd09e31 100644 --- a/src/modules/udp_transport/source/udp_socket_manager_wrapper.h +++ b/src/modules/udp_transport/source/udp_socket_manager_wrapper.h @@ -11,24 +11,13 @@ #ifndef WEBRTC_MODULES_UDP_TRANSPORT_SOURCE_UDP_SOCKET_MANAGER_WRAPPER_H_ #define WEBRTC_MODULES_UDP_TRANSPORT_SOURCE_UDP_SOCKET_MANAGER_WRAPPER_H_ +#include "system_wrappers/interface/static_instance.h" #include "typedefs.h" namespace webrtc { + class UdpSocketWrapper; -enum UdpSocketManagerCount -{ - kUdpSocketManagerDec = 0, - kUdpSocketManagerInc = 1 -}; - -enum UdpSocketManagerState -{ - kUdpSocketManagerExist = 0, - kUdpSocketManagerCreate = 1, - kUdpSocketManagerDestroy = 2 -}; - class UdpSocketManager { public: @@ -36,6 +25,11 @@ public: WebRtc_UWord8& numOfWorkThreads); static void Return(); + // Initializes the socket manager. Returns true if the manager wasn't + // already initialized. + virtual bool Init(WebRtc_Word32 id, + WebRtc_UWord8& numOfWorkThreads) = 0; + virtual WebRtc_Word32 ChangeUniqueId(const WebRtc_Word32 id) = 0; // Start listening to sockets that have been registered via the @@ -52,23 +46,26 @@ public: virtual bool RemoveSocket(UdpSocketWrapper* s) = 0; protected: - UdpSocketManager(const WebRtc_Word32 /*id*/, - WebRtc_UWord8& numOfWorkThreads); - + UdpSocketManager(); virtual ~UdpSocketManager() {} -private: - const WebRtc_UWord8 _numOfWorkThreads; + WebRtc_UWord8 _numOfWorkThreads; // Factory method. - static UdpSocketManager* CreateSocketManager( + static UdpSocketManager* CreateInstance(); + +private: + // Friend function to allow the UDP destructor to be accessed from the + // instance template. + friend UdpSocketManager* + GetStaticInstance(CountOperation count_operation); + + static UdpSocketManager* StaticInstance( + CountOperation count_operation, const WebRtc_Word32 id, WebRtc_UWord8& numOfWorkThreads); - - static UdpSocketManager* StaticInstance(const UdpSocketManagerCount inc, - const WebRtc_Word32 id, - WebRtc_UWord8& numOfWorkThreads); }; + } // namespace webrtc #endif // WEBRTC_MODULES_UDP_TRANSPORT_SOURCE_UDP_SOCKET_MANAGER_WRAPPER_H_ diff --git a/src/system_wrappers/interface/static_instance.h b/src/system_wrappers/interface/static_instance.h new file mode 100644 index 000000000..cd19eea5e --- /dev/null +++ b/src/system_wrappers/interface/static_instance.h @@ -0,0 +1,156 @@ +/* + * Copyright (c) 2011 The WebRTC project authors. All Rights Reserved. + * + * Use of this source code is governed by a BSD-style license + * that can be found in the LICENSE file in the root of the source + * tree. An additional intellectual property rights grant can be found + * in the file PATENTS. All contributing project authors may + * be found in the AUTHORS file in the root of the source tree. + */ + +#ifndef WEBRTC_SYSTEM_WRAPPERS_INTERFACE_STATICINSTANCETEMPLATE_H_ +#define WEBRTC_SYSTEM_WRAPPERS_INTERFACE_STATICINSTANCETEMPLATE_H_ + +#include + +#include "critical_section_wrapper.h" +#ifdef _WIN32 +#include "fix_interlocked_exchange_pointer_windows.h" +#endif + +namespace webrtc { + +enum CountOperation { + kRelease, + kAddRef, + kAddRefNoCreate +}; +enum CreateOperation { + kInstanceExists, + kCreate, + kDestroy +}; + +template +// Construct On First Use idiom. Avoids +// "static initialization order fiasco". +static T* GetStaticInstance(CountOperation count_operation) { + // TODO (hellner): use atomic wrapper instead. + static volatile long instance_count = 0; + static T* volatile instance = NULL; + CreateOperation state = kInstanceExists; +#ifndef _WIN32 + // This memory is staticly allocated once. The application does not try to + // free this memory. This approach is taken to avoid issues with + // destruction order for statically allocated memory. The memory will be + // reclaimed by the OS and memory leak tools will not recognize memory + // reachable from statics leaked so no noise is added by doing this. + static CriticalSectionWrapper* crit_sect( + CriticalSectionWrapper::CreateCriticalSection()); + CriticalSectionScoped lock(*crit_sect); + + if (count_operation == + kAddRefNoCreate && instance_count == 0) { + return NULL; + } + if (count_operation == + kAddRef || + count_operation == kAddRefNoCreate) { + instance_count++; + if (instance_count == 1) { + state = kCreate; + } + } else { + instance_count--; + if (instance_count == 0) { + state = kDestroy; + } + } + if (state == kCreate) { + instance = T::CreateInstance(); + } else if (state == kDestroy) { + T* old_instance = instance; + instance = NULL; + // The state will not change past this point. Release the critical + // section while deleting the object in case it would be blocking on + // access back to this object. (This is the case for the tracing class + // since the thread owned by the tracing class also traces). + // TODO(hellner): this is a bit out of place but here goes, de-couple + // thread implementation with trace implementation. + crit_sect->Leave(); + if (old_instance) { + delete old_instance; + } + // Re-acquire the lock since the scoped critical section will release + // it. + crit_sect->Enter(); + return NULL; + } +#else // _WIN32 + if (count_operation == + kAddRefNoCreate && instance_count == 0) { + return NULL; + } + if (count_operation == kAddRefNoCreate) { + if (1 == InterlockedIncrement(&instance_count)) { + // The instance has been destroyed by some other thread. Rollback. + InterlockedDecrement(&instance_count); + assert(false); + return NULL; + } + // Sanity to catch corrupt state. + if (instance == NULL) { + assert(false); + InterlockedDecrement(&instance_count); + return NULL; + } + } else if (count_operation == kAddRef) { + if (instance_count == 0) { + state = kCreate; + } else { + if (1 == InterlockedIncrement(&instance_count)) { + // InterlockedDecrement because reference count should not be + // updated just yet (that's done when the instance is created). + InterlockedDecrement(&instance_count); + state = kCreate; + } + } + } else { + int newValue = InterlockedDecrement(&instance_count); + if (newValue == 0) { + state = kDestroy; + } + } + + if (state == kCreate) { + // Create instance and let whichever thread finishes first assign its + // local copy to the global instance. All other threads reclaim their + // local copy. + T* new_instance = T::CreateInstance(); + if (1 == InterlockedIncrement(&instance_count)) { + T* old_value = static_cast (InterlockedExchangePointer( + reinterpret_cast(&instance), new_instance)); + assert(old_value == NULL); + assert(instance); + } else { + InterlockedDecrement(&instance_count); + if (new_instance) { + delete static_cast(new_instance); + } + } + return NULL; + } else if (state == kDestroy) { + T* old_value = static_cast (InterlockedExchangePointer( + reinterpret_cast(&instance), NULL)); + if (old_value) { + delete static_cast(old_value); + } + return NULL; + } +#endif // #ifndef _WIN32 + return instance; +} + +} // namspace webrtc + +#endif // WEBRTC_SYSTEM_WRAPPERS_INTERFACE_STATICINSTANCETEMPLATE_H_ diff --git a/src/system_wrappers/source/trace_impl.cc b/src/system_wrappers/source/trace_impl.cc index a2a9c5322..f964f9435 100644 --- a/src/system_wrappers/source/trace_impl.cc +++ b/src/system_wrappers/source/trace_impl.cc @@ -15,7 +15,6 @@ #ifdef _WIN32 #include "trace_windows.h" -#include "fix_interlocked_exchange_pointer_windows.h" #else #include #include @@ -49,156 +48,31 @@ public: #endif // Construct On First Use idiom. Avoids "static initialization order fiasco". -Trace* TraceImpl::StaticInstance(TraceCount inc, const TraceLevel level) +TraceImpl* TraceImpl::StaticInstance(CountOperation count_operation, + const TraceLevel level) { - // TODO (hellner): use atomic wrapper instead. - static volatile long theTraceCount = 0; - static Trace* volatile theTrace = NULL; - - TraceCreate state = WEBRTC_TRACE_EXIST; - - // Sanitys to avoid taking lock unless absolutely necessary (for - // performance reasons). inc == WEBRTC_TRACE_INC_NO_CREATE) implies that - // a message will be written to file. - if(level != kTraceAll && inc == WEBRTC_TRACE_INC_NO_CREATE) + // Sanities to avoid taking lock unless absolutely necessary (for + // performance reasons). + // count_operation == kAddRefNoCreate implies that a message will be + // written to file. + if((level != kTraceAll) && (count_operation == kAddRefNoCreate)) { if(!(level & levelFilter)) { return NULL; } } - -#ifndef _WIN32 - // TODO (pwestin): crtiSect is never reclaimed. Fix memory leak. - static CriticalSectionWrapper* crtiSect( - CriticalSectionWrapper::CreateCriticalSection()); - CriticalSectionScoped lock(*crtiSect); - - if(inc == WEBRTC_TRACE_INC_NO_CREATE && theTraceCount == 0) - { - return NULL; - } - - if(inc == WEBRTC_TRACE_INC || inc == WEBRTC_TRACE_INC_NO_CREATE) - { - theTraceCount++; - if(theTraceCount == 1) - { - state = WEBRTC_TRACE_CREATE; - } - } else { - theTraceCount--; - if(theTraceCount == 0) - { - state = WEBRTC_TRACE_DESTROY; - } - } - if(state == WEBRTC_TRACE_CREATE) - { - theTrace = TraceImpl::CreateTrace(); - - } else if(state == WEBRTC_TRACE_DESTROY) { - Trace* oldValue = theTrace; - theTrace = NULL; - // The lock is held by the scoped critical section. Release the lock - // temporarily so that the trace can be safely deleted. If the lock - // was kept during the delete, e.g. creating and destroying the trace - // too quickly may lead to a deadlock. - // This is due to the fact that starting and stopping a ThreadWrapper - // thread will trigger writing of trace messages. - // TODO (hellner): remove the tight coupling with the thread - // implementation. - crtiSect->Leave(); - if(oldValue) - { - delete static_cast(oldValue); - } - // Re-acquire the lock. - crtiSect->Enter(); - return NULL; - } -#else // _WIN32 - if(inc == WEBRTC_TRACE_INC_NO_CREATE && theTraceCount == 0) - { - return NULL; - } - if(inc == WEBRTC_TRACE_INC_NO_CREATE) - { - if(1 == InterlockedIncrement(&theTraceCount)) - { - // The trace has been destroyed by some other thread. Rollback. - InterlockedDecrement(&theTraceCount); - assert(false); - return NULL; - } - // Sanity to catch corrupt state. - if(theTrace == NULL) - { - assert(false); - InterlockedDecrement(&theTraceCount); - return NULL; - } - } else if(inc == WEBRTC_TRACE_INC) { - if(theTraceCount == 0) - { - state = WEBRTC_TRACE_CREATE; - } else { - if(1 == InterlockedIncrement(&theTraceCount)) - { - // InterlockedDecrement because reference count should not be - // updated just yet (that's done when the trace is created). - InterlockedDecrement(&theTraceCount); - state = WEBRTC_TRACE_CREATE; - } - } - } else { - int newValue = InterlockedDecrement(&theTraceCount); - if(newValue == 0) - { - state = WEBRTC_TRACE_DESTROY; - } - } - - if(state == WEBRTC_TRACE_CREATE) - { - // Create trace and let whichever thread finishes first assign its local - // copy to the global instance. All other threads reclaim their local - // copy. - Trace* newTrace = TraceImpl::CreateTrace(); - if(1 == InterlockedIncrement(&theTraceCount)) - { - Trace* oldValue = (Trace*)InterlockedExchangePointer( - reinterpret_cast(&theTrace), newTrace); - assert(oldValue == NULL); - assert(theTrace); - } else { - InterlockedDecrement(&theTraceCount); - if(newTrace) - { - delete static_cast(newTrace); - } - } - return NULL; - } else if(state == WEBRTC_TRACE_DESTROY) - { - Trace* oldValue = (Trace*)InterlockedExchangePointer( - reinterpret_cast(&theTrace), NULL); - if(oldValue) - { - delete static_cast(oldValue); - } - return NULL; - } -#endif // #ifndef _WIN32 - return theTrace; + TraceImpl* impl = + GetStaticInstance(count_operation); + return impl; } TraceImpl* TraceImpl::GetTrace(const TraceLevel level) { - return (TraceImpl*)StaticInstance(WEBRTC_TRACE_INC_NO_CREATE, level); + return StaticInstance(kAddRefNoCreate, level); } -Trace* TraceImpl::CreateTrace() +TraceImpl* TraceImpl::CreateInstance() { #ifdef WEBRTC_NO_TRACE return new TraceNoop(); @@ -884,12 +758,12 @@ bool TraceImpl::CreateFileName( void Trace::CreateTrace() { - TraceImpl::StaticInstance(WEBRTC_TRACE_INC); + TraceImpl::StaticInstance(kAddRef); } void Trace::ReturnTrace() { - TraceImpl::StaticInstance(WEBRTC_TRACE_DEC); + TraceImpl::StaticInstance(kRelease); } WebRtc_Word32 Trace::SetLevelFilter(WebRtc_UWord32 filter) diff --git a/src/system_wrappers/source/trace_impl.h b/src/system_wrappers/source/trace_impl.h index 42e82fec7..d53dfafc7 100644 --- a/src/system_wrappers/source/trace_impl.h +++ b/src/system_wrappers/source/trace_impl.h @@ -14,23 +14,11 @@ #include "system_wrappers/interface/critical_section_wrapper.h" #include "system_wrappers/interface/event_wrapper.h" #include "system_wrappers/interface/file_wrapper.h" +#include "system_wrappers/interface/static_instance.h" #include "system_wrappers/interface/trace.h" #include "system_wrappers/interface/thread_wrapper.h" namespace webrtc { -enum TraceCount -{ - WEBRTC_TRACE_DEC = 0, - WEBRTC_TRACE_INC = 1, - WEBRTC_TRACE_INC_NO_CREATE = 2 -}; - -enum TraceCreate -{ - WEBRTC_TRACE_EXIST = 0, - WEBRTC_TRACE_CREATE = 1, - WEBRTC_TRACE_DESTROY = 2 -}; // TODO (pwestin) WEBRTC_TRACE_MAX_QUEUE needs to be tweaked // TODO (hellner) the buffer should be close to how much the system can write to @@ -58,12 +46,9 @@ class TraceImpl : public Trace public: virtual ~TraceImpl(); - static Trace* CreateTrace(); + static TraceImpl* CreateInstance(); static TraceImpl* GetTrace(const TraceLevel level = kTraceAll); - static Trace* StaticInstance(TraceCount inc, - const TraceLevel level = kTraceAll); - WebRtc_Word32 SetTraceFileImpl(const WebRtc_Word8* fileName, const bool addFileCounter); WebRtc_Word32 TraceFileImpl( @@ -81,6 +66,9 @@ public: protected: TraceImpl(); + static TraceImpl* StaticInstance(CountOperation count_operation, + const TraceLevel level = kTraceAll); + // OS specific implementations virtual WebRtc_Word32 AddThreadId(char* traceMessage) const = 0; virtual WebRtc_Word32 AddTime(char* traceMessage, @@ -93,6 +81,8 @@ protected: bool Process(); private: + friend class Trace; + WebRtc_Word32 AddLevel(char* szMessage, const TraceLevel level) const; WebRtc_Word32 AddModuleAndId(char* traceMessage, const TraceModule module,