Fixed more synchronisation issues during SHM reload

diff --git a/ext/opcache/ZendAccelerator.c b/ext/opcache/ZendAccelerator.c
index addfe87ab5fdce38ec5b77709871649ff281f330..cd91dfbc4b4e8ef53bac8fe22001bde533acc3d5 100644 (file)
--- a/ext/opcache/ZendAccelerator.c
+++ b/ext/opcache/ZendAccelerator.c
@@ -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);
@@ -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;
@@ -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)
 {
@@ -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) {
@@ -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);
                }
@@ -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();
@@ -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;
 }

diff --git a/ext/opcache/zend_file_cache.c b/ext/opcache/zend_file_cache.c
index 410921f80080c13685eab7ecc7f52c650e7f793a..b65abca2254a149b641da1dd83d82862bdbf3990 100644 (file)
--- a/ext/opcache/zend_file_cache.c
+++ b/ext/opcache/zend_file_cache.c
@@ -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)
@@ -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;
 }
@@ -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();
 
@@ -1357,6 +1356,7 @@ use_process_mem:
 
        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);
 

diff --git a/ext/opcache/zend_persist_calc.c b/ext/opcache/zend_persist_calc.c
index 6c721196a49d7056e2f6c2bfb8a24f0570ef0984..25ba69bee8bf63c122beefeab904cdcd8583ae79 100644 (file)
--- a/ext/opcache/zend_persist_calc.c
+++ b/ext/opcache/zend_persist_calc.c
@@ -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) { \
@@ -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;
@@ -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);
 
@@ -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;