Skip to content

Commit

Permalink
Clarify cache/sma cleanup operations
Browse files Browse the repository at this point in the history
Rename destroy/cleanup to detach to clarify that this is only about
detaching local instances, not about cleaning up anything inside
SHM. In particular remove calls to lock destruction.
  • Loading branch information
nikic committed Nov 19, 2018
1 parent 5c869d1 commit ce47957
Show file tree
Hide file tree
Showing 7 changed files with 25 additions and 29 deletions.
15 changes: 6 additions & 9 deletions apc_cache.c
Original file line number Diff line number Diff line change
Expand Up @@ -649,20 +649,17 @@ PHP_APCU_API void apc_cache_entry_release(apc_cache_t *cache, apc_cache_entry_t
}
/* }}} */

/* {{{ apc_cache_destroy */
PHP_APCU_API void apc_cache_destroy(apc_cache_t* cache)
/* {{{ apc_cache_detach */
PHP_APCU_API void apc_cache_detach(apc_cache_t *cache)
{
/* Important: This function should not clean up anything that's in shared memory,
* only detach our process-local use of it. In particular locks cannot be destroyed
* here. */

if (!cache) {
return;
}

/* destroy lock */
DESTROY_LOCK(&cache->header->lock);

/* XXX this is definitely a leak, but freeing this causes all the apache
children to freeze. It might be because the segment is shared between
several processes. To figure out is how to free this safely. */
/*apc_sma_free(cache->shmaddr);*/
free(cache);
}
/* }}} */
Expand Down
8 changes: 4 additions & 4 deletions apc_cache_api.h
Original file line number Diff line number Diff line change
Expand Up @@ -128,11 +128,11 @@ PHP_APCU_API apc_cache_t* apc_cache_create(
PHP_APCU_API zend_bool apc_cache_preload(apc_cache_t* cache, const char* path);

/*
* apc_cache_destroy releases any OS resources associated with a cache object.
* Under apache, this function can be safely called by the child processes
* when they exit.
* apc_cache_detach detaches from the shared memory cache and cleans up
* local allocations. Under apache, this function can be safely called by
* the child processes when they exit.
*/
PHP_APCU_API void apc_cache_destroy(apc_cache_t* cache);
PHP_APCU_API void apc_cache_detach(apc_cache_t* cache);

/*
* apc_cache_clear empties a cache. This can safely be called at any time.
Expand Down
4 changes: 2 additions & 2 deletions apc_lock.c
Original file line number Diff line number Diff line change
Expand Up @@ -114,7 +114,7 @@ PHP_APCU_API zend_bool apc_lock_runlock(apc_lock_t *lock) {
}

PHP_APCU_API void apc_lock_destroy(apc_lock_t *lock) {
/* nothing */
pthread_rwlock_destroy(lock);
}

#elif defined(APC_LOCK_RECURSIVE)
Expand Down Expand Up @@ -173,7 +173,7 @@ PHP_APCU_API zend_bool apc_lock_runlock(apc_lock_t *lock) {
}

PHP_APCU_API void apc_lock_destroy(apc_lock_t *lock) {
/* nothing */
pthread_mutex_destroy(lock);
}

#elif defined(APC_SPIN_LOCK)
Expand Down
4 changes: 2 additions & 2 deletions apc_signal.c
Original file line number Diff line number Diff line change
Expand Up @@ -52,12 +52,12 @@ static void apc_clear_cache(int signo, siginfo_t *siginfo, void *context);
extern apc_cache_t* apc_user_cache;

/* {{{ apc_core_unmap
* Coredump signal handler, unmaps shm and calls previously installed handlers
* Coredump signal handler, detached from shm and calls previously installed handlers
*/
static void apc_core_unmap(int signo, siginfo_t *siginfo, void *context)
{
if (apc_user_cache) {
apc_sma_cleanup(apc_user_cache->sma);
apc_sma_detach(apc_user_cache->sma);
}
apc_rehandle_signal(signo, siginfo, context);

Expand Down
9 changes: 5 additions & 4 deletions apc_sma.c
Original file line number Diff line number Diff line change
Expand Up @@ -361,21 +361,22 @@ PHP_APCU_API void apc_sma_init(apc_sma_t* sma, void** data, apc_sma_expunge_f ex
}
}

PHP_APCU_API void apc_sma_cleanup(apc_sma_t* sma) {
PHP_APCU_API void apc_sma_detach(apc_sma_t* sma) {
/* Important: This function should not clean up anything that's in shared memory,
* only detach our process-local use of it. In particular locks cannot be destroyed
* here. */
uint i;

assert(sma->initialized);
sma->initialized = 0;

for (i = 0; i < sma->num; i++) {
// Disabled until the lock is removed from shared memory.
//SMA_DESTROY_LOCK(&SMA_LCK(sma, i));
#if APC_MMAP
apc_unmap(&sma->segs[i]);
#else
apc_shm_detach(&sma->segs[i]);
#endif
}
sma->initialized = 0;

free(sma->segs);
}
Expand Down
6 changes: 3 additions & 3 deletions apc_sma_api.h
Original file line number Diff line number Diff line change
Expand Up @@ -87,9 +87,9 @@ PHP_APCU_API void apc_sma_init(
int32_t num, zend_ulong size, char *mask);

/*
* apc_sma_api_cleanup will free the sma allocator
*/
PHP_APCU_API void apc_sma_cleanup(apc_sma_t* sma);
* apc_sma_detach will detach from shared memory and cleanup local allocations.
*/
PHP_APCU_API void apc_sma_detach(apc_sma_t* sma);

/*
* apc_smap_api_malloc will allocate a block from the sma of the given size
Expand Down
8 changes: 3 additions & 5 deletions php_apc.c
Original file line number Diff line number Diff line change
Expand Up @@ -297,11 +297,9 @@ static PHP_MSHUTDOWN_FUNCTION(apcu)
/* only shut down if APC is enabled */
if (APCG(enabled)) {
if (APCG(initialized)) {

/* destroy cache pointer */
apc_cache_destroy(apc_user_cache);
/* cleanup shared memory */
apc_sma_cleanup(&apc_sma);
/* Detach cache and shared memory allocator from shared memory. */
apc_cache_detach(apc_user_cache);
apc_sma_detach(&apc_sma);

APCG(initialized) = 0;
}
Expand Down

0 comments on commit ce47957

Please sign in to comment.