From 180b24f2a0dbf4476e1baf2fc535ebd422a5abce Mon Sep 17 00:00:00 2001 From: Jean Guyomarc'h Date: Tue, 23 Aug 2016 16:59:10 +0200 Subject: [PATCH] eina: fixtures on OSX semaphores So actually there is quite a big issue with semaphores on OSX. We use (named) POSIX semaphores, but this was a (my) mistake... I'll fix it later... The real issue is that named semaphore are persistants: when the program dies, it stays alive. This is pretty bad with eina_debug_monitor because we create a semaphore we never release, due to a wild thread... This leak of semaphores went unnoticed before commit 4a40ff95defa5fa7e6164459c50e674b53cddaf4 because the name of the semaphore was unique per process, and overriden when another process was launched. This was very bad, but saved us from overflowing the semaphore pool. It is now overflowed pretty fast when building a lot EFL, because of Eolian that runs A LOT! So that's one problem that still needs to be fixed, by using OSX' own semaphores (see T4423). Another big issue, which is now fixed is that the buffer in which we generated the semaphore ID was too small, and therefore we were reduced to one shared semaphore for a whole process... This buffer has been now set to 31 characters, which seems to be the maximum length of a semaphore ID. So now things are better, but still with a deadly issue. --- src/lib/eina/eina_inline_lock_posix.x | 29 ++++++++------------------- 1 file changed, 8 insertions(+), 21 deletions(-) diff --git a/src/lib/eina/eina_inline_lock_posix.x b/src/lib/eina/eina_inline_lock_posix.x index 1b9e73fb8f..b52a6473bf 100644 --- a/src/lib/eina/eina_inline_lock_posix.x +++ b/src/lib/eina/eina_inline_lock_posix.x @@ -96,22 +96,13 @@ typedef Eina_Lock Eina_Spinlock; #if defined(EINA_HAVE_OSX_SEMAPHORE) /* OSX supports only named semaphores. * So, we need to be able to generate a unique string identifier for each - * semaphore we want to create. - * It seems reasonable to use a counter, which is incremented each time a - * semaphore is created. However, it needs to be atomic... - * It would be easier if we were using C11 with stdatomic, but I guess it - * will just be fine without. - * That's why there are two static variables below the struct */ + * semaphore we want to create. */ struct _Eina_Semaphore { sem_t *sema; - char name[16]; + char name[32]; }; typedef struct _Eina_Semaphore Eina_Semaphore; - -static unsigned int _sem_ctr = 0; -static Eina_Spinlock _sem_ctr_lock = 0; // 0: not locked - #else typedef sem_t Eina_Semaphore; #endif @@ -848,14 +839,9 @@ eina_semaphore_new(Eina_Semaphore *sem, int count_init) #if defined(EINA_HAVE_OSX_SEMAPHORE) /* Atomic increment to generate the unique identifier */ - eina_spinlock_take(&_sem_ctr_lock); - ++_sem_ctr; - eina_spinlock_release(&_sem_ctr_lock); - - snprintf(sem->name, sizeof(sem->name), "/eina_sem_%x-%x_%x_%x_%x_%x", - (unsigned int)getpid(), _sem_ctr, - (unsigned int)rand(), (unsigned int)rand(), - (unsigned int)rand(), (unsigned int)rand()); + snprintf(sem->name, sizeof(sem->name), "/eina%x%x%x", + (unsigned int)getpid(), (unsigned int)rand(), (unsigned int)rand()); + sem->name[sizeof(sem->name) - 1] = '\0'; sem_unlink(sem->name); sem->sema = sem_open(sem->name, O_CREAT | O_EXCL, 0600, count_init); return (sem->sema == SEM_FAILED) ? EINA_FALSE : EINA_TRUE; @@ -871,8 +857,9 @@ eina_semaphore_free(Eina_Semaphore *sem) return EINA_FALSE; #if defined(EINA_HAVE_OSX_SEMAPHORE) - return ((sem_close(sem->sema) == 0) && - (sem_unlink(sem->name)) == 0) ? EINA_TRUE : EINA_FALSE; + const int closed = sem_close(sem->sema); + const int unlinked = sem_unlink(sem->name); + return ((closed == 0) && (unlinked == 0)) ? EINA_TRUE : EINA_FALSE; #else return (sem_destroy(sem) == 0) ? EINA_TRUE : EINA_FALSE; #endif