Skip to content

Commit

Permalink
Fixed more synchronisation issues during SHM reload
Browse files Browse the repository at this point in the history
  • Loading branch information
@dstogov @weltling
dstogov authored and weltling committed Feb 26, 2016
1 parent e65cc72 commit ecea7d3
Show file tree
Hide file tree
Showing 3 changed files with 141 additions and 116 deletions.
218 changes: 118 additions & 100 deletions ext/opcache/ZendAccelerator.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -220,6 +220,104 @@ static ZEND_INI_MH(accel_include_path_on_modify)
return ret;
}

static inline void accel_restart_enter(void)
{
#ifdef ZEND_WIN32
INCREMENT(restart_in);
#else
static const FLOCK_STRUCTURE(restart_in_progress, F_WRLCK, SEEK_SET, 2, 1);

if (fcntl(lock_file, F_SETLK, &restart_in_progress) == -1) {
zend_accel_error(ACCEL_LOG_DEBUG, "RestartC(+1): %s (%d)", strerror(errno), errno);
}
#endif
ZCSG(restart_in_progress) = 1;
}

static inline void accel_restart_leave(void)
{
#ifdef ZEND_WIN32
ZCSG(restart_in_progress) = 0;
DECREMENT(restart_in);
#else
static const FLOCK_STRUCTURE(restart_finished, F_UNLCK, SEEK_SET, 2, 1);

ZCSG(restart_in_progress) = 0;
if (fcntl(lock_file, F_SETLK, &restart_finished) == -1) {
zend_accel_error(ACCEL_LOG_DEBUG, "RestartC(-1): %s (%d)", strerror(errno), errno);
}
#endif
}

static inline int accel_restart_is_active(void)
{
if (ZCSG(restart_in_progress)) {
#ifndef ZEND_WIN32
FLOCK_STRUCTURE(restart_check, F_WRLCK, SEEK_SET, 2, 1);

if (fcntl(lock_file, F_GETLK, &restart_check) == -1) {
zend_accel_error(ACCEL_LOG_DEBUG, "RestartC: %s (%d)", strerror(errno), errno);
return FAILURE;
}
if (restart_check.l_type == F_UNLCK) {
ZCSG(restart_in_progress) = 0;
return 0;
} else {
return 1;
}
#else
return LOCKVAL(restart_in) != 0;
#endif
}
return 0;
}

/* Creates a read lock for SHM access */
static inline int accel_activate_add(void)
{
#ifdef ZEND_WIN32
INCREMENT(mem_usage);
#else
static const FLOCK_STRUCTURE(mem_usage_lock, F_RDLCK, SEEK_SET, 1, 1);

if (fcntl(lock_file, F_SETLK, &mem_usage_lock) == -1) {
zend_accel_error(ACCEL_LOG_DEBUG, "UpdateC(+1): %s (%d)", strerror(errno), errno);
return FAILURE;
}
#endif
return SUCCESS;
}

/* Releases a lock for SHM access */
static inline void accel_deactivate_sub(void)
{
#ifdef ZEND_WIN32
if (ZCG(counted)) {
DECREMENT(mem_usage);
ZCG(counted) = 0;
}
#else
static const FLOCK_STRUCTURE(mem_usage_unlock, F_UNLCK, SEEK_SET, 1, 1);

if (fcntl(lock_file, F_SETLK, &mem_usage_unlock) == -1) {
zend_accel_error(ACCEL_LOG_DEBUG, "UpdateC(-1): %s (%d)", strerror(errno), errno);
}
#endif
}

static inline void accel_unlock_all(void)
{
#ifdef ZEND_WIN32
accel_deactivate_sub();
#else
static const FLOCK_STRUCTURE(mem_usage_unlock_all, F_UNLCK, SEEK_SET, 0, 0);

if (fcntl(lock_file, F_SETLK, &mem_usage_unlock_all) == -1) {
zend_accel_error(ACCEL_LOG_DEBUG, "UnlockAll: %s (%d)", strerror(errno), errno);
}
#endif
}

/* Interned strings support */
static zend_string *(*orig_new_interned_string)(zend_string *str);
static void (*orig_interned_strings_snapshot)(void);
Expand Down Expand Up @@ -291,6 +389,12 @@ static zend_string *accel_find_interned_string(zend_string *str)
/* this is already an interned string */
return str;
}
if (!ZCG(counted)) {
if (accel_activate_add() == FAILURE) {
return str;
}
ZCG(counted) = 1;
}

h = zend_string_hash_val(str);
nIndex = h | ZCSG(interned_strings).nTableMask;
Expand Down Expand Up @@ -492,102 +596,6 @@ static void accel_use_shm_interned_strings(void)
}
#endif

static inline void accel_restart_enter(void)
{
#ifdef ZEND_WIN32
INCREMENT(restart_in);
#else
static const FLOCK_STRUCTURE(restart_in_progress, F_WRLCK, SEEK_SET, 2, 1);

if (fcntl(lock_file, F_SETLK, &restart_in_progress) == -1) {
zend_accel_error(ACCEL_LOG_DEBUG, "RestartC(+1): %s (%d)", strerror(errno), errno);
}
#endif
ZCSG(restart_in_progress) = 1;
}

static inline void accel_restart_leave(void)
{
#ifdef ZEND_WIN32
ZCSG(restart_in_progress) = 0;
DECREMENT(restart_in);
#else
static const FLOCK_STRUCTURE(restart_finished, F_UNLCK, SEEK_SET, 2, 1);

ZCSG(restart_in_progress) = 0;
if (fcntl(lock_file, F_SETLK, &restart_finished) == -1) {
zend_accel_error(ACCEL_LOG_DEBUG, "RestartC(-1): %s (%d)", strerror(errno), errno);
}
#endif
}

static inline int accel_restart_is_active(void)
{
if (ZCSG(restart_in_progress)) {
#ifndef ZEND_WIN32
FLOCK_STRUCTURE(restart_check, F_WRLCK, SEEK_SET, 2, 1);

if (fcntl(lock_file, F_GETLK, &restart_check) == -1) {
zend_accel_error(ACCEL_LOG_DEBUG, "RestartC: %s (%d)", strerror(errno), errno);
return FAILURE;
}
if (restart_check.l_type == F_UNLCK) {
ZCSG(restart_in_progress) = 0;
return 0;
} else {
return 1;
}
#else
return LOCKVAL(restart_in) != 0;
#endif
}
return 0;
}

/* Creates a read lock for SHM access */
static inline void accel_activate_add(void)
{
#ifdef ZEND_WIN32
INCREMENT(mem_usage);
#else
static const FLOCK_STRUCTURE(mem_usage_lock, F_RDLCK, SEEK_SET, 1, 1);

if (fcntl(lock_file, F_SETLK, &mem_usage_lock) == -1) {
zend_accel_error(ACCEL_LOG_DEBUG, "UpdateC(+1): %s (%d)", strerror(errno), errno);
}
#endif
}

/* Releases a lock for SHM access */
static inline void accel_deactivate_sub(void)
{
#ifdef ZEND_WIN32
if (ZCG(counted)) {
DECREMENT(mem_usage);
ZCG(counted) = 0;
}
#else
static const FLOCK_STRUCTURE(mem_usage_unlock, F_UNLCK, SEEK_SET, 1, 1);

if (fcntl(lock_file, F_SETLK, &mem_usage_unlock) == -1) {
zend_accel_error(ACCEL_LOG_DEBUG, "UpdateC(-1): %s (%d)", strerror(errno), errno);
}
#endif
}

static inline void accel_unlock_all(void)
{
#ifdef ZEND_WIN32
accel_deactivate_sub();
#else
static const FLOCK_STRUCTURE(mem_usage_unlock_all, F_UNLCK, SEEK_SET, 0, 0);

if (fcntl(lock_file, F_SETLK, &mem_usage_unlock_all) == -1) {
zend_accel_error(ACCEL_LOG_DEBUG, "UnlockAll: %s (%d)", strerror(errno), errno);
}
#endif
}

#ifndef ZEND_WIN32
static inline void kill_all_lockers(struct flock *mem_usage_check)
{
Expand Down Expand Up @@ -1563,7 +1571,9 @@ zend_op_array *file_cache_compile_file(zend_file_handle *file_handle, int type)
}
}

SHM_UNPROTECT();
persistent_script = zend_file_cache_script_load(file_handle);
SHM_PROTECT();
if (persistent_script) {
/* see bug #15471 (old BTS) */
if (persistent_script->full_path) {
Expand All @@ -1589,8 +1599,6 @@ zend_op_array *file_cache_compile_file(zend_file_handle *file_handle, int type)
}
zend_file_handle_dtor(file_handle);

persistent_script->dynamic_members.last_used = ZCG(request_time);

if (persistent_script->ping_auto_globals_mask) {
zend_accel_set_auto_globals(persistent_script->ping_auto_globals_mask);
}
Expand Down Expand Up @@ -1709,8 +1717,15 @@ zend_op_array *persistent_compile_file(zend_file_handle *file_handle, int type)
* each execution)
*/
if (!ZCG(counted)) {
if (accel_activate_add() == FAILURE) {
#ifdef HAVE_OPCACHE_FILE_CACHE
if (ZCG(accel_directives).file_cache) {
return file_cache_compile_file(file_handle, type);
}
#endif
return accelerator_orig_compile_file(file_handle, type);
}
ZCG(counted) = 1;
accel_activate_add();
}

SHM_UNPROTECT();
Expand Down Expand Up @@ -2874,13 +2889,16 @@ int accelerator_shm_read_lock(void)
} else {
/* here accelerator is active but we do not hold SHM lock. This means restart was scheduled
or is in progress now */
accel_activate_add(); /* acquire usage lock */
if (accel_activate_add() == FAILURE) { /* acquire usage lock */
return FAILURE;
}
/* Now if we weren't inside restart, restart would not begin until we remove usage lock */
if (ZCSG(restart_in_progress)) {
/* we already were inside restart this means it's not safe to touch shm */
accel_deactivate_now(); /* drop usage lock */
return FAILURE;
}
ZCG(counted) = 1;
}
return SUCCESS;
}
Expand Down
28 changes: 14 additions & 14 deletions ext/opcache/zend_file_cache.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -132,13 +132,8 @@ static int zend_file_cache_flock(int fd, int type)
} else { \
ZEND_ASSERT(IS_SERIALIZED(ptr)); \
(ptr) = (void*)((char*)buf + (size_t)(ptr)); \
/* script->corrupted shows if the script in SHM or not */ \
if (EXPECTED(!script->corrupted)) { \
GC_FLAGS(ptr) |= IS_STR_INTERNED | IS_STR_PERMANENT; \
} else { \
GC_FLAGS(ptr) |= IS_STR_INTERNED; \
GC_FLAGS(ptr) &= ~IS_STR_PERMANENT; \
} \
GC_FLAGS(ptr) |= IS_STR_INTERNED; \
GC_FLAGS(ptr) &= ~IS_STR_PERMANENT; \
} \
} \
} while (0)
Expand Down Expand Up @@ -224,15 +219,17 @@ static void *zend_file_cache_unserialize_interned(zend_string *str, int in_shm)
zend_string *ret;

str = (zend_string*)((char*)ZCG(mem) + ((size_t)(str) & ~Z_UL(1)));
ret = accel_new_interned_string(str);
if (ret == str) {
/* String wasn't interned but we will use it as interned anyway */
if (in_shm) {
GC_FLAGS(ret) |= IS_STR_INTERNED | IS_STR_PERMANENT;
} else {
if (in_shm) {
ret = accel_new_interned_string(str);
if (ret == str) {
/* String wasn't interned but we will use it as interned anyway */
GC_FLAGS(ret) |= IS_STR_INTERNED;
GC_FLAGS(ret) &= ~IS_STR_PERMANENT;
}
} else {
ret = str;
GC_FLAGS(ret) |= IS_STR_INTERNED;
GC_FLAGS(ret) &= ~IS_STR_PERMANENT;
}
return ret;
}
Expand Down Expand Up @@ -1303,7 +1300,9 @@ zend_persistent_script *zend_file_cache_script_load(zend_file_handle *file_handl
return NULL;
}

if (!ZCG(accel_directives).file_cache_only) {
if (!ZCG(accel_directives).file_cache_only &&
!ZCSG(restart_in_progress) &&
accelerator_shm_read_lock() == SUCCESS) {
/* exclusive lock */
zend_shared_alloc_lock();

Expand Down Expand Up @@ -1357,6 +1356,7 @@ zend_persistent_script *zend_file_cache_script_load(zend_file_handle *file_handl

if (cache_it) {
script->dynamic_members.checksum = zend_accel_script_checksum(script);
script->dynamic_members.last_used = ZCG(request_time);

zend_accel_hash_update(&ZCSG(hash), ZSTR_VAL(script->full_path), ZSTR_LEN(script->full_path), 0, script);

Expand Down
11 changes: 9 additions & 2 deletions ext/opcache/zend_persist_calc.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,9 @@
# define ADD_STRING(str) ADD_DUP_SIZE((str), _ZSTR_STRUCT_SIZE(ZSTR_LEN(str)))

# define ADD_INTERNED_STRING(str, do_free) do { \
if (!IS_ACCEL_INTERNED(str)) { \
if (ZCG(current_persistent_script)->corrupted) { \
ADD_STRING(str); \
} else if (!IS_ACCEL_INTERNED(str)) { \
zend_string *tmp = accel_new_interned_string(str); \
if (tmp != (str)) { \
if (do_free) { \
Expand Down Expand Up @@ -126,7 +128,7 @@ static void zend_persist_zval_calc(zval *z)
case IS_CONSTANT:
flags = Z_GC_FLAGS_P(z) & ~ (IS_STR_PERSISTENT | IS_STR_INTERNED | IS_STR_PERMANENT);
ADD_INTERNED_STRING(Z_STR_P(z), 0);
if (!Z_REFCOUNTED_P(z)) {
if (ZSTR_IS_INTERNED(Z_STR_P(z))) {
Z_TYPE_FLAGS_P(z) &= ~ (IS_TYPE_REFCOUNTED | IS_TYPE_COPYABLE);
}
Z_GC_FLAGS_P(z) |= flags;
Expand Down Expand Up @@ -385,11 +387,15 @@ uint zend_accel_script_persist_calc(zend_persistent_script *new_persistent_scrip
new_persistent_script->size = 0;
new_persistent_script->arena_mem = NULL;
new_persistent_script->arena_size = 0;
new_persistent_script->corrupted = 0;
ZCG(current_persistent_script) = new_persistent_script;

ADD_DUP_SIZE(new_persistent_script, sizeof(zend_persistent_script));
if (key) {
ADD_DUP_SIZE(key, key_length + 1);
} else {
/* script is not going to be saved in SHM */
new_persistent_script->corrupted = 1;
}
ADD_STRING(new_persistent_script->full_path);

Expand All @@ -408,6 +414,7 @@ uint zend_accel_script_persist_calc(zend_persistent_script *new_persistent_scrip
#endif

new_persistent_script->size += new_persistent_script->arena_size;
new_persistent_script->corrupted = 0;

ZCG(current_persistent_script) = NULL;

Expand Down

0 comments on commit ecea7d3

Please sign in to comment.