fix: fatal php shutdowns (#2293)

Fixes #2268 (and maybe others)

In that issue, a timeout during a `curl_multi` request leads to a fatal
error and bailout during `php_request_shutdown()`. After looking at the
[FPM](https://github.com/php/php-src/blob/9011bd31d7c26b2f255e550171548eb024d1e4ce/sapi/fpm/fpm/fpm_main.c#L1926)
implementation I realized it also wraps `php_request_shutdown()` with a
`zend_bailout`, which we don't. This PR wraps the shutdown function and
restarts ZTS in case of an unexpected bailout, which fixes #2268 and
prevents any potential crashes from bailouts during shutdown.

Still a draft since I think it might make sense to wrap the whole
request loop in a `zend_try`.

Author: AlliBalliBaba

frankenphp.c

@@ -1038,32 +1038,113 @@ static void set_thread_name(char *thread_name) {
 #endif
 }
 
+static inline void reset_sandboxed_environment() {
+  if (sandboxed_env != NULL) {
+    zend_hash_release(sandboxed_env);
+    sandboxed_env = NULL;
+  }
+}
+
 static void *php_thread(void *arg) {
   thread_index = (uintptr_t)arg;
   char thread_name[16] = {0};
   snprintf(thread_name, 16, "php-%" PRIxPTR, thread_index);
   set_thread_name(thread_name);
 
+  /* Initial allocation of all global PHP memory for this thread */
 #ifdef ZTS
-  /* initial resource fetch */
   (void)ts_resource(0);
 #ifdef PHP_WIN32
   ZEND_TSRMLS_CACHE_UPDATE();
 #endif
 #endif
 
-  // loop until Go signals to stop
-  char *scriptName = NULL;
-  while ((scriptName = go_frankenphp_before_script_execution(thread_index))) {
-    go_frankenphp_after_script_execution(thread_index,
-                                         frankenphp_execute_script(scriptName));
+  bool thread_is_healthy = true;
+  bool has_attempted_shutdown = false;
+
+  /* Main loop of the PHP thread, execute a PHP script and repeat until Go
+   * signals to stop */
+  zend_first_try {
+    char *scriptName = NULL;
+    while ((scriptName = go_frankenphp_before_script_execution(thread_index))) {
+      has_attempted_shutdown = false;
+
+      frankenphp_update_request_context();
+
+      if (UNEXPECTED(php_request_startup() == FAILURE)) {
+        /* Request startup failed, bail out to zend_catch */
+        frankenphp_log_message("Request startup failed, thread is unhealthy",
+                               LOG_ERR);
+        zend_bailout();
+      }
+
+      zend_file_handle file_handle;
+      zend_stream_init_filename(&file_handle, scriptName);
+
+      file_handle.primary_script = 1;
+      EG(exit_status) = 0;
+
+      /* Execute the PHP script, potential bailout to zend_catch */
+      php_execute_script(&file_handle);
+      zend_destroy_file_handle(&file_handle);
+      reset_sandboxed_environment();
+
+      /* Update the last memory usage for metrics */
+      __atomic_store_n(&thread_metrics[thread_index].last_memory_usage,
+                       zend_memory_usage(0), __ATOMIC_RELAXED);
+
+      has_attempted_shutdown = true;
+
+      /* shutdown the request, potential bailout to zend_catch */
+      php_request_shutdown((void *)0);
+      frankenphp_free_request_context();
+      go_frankenphp_after_script_execution(thread_index, EG(exit_status));
+    }
   }
+  zend_catch {
+    /* Critical failure from php_execute_script or php_request_shutdown, mark
+     * the thread as unhealthy */
+    thread_is_healthy = false;
+    if (!has_attempted_shutdown) {
+      /* php_request_shutdown() was not called, force a shutdown now */
+      reset_sandboxed_environment();
+      zend_try { php_request_shutdown((void *)0); }
+      zend_catch {}
+      zend_end_try();
+    }
 
+    /* Log the last error message, it must be cleared to prevent a crash when
+     * freeing execution globals */
+    if (PG(last_error_message)) {
+      go_log_attrs(thread_index, PG(last_error_message), 8, NULL);
+      PG(last_error_message) = NULL;
+      PG(last_error_file) = NULL;
+    }
+    frankenphp_free_request_context();
+    go_frankenphp_after_script_execution(thread_index, EG(exit_status));
+  }
+  zend_end_try();
+
+  /* free all global PHP memory reserved for this thread */
 #ifdef ZTS
   ts_free_thread();
 #endif
 
-  go_frankenphp_on_thread_shutdown(thread_index);
+  /* Thread is healthy, signal to Go that the thread has shut down */
+  if (thread_is_healthy) {
+    go_frankenphp_on_thread_shutdown(thread_index);
+
+    return NULL;
+  }
+
+  /* Thread is unhealthy, PHP globals might be in a bad state after a bailout,
+   * restart the entire thread */
+  frankenphp_log_message("Restarting unhealthy thread", LOG_WARNING);
+
+  if (!frankenphp_new_php_thread(thread_index)) {
+    /* probably unreachable */
+    frankenphp_log_message("Failed to restart an unhealthy thread", LOG_ERR);
+  }
 
   return NULL;
 }
@@ -1197,55 +1278,6 @@ bool frankenphp_new_php_thread(uintptr_t thread_index) {
   return true;
 }
 
-static int frankenphp_request_startup() {
-  frankenphp_update_request_context();
-  if (php_request_startup() == SUCCESS) {
-    return SUCCESS;
-  }
-
-  php_request_shutdown((void *)0);
-  frankenphp_free_request_context();
-
-  return FAILURE;
-}
-
-int frankenphp_execute_script(char *file_name) {
-  if (frankenphp_request_startup() == FAILURE) {
-
-    return FAILURE;
-  }
-
-  int status = SUCCESS;
-
-  zend_file_handle file_handle;
-  zend_stream_init_filename(&file_handle, file_name);
-
-  file_handle.primary_script = 1;
-
-  zend_first_try {
-    EG(exit_status) = 0;
-    php_execute_script(&file_handle);
-    status = EG(exit_status);
-  }
-  zend_catch { status = EG(exit_status); }
-  zend_end_try();
-
-  zend_destroy_file_handle(&file_handle);
-
-  /* Reset the sandboxed environment if it is in use */
-  if (sandboxed_env != NULL) {
-    zend_hash_release(sandboxed_env);
-    sandboxed_env = NULL;
-  }
-
-  __atomic_store_n(&thread_metrics[thread_index].last_memory_usage,
-                   zend_memory_usage(0), __ATOMIC_RELAXED);
-  php_request_shutdown((void *)0);
-  frankenphp_free_request_context();
-
-  return status;
-}
-
 /* Use global variables to store CLI arguments to prevent useless allocations */
 static char *cli_script;
 static int cli_argc;

frankenphp.h

@@ -169,7 +169,6 @@ int frankenphp_new_main_thread(int num_threads);
 bool frankenphp_new_php_thread(uintptr_t thread_index);
 
 bool frankenphp_shutdown_dummy_request(void);
-int frankenphp_execute_script(char *file_name);
 void frankenphp_update_local_thread_context(bool is_worker);
 
 int frankenphp_execute_script_cli(char *script, int argc, char **argv,