This patch fixes locking and reentrancy bugs in ElectricFence 2.2.2 so that it may safely be used in threaded programs. I emailed it to Bruce Perens when I wrote it in 2001 but never received any reply from him, so here it is. If anyone knows how to get hold of him, please let me know and/or pass this on. I see other newer 3rd party releases of ElectricFence still have the same old broken locking code in it, which is a shame since this utility is otherwise very useful. -- Howard Chu diff -ur ElectricFence-2.2.2/efence.c ElectricFence-2.2.2b/efence.c --- ElectricFence-2.2.2/efence.c 1999-04-12 18:00:49.000000000 -0700 +++ ElectricFence-2.2.2b/efence.c 2003-05-24 04:49:20.000000000 -0700 @@ -34,10 +34,12 @@ #include #include #include -#ifdef USE_SEMAPHORE +#if defined(USE_PTHREAD) || defined(USE_SEMAPHORE) # include +#ifdef USE_SEMAPHORE # include #endif +#endif #ifdef malloc #undef malloc @@ -179,17 +181,11 @@ * these routines. * Also, we use semEnabled as a boolean to see if we should be * using the semaphore. - * semThread is set to the thread id of the thread that currently - * has the semaphore so that when/if it tries to get the semaphore - * again (realloc calling malloc/free) - nothing will happen to the - * semaphore. - * semDepth is used to keep track of how many times the same thread - * gets the semaphore - so we know when it is actually freed. */ static sem_t EF_sem = { 0 }; static int semEnabled = 0; -static pthread_t semThread = (pthread_t) 0; -static int semDepth = 0; +#elif defined(USE_PTHREAD) +static pthread_mutex_t EF_mutex; #endif /* @@ -198,6 +194,11 @@ */ static size_t bytesPerPage = 0; +static void * +EF_memalign(size_t alignment, size_t userSize, int dolock); +static void +EF_free(void *address, int dolock); + static void lock() { @@ -206,20 +207,11 @@ if (!semEnabled) return; - /* Do we already have the semaphore? */ - if (semThread == pthread_self()) { - /* Increment semDepth - push one stack level */ - semDepth++; - return; - } - /* Wait for the semaphore. */ while (sem_wait(&EF_sem) < 0) /* try again */; - - /* Let everyone know who has the semaphore. */ - semThread = pthread_self(); - semDepth++; +#elif defined(USE_PTHREAD) + pthread_mutex_lock(&EF_mutex); #endif /* USE_SEMAPHORE */ } @@ -231,31 +223,10 @@ if (!semEnabled) return; - /* Do we have the semaphore? Cannot free it if we don't. */ - if (semThread != pthread_self()) { - if ( semThread == 0 ) - EF_InternalError( - "Releasing semaphore that wasn't locked."); - - else - EF_InternalError( - "Semaphore doesn't belong to thread."); - } - - /* Make sure this is positive as well. */ - if (semDepth <= 0) - EF_InternalError("Semaphore depth"); - /* Decrement semDepth - popping one stack level */ - semDepth--; - - /* Only actually free the semaphore when we've reached the top */ - /* of our call stack. */ - if (semDepth == 0) { - /* Zero this before actually free'ing the semaphore. */ - semThread = (pthread_t) 0; - if (sem_post(&EF_sem) < 0) - EF_InternalError("Failed to post the semaphore."); - } + if (sem_post(&EF_sem) < 0) + EF_InternalError("Failed to post the semaphore."); +#elif defined(USE_PTHREAD) + pthread_mutex_unlock(&EF_mutex); #endif /* USE_SEMAPHORE */ } @@ -277,6 +248,8 @@ if (sem_init(&EF_sem, 0, 1) >= 0) { semEnabled = 1; } +#elif defined(USE_PTHREAD) + pthread_mutex_init(&EF_mutex, NULL); #endif lock(); @@ -412,7 +385,7 @@ noAllocationListProtection = 1; internalUse = 1; - newAllocation = malloc(newSize); + newAllocation = EF_memalign(EF_ALIGNMENT, newSize, 0); memcpy(newAllocation, allocationList, allocationListSize); memset(&(((char *)newAllocation)[allocationListSize]), 0, bytesPerPage); @@ -421,7 +394,7 @@ slotCount += slotsPerPage; unUsedSlots += slotsPerPage; - free(oldAllocation); + EF_free(oldAllocation, 0); /* * Keep access to the allocation list open at this point, because @@ -453,8 +426,8 @@ * so that it won't waste even more. It's slow, but thrashing because your * working set is too big for a system's RAM is even slower. */ -extern C_LINKAGE void * -memalign(size_t alignment, size_t userSize) +static void * +EF_memalign(size_t alignment, size_t userSize, int dolock) { register Slot * slot; register size_t count; @@ -467,7 +440,8 @@ if ( allocationList == 0 ) initialize(); - lock(); + if (dolock) + lock(); if ( userSize == 0 && !EF_ALLOW_MALLOC_0 ) EF_Abort("Allocating 0 bytes, probably a bug."); @@ -665,11 +639,18 @@ if ( !internalUse ) Page_DenyAccess(allocationList, allocationListSize); - release(); + if (dolock) + release(); return address; } +extern C_LINKAGE void * +memalign(size_t alignment, size_t userSize) +{ + return EF_memalign(alignment, userSize, 1); +} + /* * Find the slot structure for a user address. */ @@ -728,6 +709,12 @@ extern C_LINKAGE void free(void * address) { + EF_free(address, 1); +} + +static void +EF_free(void *address, int dolock) +{ Slot * slot; Slot * previousSlot = 0; Slot * nextSlot = 0; @@ -738,7 +725,8 @@ if ( allocationList == 0 ) EF_Abort("free() called before first malloc()."); - lock(); + if (dolock) + lock(); if ( !noAllocationListProtection ) Page_AllowAccess(allocationList, allocationListSize); @@ -810,7 +798,8 @@ if ( !noAllocationListProtection ) Page_DenyAccess(allocationList, allocationListSize); - release(); + if (dolock) + release(); } extern C_LINKAGE void * @@ -823,7 +812,7 @@ lock(); - newBuffer = malloc(newSize); + newBuffer = EF_memalign(EF_ALIGNMENT, newSize, 0); if ( oldBuffer ) { size_t size; @@ -847,7 +836,7 @@ if ( size > 0 ) memcpy(newBuffer, oldBuffer, size); - free(oldBuffer); + EF_free(oldBuffer, 0); noAllocationListProtection = 0; Page_DenyAccess(allocationList, allocationListSize); @@ -856,9 +845,7 @@ /* Internal memory was re-protected in free() */ } - release(); - return newBuffer; } Only in ElectricFence-2.2.2b: efence.o Only in ElectricFence-2.2.2b: eftest Only in ElectricFence-2.2.2b: eftest.o Only in ElectricFence-2.2.2b: libefence.a Only in ElectricFence-2.2.2b: libefence.so.0.0 diff -ur ElectricFence-2.2.2/Makefile ElectricFence-2.2.2b/Makefile --- ElectricFence-2.2.2/Makefile 1999-04-13 10:22:49.000000000 -0700 +++ ElectricFence-2.2.2b/Makefile 2003-05-24 04:51:56.000000000 -0700 @@ -1,5 +1,5 @@ PIC= -fPIC -CFLAGS= -g -DUSE_SEMAPHORE $(PIC) +CFLAGS= -g -DUSE_PTHREAD $(PIC) LIBS= -lpthread prefix=/usr Only in ElectricFence-2.2.2b: man Only in ElectricFence-2.2.2b: page.o diff -ur ElectricFence-2.2.2/print.c ElectricFence-2.2.2b/print.c --- ElectricFence-2.2.2/print.c 1999-04-11 16:29:21.000000000 -0700 +++ ElectricFence-2.2.2b/print.c 2001-11-25 16:29:07.000000000 -0800 @@ -111,7 +111,7 @@ break; case 'c': { - char c = va_arg(args, char); + char c = va_arg(args, int); (void) write(2, &c, 1); } Only in ElectricFence-2.2.2b: print.o Only in ElectricFence-2.2.2b: RCS Only in ElectricFence-2.2.2b: tstheap Only in ElectricFence-2.2.2b: tstheap.o