Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use simple mutexes for cases where RDLOCK is not native. #23

Closed
wants to merge 10 commits into from
18 changes: 8 additions & 10 deletions apc_cache.c
Expand Up @@ -36,6 +36,7 @@
#include "php_scandir.h"
#include "SAPI.h"
#include "TSRM.h"
#include "php_main.h"
#include "ext/standard/md5.h"
#include "ext/standard/php_var.h"
#include "ext/standard/php_smart_str.h"
Expand Down Expand Up @@ -179,8 +180,6 @@ PHP_APCU_API void apc_cache_remove_slot(apc_cache_t* cache, apc_cache_slot_t** s
PHP_APCU_API void apc_cache_gc(apc_cache_t* cache TSRMLS_DC)
{
apc_cache_slot_t** slot;

time_t now;

/* This function scans the list of removed cache entries and deletes any
* entry whose reference count is zero or that has been on the gc
Expand All @@ -201,12 +200,11 @@ PHP_APCU_API void apc_cache_gc(apc_cache_t* cache TSRMLS_DC)
{
slot = &cache->header->gc;

now = time(0);

while (*slot != NULL) {
int gc_sec = cache->gc_ttl ? (now - (*slot)->dtime) : 0;
time_t now = time(0);
time_t gc_sec = cache->gc_ttl ? (now - (*slot)->dtime) : 0;

if (!(*slot)->value->ref_count || gc_sec > cache->gc_ttl) {
if (!(*slot)->value->ref_count || gc_sec > (time_t)cache->gc_ttl) {
apc_cache_slot_t* dead = *slot;

/* good ol' whining */
Expand Down Expand Up @@ -572,7 +570,7 @@ PHP_APCU_API zend_bool apc_cache_preload(apc_cache_t* cache, const char *path TS
/* {{{ apc_cache_release */
PHP_APCU_API void apc_cache_release(apc_cache_t* cache, apc_cache_entry_t* entry TSRMLS_DC)
{
entry->ref_count--;
ATOMIC_DEC(entry->ref_count);
}
/* }}} */

Expand Down Expand Up @@ -875,7 +873,7 @@ PHP_APCU_API zend_bool apc_cache_insert(apc_cache_t* cache,
* access ttl on it and removing entries that haven't been accessed for ttl seconds and secondly
* we see if the entry has a hard ttl on it and remove it if it has been around longer than its ttl
*/
if((cache->ttl && (*slot)->atime < (t - cache->ttl)) ||
if((cache->ttl && (time_t)(*slot)->atime < (t - (time_t)cache->ttl)) ||
((*slot)->value->ttl && (time_t) ((*slot)->ctime + (*slot)->value->ttl) < t)) {
apc_cache_remove_slot(cache, slot TSRMLS_CC);
continue;
Expand Down Expand Up @@ -951,7 +949,7 @@ PHP_APCU_API apc_cache_entry_t* apc_cache_find(apc_cache_t* cache, char *strkey,

/* Otherwise we are fine, increase counters and return the cache entry */
(*slot)->nhits++;
(*slot)->value->ref_count++;
ATOMIC_INC((*slot)->value->ref_count);
(*slot)->atime = t;

/* set cache num hits */
Expand Down Expand Up @@ -1621,7 +1619,7 @@ PHP_APCU_API zval* apc_cache_info(apc_cache_t* cache, zend_bool limited TSRMLS_D
zval *gc = NULL;
zval *slots = NULL;
apc_cache_slot_t* p;
int i, j;
zend_ulong i, j;

if(!cache) {
return NULL;
Expand Down
4 changes: 4 additions & 0 deletions apc_cache.h
Expand Up @@ -40,6 +40,10 @@
# include "apc_cache_api.h"
#endif

#ifdef APC_FULL_BC
# define APC_CACHE_IS_USER(ct, ct_len) ((ct_len) == 4 && !strncasecmp("user", (ct), 4))
#endif

#endif

/*
Expand Down
22 changes: 22 additions & 0 deletions apc_iterator.c
Expand Up @@ -155,6 +155,9 @@ static zend_object_value apc_iterator_create(zend_class_entry *ce TSRMLS_DC) {
#endif
iterator->obj.guards = NULL;
iterator->initialized = 0;
iterator->stack = NULL;
iterator->regex_len = 0;
iterator->search_hash = NULL;
retval.handle = zend_objects_store_put(iterator, apc_iterator_destroy, apc_iterator_free, NULL TSRMLS_CC);
retval.handlers = &apc_iterator_object_handlers;

Expand Down Expand Up @@ -309,10 +312,21 @@ PHP_METHOD(apc_iterator, __construct) {
long chunk_size=0;
zval *search = NULL;
long list = APC_LIST_ACTIVE;
#if defined(APC_FULL_BC) && APC_FULL_BC
/* these are ignored */
char *cache_type;
int cache_type_len;
#endif

#if defined(APC_FULL_BC) && APC_FULL_BC
if (zend_parse_parameters(ZEND_NUM_ARGS() TSRMLS_CC, "s|zlll", &cache_type, &cache_type_len, &search, &format, &chunk_size, &list) == FAILURE) {
return;
}
#else
if (zend_parse_parameters(ZEND_NUM_ARGS() TSRMLS_CC, "|zlll", &search, &format, &chunk_size, &list) == FAILURE) {
return;
}
#endif

if (!APCG(enabled)) {
apc_error("APC must be enabled to use APCIterator." TSRMLS_CC);
Expand All @@ -336,6 +350,12 @@ PHP_METHOD(apc_iterator, __construct) {
apc_warning("APCIterator invalid list type." TSRMLS_CC);
return;
}
#if defined(APC_FULL_BC) && APC_FULL_BC
if (!APC_CACHE_IS_USER(cache_type, cache_type_len)) {
iterator->initialized = 0;
return;
}
#endif

iterator->slot_idx = 0;
iterator->stack_idx = 0;
Expand Down Expand Up @@ -550,7 +570,9 @@ PHP_METHOD(apc_iterator, getTotalCount) {

PHP_APC_ARGINFO
ZEND_BEGIN_ARG_INFO_EX(arginfo_apc_iterator___construct, 0, 0, 1)
#if defined(APC_FULL_BC) && APC_FULL_BC
ZEND_ARG_INFO(0, cache)
#endif
ZEND_ARG_INFO(0, search)
ZEND_ARG_INFO(0, format)
ZEND_ARG_INFO(0, chunk_size)
Expand Down
18 changes: 6 additions & 12 deletions apc_lock.c
Expand Up @@ -88,10 +88,7 @@ PHP_APCU_API zend_bool apc_lock_create(apc_lock_t *lock TSRMLS_DC) {
#ifndef PHP_WIN32
# ifndef APC_NATIVE_RWLOCK
{
/* Emulated */
pthread_mutex_init(&lock->read, &apc_lock_attr);
pthread_mutex_init(&lock->write, &apc_lock_attr);

pthread_mutex_init(lock, &apc_lock_attr);
return 1;
}
# else
Expand All @@ -110,7 +107,7 @@ PHP_APCU_API zend_bool apc_lock_create(apc_lock_t *lock TSRMLS_DC) {
PHP_APCU_API zend_bool apc_lock_rlock(apc_lock_t *lock TSRMLS_DC) {
#ifndef PHP_WIN32
# ifndef APC_NATIVE_RWLOCK
pthread_mutex_lock(&lock->read);
pthread_mutex_lock(lock);
# else
pthread_rwlock_rdlock(lock);
# endif
Expand All @@ -123,8 +120,7 @@ PHP_APCU_API zend_bool apc_lock_rlock(apc_lock_t *lock TSRMLS_DC) {
PHP_APCU_API zend_bool apc_lock_wlock(apc_lock_t *lock TSRMLS_DC) {
#ifndef PHP_WIN32
# ifndef APC_NATIVE_RWLOCK
pthread_mutex_lock(&lock->read);
pthread_mutex_lock(&lock->write);
pthread_mutex_lock(lock);
# else
pthread_rwlock_wrlock(lock);
# endif
Expand All @@ -137,8 +133,7 @@ PHP_APCU_API zend_bool apc_lock_wlock(apc_lock_t *lock TSRMLS_DC) {
PHP_APCU_API zend_bool apc_lock_wunlock(apc_lock_t *lock TSRMLS_DC) {
#ifndef PHP_WIN32
# ifndef APC_NATIVE_RWLOCK
pthread_mutex_unlock(&lock->read);
pthread_mutex_unlock(&lock->write);
pthread_mutex_unlock(lock);
# else
pthread_rwlock_unlock(lock);
# endif
Expand All @@ -151,7 +146,7 @@ PHP_APCU_API zend_bool apc_lock_wunlock(apc_lock_t *lock TSRMLS_DC) {
PHP_APCU_API zend_bool apc_lock_runlock(apc_lock_t *lock TSRMLS_DC) {
#ifndef PHP_WIN32
# ifndef APC_NATIVE_RWLOCK
pthread_mutex_unlock(&lock->read);
pthread_mutex_unlock(lock);
# else
pthread_rwlock_unlock(lock);
# endif
Expand All @@ -164,8 +159,7 @@ PHP_APCU_API zend_bool apc_lock_runlock(apc_lock_t *lock TSRMLS_DC) {
PHP_APCU_API void apc_lock_destroy(apc_lock_t *lock TSRMLS_DC) {
#ifndef PHP_WIN32
# ifndef APC_NATIVE_RWLOCK
pthread_mutex_destroy(&lock->read);
pthread_mutex_destroy(&lock->write);
pthread_mutex_destroy(lock);
# else
pthread_rwlock_destroy(lock);
# endif
Expand Down
20 changes: 16 additions & 4 deletions apc_lock_api.h
Expand Up @@ -36,10 +36,7 @@
# ifdef APC_NATIVE_RWLOCK
typedef pthread_rwlock_t apc_lock_t;
# else
typedef struct _apc_lock_t {
pthread_mutex_t read;
pthread_mutex_t write;
} apc_lock_t;
typedef pthread_mutex_t apc_lock_t;
# endif
#else
/* XXX kernel lock mode only for now, compatible through all the wins, add more ifdefs for others */
Expand Down Expand Up @@ -85,5 +82,20 @@ PHP_APCU_API void apc_lock_destroy(apc_lock_t *lock TSRMLS_DC); /* }}} */
#define APC_RLOCK(o) RLOCK(&(o)->lock)
#define APC_RUNLOCK(o) RUNLOCK(&(o)->lock) /* }}} */

/* {{{ atomic operations : rdlocks have race conditions on refcounts without these */
#if APC_NATIVE_RWLOCK
# ifdef PHP_WIN32
# define ATOMIC_INC(a) (void)InterlockedIncrement(&(a))
# define ATOMIC_DEC(a) (void)InterlockedDecrement(&(a))
# else
# define ATOMIC_INC(a) (void)__sync_add_and_fetch(&(a), 1)
# define ATOMIC_DEC(a) (void)__sync_sub_and_fetch(&(a), 1)
# endif
#else /* mutex locks, no more races */
# define ATOMIC_INC(a) (void)(++(a));
# define ATOMIC_DEC(a) (void)(--(a));
#endif
/* }}} */

#endif

18 changes: 18 additions & 0 deletions config.m4
Expand Up @@ -82,6 +82,24 @@ if test "$PHP_APCU" != "no"; then
AC_DEFINE(APC_NATIVE_RWLOCK, 1, [ ])
])
fi

AC_CACHE_CHECK([whether the target compiler supports builtin atomics], PHP_cv_APC_GCC_ATOMICS, [
AC_TRY_LINK([],[
int foo = 0;
__sync_fetch_and_add(&foo, 1);
__sync_bool_compare_and_swap(&foo, 0, 1);
return __sync_fetch_and_add(&foo, 1);
],
[PHP_cv_APC_GCC_ATOMICS=yes],
[PHP_cv_APC_GCC_ATOMICS=no])
])
if test "x${PHP_cv_APC_GCC_ATOMICS}" != "xno"; then
AC_DEFINE(HAVE_ATOMIC_OPERATIONS, 1,
[Define this if your target compiler supports builtin atomics])
else
dnl using rdlock without atomics is unsafe and will not work right
AC_DEFINE(APC_NATIVE_RWLOCK, 0, [ ])
fi

AC_CHECK_FUNCS(sigaction)
AC_CACHE_CHECK(for union semun, php_cv_semun,
Expand Down
1 change: 1 addition & 0 deletions config.w32
Expand Up @@ -21,6 +21,7 @@ if(PHP_APCU != 'no')
AC_DEFINE('APC_SRWLOCK_KERNEL', 1);
ADD_FLAG('CFLAGS_APC', '/D WIN32_ONLY_COMPILER=1');

AC_DEFINE('HAVE_ATOMIC_OPERATIONS', 1);
AC_DEFINE('HAVE_APCU', 1);

if (PHP_APC_BC != 'no')
Expand Down
2 changes: 2 additions & 0 deletions package.xml
Expand Up @@ -29,6 +29,7 @@
<license uri="http://www.php.net/license">PHP License</license>
<notes>
- Fixed bug #15 APC compatibility option broken. (Anatol)
- Fixed bug #20 APCu's APCIterator constructor is not compatable with APC. (Anatol)
</notes>
<contents>
<dir name="/">
Expand Down Expand Up @@ -62,6 +63,7 @@
<file name="tests/iterator_005.phpt" role="test" />
<file name="tests/iterator_006.phpt" role="test" />
<file name="tests/iterator_007.phpt" role="test" />
<file name="tests/iterator_008.phpt" role="test" />
<file name="tests/php_5_3_ns.inc" role="test" />
<file name="tests/server_test.inc" role="test" />
<file name="tests/skipif.inc" role="test" />
Expand Down
30 changes: 28 additions & 2 deletions php_apc.c
Expand Up @@ -391,10 +391,29 @@ static PHP_RINIT_FUNCTION(apcu)
}
/* }}} */

#ifdef APC_FULL_BC
/* {{{ proto void apc_clear_cache([string cache]) */
PHP_FUNCTION(apcu_clear_cache)
{
char *ignored;
int ignlen = 0;

if (zend_parse_parameters(ZEND_NUM_ARGS() TSRMLS_CC, "|s", &ignored, &ignlen) == FAILURE) {
return;
}

if (0 == ignlen || APC_CACHE_IS_USER(ignored, ignlen)) {
apc_cache_clear(apc_user_cache TSRMLS_CC);
}

RETURN_TRUE;
}
/* }}} */
#else
/* {{{ proto void apc_clear_cache() */
PHP_FUNCTION(apcu_clear_cache)
{
if (zend_parse_parameters(ZEND_NUM_ARGS() TSRMLS_CC, "") == FAILURE) {
if (zend_parse_parameters_none() == FAILURE) {
return;
}

Expand All @@ -403,6 +422,7 @@ PHP_FUNCTION(apcu_clear_cache)
RETURN_TRUE;
}
/* }}} */
#endif

/* {{{ proto array apc_cache_info([bool limited]) */
PHP_FUNCTION(apcu_cache_info)
Expand Down Expand Up @@ -1271,10 +1291,16 @@ ZEND_BEGIN_ARG_INFO_EX(arginfo_apcu_cache_info, 0, 0, 0)
ZEND_ARG_INFO(0, limited)
ZEND_END_ARG_INFO()

#ifdef APC_FULL_BC
PHP_APC_ARGINFO
ZEND_BEGIN_ARG_INFO_EX(arginfo_apcu_clear_cache, 0, 0, 0)
ZEND_ARG_INFO(0, info)
ZEND_ARG_INFO(0, cache)
ZEND_END_ARG_INFO()
#else
PHP_APC_ARGINFO
ZEND_BEGIN_ARG_INFO_EX(arginfo_apcu_clear_cache, 0, 0, 0)
ZEND_END_ARG_INFO()
#endif

PHP_APC_ARGINFO
ZEND_BEGIN_ARG_INFO_EX(arginfo_apcu_sma_info, 0, 0, 0)
Expand Down
6 changes: 5 additions & 1 deletion tests/iterator_001.phpt
Expand Up @@ -7,7 +7,11 @@ apc.enabled=1
apc.enable_cli=1
--FILE--
<?php
$it = new APCIterator();
if (APCU_APC_FULL_BC) {
$it = new APCIterator('user');
} else {
$it = new APCIterator();
}
for($i = 0; $i < 41; $i++) {
apc_store("key$i", "value$i");
}
Expand Down
6 changes: 5 additions & 1 deletion tests/iterator_002.phpt
Expand Up @@ -9,7 +9,11 @@ apc.file_update_protection=0
--FILE--
<?php

$it = new APCIterator('/key[0-9]0/');
if (APCU_APC_FULL_BC) {
$it = new APCIterator('user', '/key[0-9]0/');
} else {
$it = new APCIterator('/key[0-9]0/');
}
for($i = 0; $i < 41; $i++) {
apc_store("key$i", "value$i");
}
Expand Down
6 changes: 5 additions & 1 deletion tests/iterator_003.phpt
Expand Up @@ -9,7 +9,11 @@ apc.file_update_protection=0
--FILE--
<?php

$it = new APCIterator(NULL, APC_ITER_ALL, 10);
if (APCU_APC_FULL_BC) {
$it = new APCIterator('user', NULL, APC_ITER_ALL, 10);
} else {
$it = new APCIterator(NULL, APC_ITER_ALL, 10);
}
for($i = 0; $i < 41; $i++) {
apc_store("key$i", "value$i");
}
Expand Down
6 changes: 5 additions & 1 deletion tests/iterator_004.phpt
Expand Up @@ -9,7 +9,11 @@ apc.file_update_protection=0
--FILE--
<?php

$it = new APCIterator('/key[0-9]0/', APC_ITER_ALL, 1, APC_LIST_ACTIVE);
if (APCU_APC_FULL_BC) {
$it = new APCIterator('user', '/key[0-9]0/', APC_ITER_ALL, 1, APC_LIST_ACTIVE);
} else {
$it = new APCIterator('/key[0-9]0/', APC_ITER_ALL, 1, APC_LIST_ACTIVE);
}
for($i = 0; $i < 41; $i++) {
apc_store("key$i", "value$i");
}
Expand Down