From 5e12d9f8fe9533f97a04570d44582c1714227a8c Mon Sep 17 00:00:00 2001 From: Andre Rudlaff Date: Fri, 26 Apr 2013 18:55:36 +0200 Subject: [PATCH 1/3] don't use named semaphore for syncing image loading thread Named semaphores are unique across process bounds, therefore a semaphore left in an invalid state may cause locks on other processes. The code has been exchanged by using pthread mutex and condition variables --- cocos2dx/textures/CCTextureCache.cpp | 77 +++++++--------------------- 1 file changed, 18 insertions(+), 59 deletions(-) diff --git a/cocos2dx/textures/CCTextureCache.cpp b/cocos2dx/textures/CCTextureCache.cpp index 19599bb487..4f9fa961ed 100644 --- a/cocos2dx/textures/CCTextureCache.cpp +++ b/cocos2dx/textures/CCTextureCache.cpp @@ -42,7 +42,6 @@ THE SOFTWARE. #include #include #include -#include using namespace std; @@ -64,26 +63,15 @@ typedef struct _ImageInfo static pthread_t s_loadingThread; +static pthread_mutex_t s_SleepMutex; +static pthread_cond_t s_SleepCondition; + static pthread_mutex_t s_asyncStructQueueMutex; static pthread_mutex_t s_ImageInfoMutex; -static sem_t* s_pSem = NULL; + static unsigned long s_nAsyncRefCount = 0; -#if CC_TARGET_PLATFORM == CC_PLATFORM_IOS - #define CC_ASYNC_TEXTURE_CACHE_USE_NAMED_SEMAPHORE 1 -#else - #define CC_ASYNC_TEXTURE_CACHE_USE_NAMED_SEMAPHORE 0 -#endif - - -#if CC_ASYNC_TEXTURE_CACHE_USE_NAMED_SEMAPHORE - #define CC_ASYNC_TEXTURE_CACHE_SEMAPHORE "ccAsync" -#else - static sem_t s_sem; -#endif - - static bool need_quit = false; static std::queue* s_pAsyncStructQueue = NULL; @@ -122,25 +110,19 @@ static void* loadImage(void* data) // create autorelease pool for iOS CCThread thread; thread.createAutoreleasePool(); - - // wait for rendering thread to ask for loading if s_pAsyncStructQueue is empty - int semWaitRet = sem_wait(s_pSem); - if( semWaitRet < 0 ) - { - CCLOG( "CCTextureCache async thread semaphore error: %s\n", strerror( errno ) ); - break; - } std::queue *pQueue = s_pAsyncStructQueue; - pthread_mutex_lock(&s_asyncStructQueueMutex);// get async struct from queue if (pQueue->empty()) { pthread_mutex_unlock(&s_asyncStructQueueMutex); - if (need_quit) + if (need_quit) { break; - else + } + else { + pthread_cond_wait(&s_SleepCondition, &s_SleepMutex); continue; + } } else { @@ -182,17 +164,12 @@ static void* loadImage(void* data) pthread_mutex_unlock(&s_ImageInfoMutex); } - if( s_pSem != NULL ) + if( s_pAsyncStructQueue != NULL ) { - #if CC_ASYNC_TEXTURE_CACHE_USE_NAMED_SEMAPHORE - sem_unlink(CC_ASYNC_TEXTURE_CACHE_SEMAPHORE); - sem_close(s_pSem); - #else - sem_destroy(s_pSem); - #endif - s_pSem = NULL; delete s_pAsyncStructQueue; + s_pAsyncStructQueue = NULL; delete s_pImageQueue; + s_pImageQueue = NULL; } return 0; @@ -223,11 +200,8 @@ CCTextureCache::~CCTextureCache() { CCLOGINFO("cocos2d: deallocing CCTextureCache."); need_quit = true; - if (s_pSem != NULL) - { - sem_post(s_pSem); - } - + + pthread_cond_signal(&s_SleepCondition); CC_SAFE_RELEASE(m_pTextures); } @@ -277,30 +251,15 @@ void CCTextureCache::addImageAsync(const char *path, CCObject *target, SEL_CallF } // lazy init - if (s_pSem == NULL) + if (s_pAsyncStructQueue == NULL) { -#if CC_ASYNC_TEXTURE_CACHE_USE_NAMED_SEMAPHORE - s_pSem = sem_open(CC_ASYNC_TEXTURE_CACHE_SEMAPHORE, O_CREAT, 0644, 0); - if( s_pSem == SEM_FAILED ) - { - CCLOG( "CCTextureCache async thread semaphore init error: %s\n", strerror( errno ) ); - s_pSem = NULL; - return; - } -#else - int semInitRet = sem_init(&s_sem, 0, 0); - if( semInitRet < 0 ) - { - CCLOG( "CCTextureCache async thread semaphore init error: %s\n", strerror( errno ) ); - return; - } - s_pSem = &s_sem; -#endif s_pAsyncStructQueue = new queue(); s_pImageQueue = new queue(); pthread_mutex_init(&s_asyncStructQueueMutex, NULL); pthread_mutex_init(&s_ImageInfoMutex, NULL); + pthread_mutex_init(&s_SleepMutex, NULL); + pthread_cond_init(&s_SleepCondition, NULL); pthread_create(&s_loadingThread, NULL, loadImage, NULL); need_quit = false; @@ -329,7 +288,7 @@ void CCTextureCache::addImageAsync(const char *path, CCObject *target, SEL_CallF s_pAsyncStructQueue->push(data); pthread_mutex_unlock(&s_asyncStructQueueMutex); - sem_post(s_pSem); + pthread_cond_signal(&s_SleepCondition); } void CCTextureCache::addImageAsyncCallBack(float dt) From 43615fe0a7ca769732425bca741c1959b54ef166 Mon Sep 17 00:00:00 2001 From: Andre Rudlaff Date: Sat, 27 Apr 2013 17:44:33 +0200 Subject: [PATCH 2/3] don't use named semaphore in HttpClient this patch removes using named semaphores for Http requests. This also destroys the mutexes from CCTextureCache when the thread is destroyed --- cocos2dx/textures/CCTextureCache.cpp | 5 ++ extensions/network/HttpClient.cpp | 69 ++++++++-------------------- 2 files changed, 24 insertions(+), 50 deletions(-) diff --git a/cocos2dx/textures/CCTextureCache.cpp b/cocos2dx/textures/CCTextureCache.cpp index 4f9fa961ed..c5b43ae678 100644 --- a/cocos2dx/textures/CCTextureCache.cpp +++ b/cocos2dx/textures/CCTextureCache.cpp @@ -170,6 +170,11 @@ static void* loadImage(void* data) s_pAsyncStructQueue = NULL; delete s_pImageQueue; s_pImageQueue = NULL; + + pthread_mutex_destroy(&s_asyncStructQueueMutex); + pthread_mutex_destroy(&s_ImageInfoMutex); + pthread_mutex_destroy(&s_SleepMutex); + pthread_cond_destroy(&s_SleepCondition); } return 0; diff --git a/extensions/network/HttpClient.cpp b/extensions/network/HttpClient.cpp index 0473502ff8..47cb9429d1 100644 --- a/extensions/network/HttpClient.cpp +++ b/extensions/network/HttpClient.cpp @@ -28,7 +28,6 @@ #include #include -#include #include #include "curl/curl.h" @@ -38,21 +37,12 @@ NS_CC_EXT_BEGIN static pthread_t s_networkThread; static pthread_mutex_t s_requestQueueMutex; static pthread_mutex_t s_responseQueueMutex; -static sem_t * s_pSem = NULL; + +static pthread_mutex_t s_SleepMutex; +static pthread_cond_t s_SleepCondition; + static unsigned long s_asyncRequestCount = 0; -#if CC_TARGET_PLATFORM == CC_PLATFORM_IOS || CC_TARGET_PLATFORM == CC_PLATFORM_MAC -#define CC_ASYNC_HTTPREQUEST_USE_NAMED_SEMAPHORE 1 -#else -#define CC_ASYNC_HTTPREQUEST_USE_NAMED_SEMAPHORE 0 -#endif - -#if CC_ASYNC_HTTPREQUEST_USE_NAMED_SEMAPHORE -#define CC_ASYNC_HTTPREQUEST_SEMAPHORE "ccHttpAsync" -#else -static sem_t s_sem; -#endif - #if (CC_TARGET_PLATFORM == CC_PLATFORM_WIN32) typedef int int32_t; #endif @@ -96,13 +86,6 @@ static void* networkThread(void *data) while (true) { - // Wait for http request tasks from main thread - int semWaitRet = sem_wait(s_pSem); - if (semWaitRet < 0) { - CCLog("HttpRequest async thread semaphore error: %s", strerror(errno)); - break; - } - if (need_quit) { break; @@ -122,6 +105,8 @@ static void* networkThread(void *data) if (NULL == request) { + // Wait for http request tasks from main thread + pthread_cond_wait(&s_SleepCondition, &s_SleepMutex); continue; } @@ -188,21 +173,18 @@ static void* networkThread(void *data) pthread_mutex_unlock(&s_requestQueueMutex); s_asyncRequestCount -= s_requestQueue->count(); - if (s_pSem != NULL) { -#if CC_ASYNC_HTTPREQUEST_USE_NAMED_SEMAPHORE - sem_unlink(CC_ASYNC_HTTPREQUEST_SEMAPHORE); - sem_close(s_pSem); -#else - sem_destroy(s_pSem); -#endif - - s_pSem = NULL; + if (s_requestQueue != NULL) { pthread_mutex_destroy(&s_requestQueueMutex); pthread_mutex_destroy(&s_responseQueueMutex); + pthread_mutex_destroy(&s_SleepMutex); + pthread_cond_destroy(&s_SleepCondition); + s_requestQueue->release(); + s_requestQueue = NULL; s_responseQueue->release(); + s_responseQueue = NULL; } pthread_exit(NULL); @@ -416,8 +398,8 @@ CCHttpClient::~CCHttpClient() { need_quit = true; - if (s_pSem != NULL) { - sem_post(s_pSem); + if (s_requestQueue != NULL) { + pthread_cond_signal(&s_SleepCondition); } s_pHttpClient = NULL; @@ -426,25 +408,9 @@ CCHttpClient::~CCHttpClient() //Lazy create semaphore & mutex & thread bool CCHttpClient::lazyInitThreadSemphore() { - if (s_pSem != NULL) { + if (s_requestQueue != NULL) { return true; } else { -#if CC_ASYNC_HTTPREQUEST_USE_NAMED_SEMAPHORE - s_pSem = sem_open(CC_ASYNC_HTTPREQUEST_SEMAPHORE, O_CREAT, 0644, 0); - if (s_pSem == SEM_FAILED) { - CCLog("Open HttpRequest Semaphore failed"); - s_pSem = NULL; - return false; - } -#else - int semRet = sem_init(&s_sem, 0, 0); - if (semRet < 0) { - CCLog("Init HttpRequest Semaphore failed"); - return false; - } - - s_pSem = &s_sem; -#endif s_requestQueue = new CCArray(); s_requestQueue->init(); @@ -455,6 +421,9 @@ bool CCHttpClient::lazyInitThreadSemphore() pthread_mutex_init(&s_requestQueueMutex, NULL); pthread_mutex_init(&s_responseQueueMutex, NULL); + pthread_mutex_init(&s_SleepMutex, NULL); + pthread_cond_init(&s_SleepCondition, NULL); + pthread_create(&s_networkThread, NULL, networkThread, NULL); pthread_detach(s_networkThread); @@ -486,7 +455,7 @@ void CCHttpClient::send(CCHttpRequest* request) pthread_mutex_unlock(&s_requestQueueMutex); // Notify thread start to work - sem_post(s_pSem); + pthread_cond_signal(&s_SleepCondition); } // Poll and notify main thread if responses exists in queue From 38e96cbe24fcbaf3c04bffc52f9cdecfca638798 Mon Sep 17 00:00:00 2001 From: minggo Date: Sat, 27 Apr 2013 23:53:31 +0800 Subject: [PATCH 3/3] Update AUTHORS --- AUTHORS | 1 + 1 file changed, 1 insertion(+) diff --git a/AUTHORS b/AUTHORS index 7d303f69d2..ba28b9e069 100644 --- a/AUTHORS +++ b/AUTHORS @@ -101,6 +101,7 @@ Developers: Fixed potential crash in CCScheduler::removeHashElement. Fixed potential crash in CCSaxParser. Added kResolutionFixedHeight and kResolutionFixedWidth resolution policy. + Use pthread mutex and condition variable instead of pthred semaphore to load image asynchronizely. Nat Weiss (iphonegamekit.com) author of Mac port