Fix GH-8739: OPcache restart corrupts SHM in threaded SAPIs

ext/opcache/ZendAccelerator.c

@@ -333,13 +333,93 @@ static inline int accel_restart_is_active(void)
 	return 0;
 }
 
+#if defined(ZTS) && !defined(ZEND_WIN32)
+/* fcntl() locks belong to the process, so under ZTS they cannot tell which threads
+ * use SHM (GH-8739). The threads of this process that use SHM are counted here, and
+ * the process holds the reader lock (byte 1) whenever the count is not zero, so other
+ * processes still see it. A restart needs a zero count and a write lock on byte 1.
+ * The 0 <-> 1 transitions and the restart are serialized by accel_zts_readers_mutex,
+ * other changes of the count are lock-free. */
+static MUTEX_T accel_zts_readers_mutex = NULL;
+static zend_atomic_int accel_zts_readers;
+static time_t accel_zts_next_warning;
+
+static bool accel_zts_lock_reader_byte(int type)
+{
+	struct flock lock;
+
+	lock.l_type = type;
+	lock.l_whence = SEEK_SET;
+	lock.l_start = 1;
+	lock.l_len = 1;
+
+	return fcntl(lock_file, F_SETLK, &lock) != -1;
+}
+
+/* Must only be called from RINIT; on success the caller sets ZCG(counted) */
+static bool accel_zts_reader_enter(void)
+{
+	int count;
+
+#ifndef HAVE_NO_ATOMICS
+	/* the process already holds the reader lock: only a positive count may grow */
+	count = zend_atomic_int_load_ex(&accel_zts_readers);
+	while (count > 0) {
+		if (zend_atomic_int_compare_exchange_ex(&accel_zts_readers, &count, count + 1)) {
+			return true;
+		}
+	}
+#endif
+
+	tsrm_mutex_lock(accel_zts_readers_mutex);
+	count = zend_atomic_int_load_ex(&accel_zts_readers);
+	/* fails while another process restarts the cache (it holds the write lock) */
+	if (count == 0 && !accel_zts_lock_reader_byte(F_RDLCK)) {
+		zend_accel_error(ACCEL_LOG_DEBUG, "UpdateC(+1):  %s (%d)", strerror(errno), errno);
+		tsrm_mutex_unlock(accel_zts_readers_mutex);
+		return false;
+	}
+	while (!zend_atomic_int_compare_exchange_ex(&accel_zts_readers, &count, count + 1));
+	tsrm_mutex_unlock(accel_zts_readers_mutex);
+	return true;
+}
+
+/* Releases one reference; does not use ZCG() as it is also called from accel_globals_dtor() */
+static void accel_zts_reader_leave(void)
+{
+	int count;
+
+#ifndef HAVE_NO_ATOMICS
+	count = zend_atomic_int_load_ex(&accel_zts_readers);
+	while (count > 1) {
+		if (zend_atomic_int_compare_exchange_ex(&accel_zts_readers, &count, count - 1)) {
+			return;
+		}
+	}
+#endif
+
+	tsrm_mutex_lock(accel_zts_readers_mutex);
+	count = zend_atomic_int_load_ex(&accel_zts_readers);
+	while (!zend_atomic_int_compare_exchange_ex(&accel_zts_readers, &count, count - 1));
+	if (count == 1) {
+		/* the last reader of this process; unlock before another thread may relock */
+		accel_zts_lock_reader_byte(F_UNLCK);
+	}
+	tsrm_mutex_unlock(accel_zts_readers_mutex);
+}
+#endif
+
 /* Creates a read lock for SHM access */
 static inline zend_result accel_activate_add(void)
 {
 #ifdef ZEND_WIN32
 	SHM_UNPROTECT();
 	INCREMENT(mem_usage);
 	SHM_PROTECT();
+#elif defined(ZTS)
+	/* A thread becomes a reader only in RINIT, see accel_zts_reader_enter(). A request
+	 * that was not admitted there must not start to use SHM later. */
+	return FAILURE;
 #else
 	struct flock mem_usage_lock;
 
@@ -366,6 +446,8 @@ static inline void accel_deactivate_sub(void)
 		ZCG(counted) = false;
 		SHM_PROTECT();
 	}
+#elif defined(ZTS)
+	/* nothing to release: accel_activate_add() never succeeds under ZTS */
 #else
 	struct flock mem_usage_unlock;
 
@@ -384,6 +466,13 @@ static inline void accel_unlock_all(void)
 {
 #ifdef ZEND_WIN32
 	accel_deactivate_sub();
+#elif defined(ZTS)
+	/* Release this thread's reference only. Never unlock the whole file: fcntl() locks
+	 * belong to the process, and the SHM write lock (byte 0) and the restart lock
+	 * (byte 2) may be held by other threads. */
+	if (ZCG(counted)) {
+		accel_zts_reader_leave();
+	}
 #else
 	if (lock_file == -1) {
 		return;
@@ -934,6 +1023,46 @@ static inline bool accel_is_inactive(void)
 	return false;
 }
 
+#if defined(ZTS) && !defined(ZEND_WIN32)
+/* Replaces accel_is_inactive() under ZTS; called under zend_shared_alloc_lock().
+ * On success the readers mutex and the write lock on byte 1 are held until
+ * accel_zts_restart_end(): no thread of this process can become a reader (the count is
+ * zero and may only grow from zero under the mutex), and other processes can neither
+ * hold the reader lock nor take it. */
+static bool accel_zts_restart_begin(void)
+{
+	int readers;
+
+	tsrm_mutex_lock(accel_zts_readers_mutex);
+	readers = zend_atomic_int_load_ex(&accel_zts_readers);
+	if (readers == 0 && accel_zts_lock_reader_byte(F_WRLCK)) {
+		return true;
+	}
+	tsrm_mutex_unlock(accel_zts_readers_mutex);
+
+	if (readers == 0) {
+		/* readers in other processes: keep the force_restart_timeout handling */
+		(void) accel_is_inactive();
+	} else if (ZCG(accel_directives).force_restart_timeout
+			&& ZCSG(force_restart_time)
+			&& time(NULL) >= ZCSG(force_restart_time)
+			&& time(NULL) >= accel_zts_next_warning) {
+		/* threads cannot be killed, and this process must not kill itself */
+		zend_accel_error(ACCEL_LOG_WARNING, "Restart delayed for %ld seconds by %d threads of process %d still using the cache",
+			(long)(time(NULL) - ZCSG(force_restart_time) + ZCG(accel_directives).force_restart_timeout), readers, getpid());
+		accel_zts_next_warning = time(NULL) + ZCG(accel_directives).force_restart_timeout;
+	}
+	return false;
+}
+
+static void accel_zts_restart_end(void)
+{
+	accel_zts_lock_reader_byte(F_UNLCK);
+	accel_zts_next_warning = 0;
+	tsrm_mutex_unlock(accel_zts_readers_mutex);
+}
+#endif
+
 static int zend_get_stream_timestamp(const char *filename, zend_stat_t *statbuf)
 {
 	php_stream_wrapper *wrapper;
@@ -2700,7 +2829,11 @@ ZEND_RINIT_FUNCTION(zend_accelerator)
 	if (ZCSG(restart_pending)) {
 		zend_shared_alloc_lock();
 		if (ZCSG(restart_pending)) { /* check again, to ensure that the cache wasn't already cleaned by another process */
+#if defined(ZTS) && !defined(ZEND_WIN32)
+			if (accel_zts_restart_begin()) {
+#else
 			if (accel_is_inactive()) {
+#endif
 				zend_accel_error(ACCEL_LOG_DEBUG, "Restarting!");
 				ZCSG(restart_pending) = false;
 				switch ZCSG(restart_reason) {
@@ -2740,13 +2873,29 @@ ZEND_RINIT_FUNCTION(zend_accelerator)
 					ZCSG(last_restart_time)++;
 				}
 				accel_restart_leave();
+#if defined(ZTS) && !defined(ZEND_WIN32)
+				accel_zts_restart_end();
+#endif
 			}
 		}
 		zend_shared_alloc_unlock();
 	}
 
 	ZCG(accelerator_enabled) = ZCSG(accelerator_enabled);
 
+#if defined(ZTS) && !defined(ZEND_WIN32)
+	/* Under ZTS this is the only place where a request starts to use SHM */
+	if (ZCG(accelerator_enabled)) {
+		if (accel_zts_reader_enter()) {
+			ZCG(counted) = true;
+			/* SHM-backed keys may be added to the PCRE cache again */
+			ZCG(pcre_reseted) = false;
+		} else {
+			ZCG(accelerator_enabled) = false;
+		}
+	}
+#endif
+
 	SHM_PROTECT();
 	HANDLE_UNBLOCK_INTERRUPTIONS();
 
@@ -2971,6 +3120,16 @@ static void accel_globals_ctor(zend_accel_globals *accel_globals)
 
 static void accel_globals_dtor(zend_accel_globals *accel_globals)
 {
+#if defined(ZTS) && !defined(ZEND_WIN32)
+	/* The thread is retired without its request cleanup (e.g. another extension
+	 * bailed out in post_deactivate): release its reference, otherwise restarts would
+	 * wait for it forever. Uses only the given globals: ts_free_id() calls this for
+	 * all threads from the current one. */
+	if (accel_globals->counted && accel_zts_readers_mutex) {
+		accel_globals->counted = false;
+		accel_zts_reader_leave();
+	}
+#endif
 	zend_string_free(accel_globals->key);
 	if (accel_globals->preloaded_internal_run_time_cache) {
 		pefree(accel_globals->preloaded_internal_run_time_cache, 1);
@@ -3296,6 +3455,13 @@ static zend_result accel_post_startup(void)
 
 		/* from this point further, shared memory is supposed to be OK */
 
+#if defined(ZTS) && !defined(ZEND_WIN32)
+		/* before preloading runs its request and before SAPIs fork their children */
+		accel_zts_readers_mutex = tsrm_mutex_alloc();
+		ZEND_ATOMIC_INT_INIT(&accel_zts_readers, 0);
+		accel_zts_next_warning = 0;
+#endif
+
 		/* remember the last restart time in the process memory */
 		ZCG(last_restart_time) = ZCSG(last_restart_time);
 
@@ -3400,6 +3566,12 @@ static void (*orig_post_shutdown_cb)(void);
 static void accel_post_shutdown(void)
 {
 	zend_shared_alloc_shutdown();
+#if defined(ZTS) && !defined(ZEND_WIN32)
+	if (accel_zts_readers_mutex) {
+		tsrm_mutex_free(accel_zts_readers_mutex);
+		accel_zts_readers_mutex = NULL;
+	}
+#endif
 }
 
 void accel_shutdown(void)