feat: improve sessions handling in workers

Dockerfile

@@ -30,6 +30,7 @@ RUN apt-get update && \
     gdb \
     valgrind \
     neovim && \
+    zsh && \
     echo 'set auto-load safe-path /' > /root/.gdbinit && \
     echo '* soft core unlimited' >> /etc/security/limits.conf \
     && \
@@ -40,7 +41,7 @@ RUN git clone https://github.com/dunglas/php-src.git && \
     git checkout frankenphp-8.2 && \
     # --enable-embed is only necessary to generate libphp.so, we don't use this SAPI directly
     ./buildconf && \
-    ./configure --enable-embed=static --enable-zts --disable-zend-signals --enable-static --enable-debug && \
+    ./configure --enable-embed=static --enable-zts --disable-zend-signals --enable-debug && \
     make -j6 && \
     make install && \
     #rm -Rf php-src/ && \

README.md

@@ -58,9 +58,7 @@ Then run the configure script:
     --enable-zts \
     --disable-zend-signals \
     --disable-opcache-jit \
-    --with-iconv=/opt/homebrew/opt/libiconv/ \
-    --enable-static \
-    --enable-shared=no
+    --with-iconv=/opt/homebrew/opt/libiconv/
 ```
 
 These flags are required, but you can add other flags (extra extensions...)

frankenphp.c

@@ -17,56 +17,6 @@
 ZEND_TSRMLS_CACHE_DEFINE()
 #endif
 
-// Helper functions copied from the PHP source code
-
-// main/php_variables.c
-
-static zend_always_inline void php_register_variable_quick(const char *name, size_t name_len, zval *val, HashTable *ht)
-{
-	zend_string *key = zend_string_init_interned(name, name_len, 0);
-
-	zend_hash_update_ind(ht, key, val);
-	zend_string_release_ex(key, 0);
-}
-
-static inline void php_register_server_variables(void)
-{
-	zval tmp;
-	zval *arr = &PG(http_globals)[TRACK_VARS_SERVER];
-	HashTable *ht;
-
-	zval_ptr_dtor_nogc(arr);
-	array_init(arr);
-
-	/* Server variables */
-	if (sapi_module.register_server_variables) {
-		sapi_module.register_server_variables(arr);
-	}
-	ht = Z_ARRVAL_P(arr);
-
-	/* PHP Authentication support */
-	if (SG(request_info).auth_user) {
-		ZVAL_STRING(&tmp, SG(request_info).auth_user);
-		php_register_variable_quick("PHP_AUTH_USER", sizeof("PHP_AUTH_USER")-1, &tmp, ht);
-	}
-	if (SG(request_info).auth_password) {
-		ZVAL_STRING(&tmp, SG(request_info).auth_password);
-		php_register_variable_quick("PHP_AUTH_PW", sizeof("PHP_AUTH_PW")-1, &tmp, ht);
-	}
-	if (SG(request_info).auth_digest) {
-		ZVAL_STRING(&tmp, SG(request_info).auth_digest);
-		php_register_variable_quick("PHP_AUTH_DIGEST", sizeof("PHP_AUTH_DIGEST")-1, &tmp, ht);
-	}
-
-	/* store request init time */
-	ZVAL_DOUBLE(&tmp, sapi_get_request_time());
-	php_register_variable_quick("REQUEST_TIME_FLOAT", sizeof("REQUEST_TIME_FLOAT")-1, &tmp, ht);
-	ZVAL_LONG(&tmp, zend_dval_to_lval(Z_DVAL(tmp)));
-	php_register_variable_quick("REQUEST_TIME", sizeof("REQUEST_TIME")-1, &tmp, ht);
-}
-
-// End of copied functions
-
 int frankenphp_check_version() {
 #ifndef ZTS
     return -1;
@@ -83,71 +33,27 @@ typedef struct frankenphp_server_context {
 	uintptr_t current_request;
 	uintptr_t main_request; // Only available during worker initialization
 	char *cookie_data;
-	HashTable *autoload_functions;
+	zend_module_entry *session_module;
 } frankenphp_server_context;
 
 // Adapted from php_request_shutdown
 void frankenphp_worker_request_shutdown(uintptr_t current_request) {
-	/* 0. skipped: Call any open observer end handlers that are still open after a zend_bailout */
-	/* 1. skipped: Call all possible shutdown functions registered with register_shutdown_function() */
-	/* 2. skipped: Call all possible __destruct() functions */
-
-	/* 3. Flush all output buffers */
+	/* Flush all output buffers */
 	zend_try {
 		php_output_end_all();
 	} zend_end_try();
 
-	/* 4. Reset max_execution_time (no longer executing php code after response sent) */
+	/* Reset max_execution_time (no longer executing php code after response sent) */
 	zend_try {
 		zend_unset_timeout();
 	} zend_end_try();
 
-	/* 5. Call all extensions RSHUTDOWN functions */
-	if (PG(modules_activated)) {
-		zend_deactivate_modules();
-	}
-
-	/* 6. Shutdown output layer (send the set HTTP headers, cleanup output handlers, etc.) */
+	/* Shutdown output layer (send the set HTTP headers, cleanup output handlers, etc.) */
 	zend_try {
 		php_output_deactivate();
 	} zend_end_try();
-	
-	/* 8. Destroy super-globals */
-	/*zend_try {
-		int i;
-
-		for (i=0; i<NUM_TRACK_VARS; i++) {
-			zval_ptr_dtor(&PG(http_globals)[i]);
-		}
-	} zend_end_try();*/
-
-	/* 9. skipped: Shutdown scanner/executor/compiler and restore ini entries */
-
-	/* 10. free request-bound globals */
-	// todo: check if it's a good idea
-	// php_free_request_globals()
-	// clear_last_error()
-	/*if (PG(last_error_message)) {
-		zend_string_release(PG(last_error_message));
-		PG(last_error_message) = NULL;
-	}
-	if (PG(last_error_file)) {
-		zend_string_release(PG(last_error_file));
-		PG(last_error_file) = NULL;
-	}
-	if (PG(php_sys_temp_dir)) {
-		efree(PG(php_sys_temp_dir));
-		PG(php_sys_temp_dir) = NULL;
-	}*/
-
-	/* 11. Call all extensions post-RSHUTDOWN functions */
-	zend_try {
-		zend_post_deactivate_modules();
-	} zend_end_try();
-
-	/* 13. skipped: free virtual CWD memory */
 
-	/* 12. SAPI related shutdown (free stuff) */
+	/* SAPI related shutdown (free stuff) */
 	frankenphp_clean_server_context();
 	zend_try {
 		sapi_deactivate();
@@ -155,14 +61,6 @@ void frankenphp_worker_request_shutdown(uintptr_t current_request) {
 
 	if (current_request != 0) go_frankenphp_worker_handle_request_end(current_request);
 
-	/* 14. Destroy stream hashes */
-	// todo: check if it's a good idea
-	/*zend_try {
-		php_shutdown_stream_hashes();
-	} zend_end_try();*/
-
-	/* 15. skipped: Free Willy (here be crashes) */
-
 	zend_set_memory_limit(PG(memory_limit));
 }
 
@@ -177,30 +75,18 @@ int frankenphp_worker_request_startup() {
 		php_output_activate();
 
 		/* initialize global variables */
-		PG(modules_activated) = 0;
 		PG(header_is_being_sent) = 0;
 		PG(connection_status) = PHP_CONNECTION_NORMAL;
-		PG(in_user_include) = 0;
 
 		// Keep the current execution context
-		//zend_activate();
 		sapi_activate();
 
-/*#ifdef ZEND_SIGNALS
-		zend_signal_activate();
-#endif*/
-
 		if (PG(max_input_time) == -1) {
 			zend_set_timeout(EG(timeout_seconds), 1);
 		} else {
 			zend_set_timeout(PG(max_input_time), 1);
 		}
 
-		/* Disable realpath cache if an open_basedir is set */
-		//if (PG(open_basedir) && *PG(open_basedir)) {
-		//	CWDG(realpath_cache_size_limit) = 0;
-		//}
-
 		if (PG(expose_php)) {
 			sapi_add_header(SAPI_PHP_VERSION_HEADER, sizeof(SAPI_PHP_VERSION_HEADER)-1, 1);
 		}
@@ -217,18 +103,10 @@ int frankenphp_worker_request_startup() {
 			php_output_set_implicit_flush(1);
 		}
 
-		PG(during_request_startup) = 0;
-
 		php_hash_environment();
 
-		// todo: find what we need to call php_register_server_variables();
-		php_register_server_variables();
-		zend_hash_update(&EG(symbol_table), ZSTR_KNOWN(ZEND_STR_AUTOGLOBAL_SERVER), &PG(http_globals)[TRACK_VARS_SERVER]);
-		Z_ADDREF(PG(http_globals)[TRACK_VARS_SERVER]);
-		HT_ALLOW_COW_VIOLATION(Z_ARRVAL(PG(http_globals)[TRACK_VARS_SERVER]));
-
-		zend_activate_modules();
-		PG(modules_activated)=1;
+		// Re-populate $_SERVER
+		zend_is_auto_global(ZSTR_KNOWN(ZEND_STR_AUTOGLOBAL_SERVER));
 	} zend_catch {
 		retval = FAILURE;
 	} zend_end_try();
@@ -238,46 +116,6 @@ int frankenphp_worker_request_startup() {
 	return retval;
 }
 
-void save_autload_functions() {
-	zval retval = {0};
-	zend_fcall_info fci = {0};
-	zend_fcall_info_cache fci_cache = {0};
-
-	zend_string *func_name = zend_string_init(ZEND_STRL("spl_autoload_functions"), 0);
-	ZVAL_STR(&fci.function_name, func_name);
-
-	fci.size = sizeof fci;
-	fci.retval = &retval;
-
-	zend_call_function(&fci, &fci_cache);
-
-	zend_string_release(func_name);
-
-	((frankenphp_server_context *) SG(server_context))->autoload_functions = Z_ARRVAL_P(fci.retval);
-}
-
-void restore_autoload_functions() {
-	HashTable *functions = ((frankenphp_server_context *) SG(server_context))->autoload_functions;
-
-	zval *val;
-	zval retval = {0};
-	zend_fcall_info fci = {0};
-	zend_fcall_info_cache fci_cache = {0};
-
-	zend_string *func_name = zend_string_init(ZEND_STRL("spl_autoload_register"), 0);
-	ZVAL_STR(&fci.function_name, func_name);
-
-	fci.size = sizeof fci;
-	fci.retval = &retval;
-	fci.param_count = 1;
-
-	ZEND_HASH_FOREACH_VAL(functions, val) {
-		fci.params = val;
-
-		zend_call_function(&fci, &fci_cache);
-	} ZEND_HASH_FOREACH_END();
-}
-
 ZEND_BEGIN_ARG_INFO_EX(arginfo_frankenphp_handle_request, 0, 0, 1)
     ZEND_ARG_CALLABLE_INFO(false, handler, false)
 ZEND_END_ARG_INFO()
@@ -294,8 +132,8 @@ PHP_FUNCTION(frankenphp_handle_request) {
 
 	uintptr_t previous_request = ctx->current_request;
 	if (ctx->main_request) {
-		// Save the main autoloader
-		save_autload_functions();
+		// Store a pointer to the session module
+		ctx->session_module = zend_hash_str_find_ptr(&module_registry, "session", sizeof("session") -1);
 
 		// Clean the first dummy request created to initialize the worker
 		frankenphp_worker_request_shutdown(0);
@@ -318,7 +156,9 @@ PHP_FUNCTION(frankenphp_handle_request) {
 		RETURN_FALSE;
 	}
 
-	restore_autoload_functions();
+	// Call session module's rinit
+	if (ctx->session_module)
+		ctx->session_module->request_startup_func(ctx->session_module->type, ctx->session_module->module_number);
 
 	// Call the PHP func
 	zval retval = {0};
@@ -326,6 +166,10 @@ PHP_FUNCTION(frankenphp_handle_request) {
 	fci.retval = &retval;
 	zend_call_function(&fci, &fcc);
 
+	// Call session module's rshutdown
+	if (ctx->session_module)
+		ctx->session_module->request_shutdown_func(ctx->session_module->type, ctx->session_module->module_number);
+
 	frankenphp_worker_request_shutdown(next_request);
 
 	RETURN_TRUE;
@@ -385,7 +229,6 @@ uintptr_t frankenphp_request_shutdown()
 
 	free(ctx->cookie_data);
 	((frankenphp_server_context*) SG(server_context))->cookie_data = NULL;
-
 	uintptr_t rh = frankenphp_clean_server_context();
 
 	free(ctx);
@@ -523,15 +366,6 @@ static void frankenphp_register_variables(zval *track_vars_array)
 	// https://www.php.net/manual/en/reserved.variables.server.php
 	frankenphp_server_context* ctx = SG(server_context);
 
-	// todo: remove this
-	/*if (ctx->current_request == 0 && ctx->worker_filename != NULL) {
-		// todo: also register PHP_SELF etc
-		php_register_variable_safe("SCRIPT_FILENAME", ctx->worker_filename, strlen(ctx->worker_filename), track_vars_array);
-	}*/
-
-	// todo: import or not environment variables set in the parent process?
-	//php_import_environment_variables(track_vars_array);
-
 	go_register_variables(ctx->current_request ? ctx->current_request : ctx->main_request, track_vars_array);
 }
 

frankenphp.go

@@ -332,7 +332,7 @@ func go_ub_write(rh C.uintptr_t, cString *C.char, length C.int) C.size_t {
 	var writer io.Writer
 	if fc.responseWriter == nil {
 		var b bytes.Buffer
-		// log the output of the
+		// log the output of the worker
 		writer = &b
 	} else {
 		writer = fc.responseWriter

worker.go

@@ -7,6 +7,7 @@ import (
 	"context"
 	"fmt"
 	"net/http"
+	"path/filepath"
 	"runtime/cgo"
 	"sync"
 
@@ -20,11 +21,16 @@ var (
 )
 
 func startWorkers(fileName string, nbWorkers int) error {
-	if _, ok := workersRequestChans.Load(fileName); ok {
-		panic(fmt.Errorf("workers %q: already started", fileName))
+	absFileName, err := filepath.Abs(fileName)
+	if err != nil {
+		return fmt.Errorf("workers %q: %w", fileName, err)
 	}
 
-	workersRequestChans.Store(fileName, make(chan *http.Request))
+	if _, ok := workersRequestChans.Load(absFileName); ok {
+		return fmt.Errorf("workers %q: already started", absFileName)
+	}
+
+	workersRequestChans.Store(absFileName, make(chan *http.Request))
 	shutdownWG.Add(nbWorkers)
 	workersReadyWG.Add(nbWorkers)
 
@@ -43,7 +49,7 @@ func startWorkers(fileName string, nbWorkers int) error {
 			if err != nil {
 				m.Lock()
 				defer m.Unlock()
-				errors = append(errors, fmt.Errorf("workers %q: unable to create main worker request: %w", fileName, err))
+				errors = append(errors, fmt.Errorf("workers %q: unable to create main worker request: %w", absFileName, err))
 
 				return
 			}
@@ -52,20 +58,20 @@ func startWorkers(fileName string, nbWorkers int) error {
 				r.Context(),
 				contextKey,
 				&FrankenPHPContext{
-					Env: map[string]string{"SCRIPT_FILENAME": fileName},
+					Env: map[string]string{"SCRIPT_FILENAME": absFileName},
 				},
 			)
 
-			l.Debug("starting", zap.String("worker", fileName))
+			l.Debug("starting", zap.String("worker", absFileName))
 			if err := ServeHTTP(nil, r.WithContext(ctx)); err != nil {
 				m.Lock()
 				defer m.Unlock()
-				errors = append(errors, fmt.Errorf("workers %q: unable to start: %w", fileName, err))
+				errors = append(errors, fmt.Errorf("workers %q: unable to start: %w", absFileName, err))
 
 				return
 			}
 			// TODO: check if the termination is expected
-			l.Debug("terminated", zap.String("worker", fileName))
+			l.Debug("terminated", zap.String("worker", absFileName))
 		}()
 	}