Browse Source

refactor: removes context on the C side (#1404)

Alexander Stecher 2 days ago
parent
commit
f50248a7d2
10 changed files with 326 additions and 317 deletions
  1. 9 10
      cgi.go
  2. 164 0
      context.go
  3. 42 110
      frankenphp.c
  4. 58 171
      frankenphp.go
  5. 8 5
      frankenphp.h
  6. 21 0
      phpmainthread_test.go
  7. 13 6
      phpthread.go
  8. 7 7
      request_options.go
  9. 3 3
      scaling.go
  10. 1 5
      threadinactive.go

+ 9 - 10
cgi.go

@@ -14,7 +14,6 @@ import "C"
 import (
 	"crypto/tls"
 	"net"
-	"net/http"
 	"path/filepath"
 	"strings"
 	"unsafe"
@@ -56,7 +55,8 @@ var knownServerKeys = []string{
 //
 // TODO: handle this case https://github.com/caddyserver/caddy/issues/3718
 // Inspired by https://github.com/caddyserver/caddy/blob/master/modules/caddyhttp/reverseproxy/fastcgi/fastcgi.go
-func addKnownVariablesToServer(thread *phpThread, request *http.Request, fc *FrankenPHPContext, trackVarsArray *C.zval) {
+func addKnownVariablesToServer(thread *phpThread, fc *frankenPHPContext, trackVarsArray *C.zval) {
+	request := fc.request
 	keys := mainThread.knownServerKeys
 	// Separate remote IP and port; more lenient than net.SplitHostPort
 	var ip, port string
@@ -168,8 +168,8 @@ func packCgiVariable(key *C.zend_string, value string) C.ht_key_value_pair {
 	return C.ht_key_value_pair{key, toUnsafeChar(value), C.size_t(len(value))}
 }
 
-func addHeadersToServer(request *http.Request, thread *phpThread, fc *FrankenPHPContext, trackVarsArray *C.zval) {
-	for field, val := range request.Header {
+func addHeadersToServer(thread *phpThread, fc *frankenPHPContext, trackVarsArray *C.zval) {
+	for field, val := range fc.request.Header {
 		if k := mainThread.commonHeaders[field]; k != nil {
 			v := strings.Join(val, ", ")
 			C.frankenphp_register_single(k, toUnsafeChar(v), C.size_t(len(v)), trackVarsArray)
@@ -184,7 +184,7 @@ func addHeadersToServer(request *http.Request, thread *phpThread, fc *FrankenPHP
 	}
 }
 
-func addPreparedEnvToServer(fc *FrankenPHPContext, trackVarsArray *C.zval) {
+func addPreparedEnvToServer(fc *frankenPHPContext, trackVarsArray *C.zval) {
 	for k, v := range fc.env {
 		C.frankenphp_register_variable_safe(toUnsafeChar(k), toUnsafeChar(v), C.size_t(len(v)), trackVarsArray)
 	}
@@ -194,11 +194,10 @@ func addPreparedEnvToServer(fc *FrankenPHPContext, trackVarsArray *C.zval) {
 //export go_register_variables
 func go_register_variables(threadIndex C.uintptr_t, trackVarsArray *C.zval) {
 	thread := phpThreads[threadIndex]
-	r := thread.getActiveRequest()
-	fc := r.Context().Value(contextKey).(*FrankenPHPContext)
+	fc := thread.getRequestContext()
 
-	addKnownVariablesToServer(thread, r, fc, trackVarsArray)
-	addHeadersToServer(r, thread, fc, trackVarsArray)
+	addKnownVariablesToServer(thread, fc, trackVarsArray)
+	addHeadersToServer(thread, fc, trackVarsArray)
 
 	// The Prepared Environment is registered last and can overwrite any previous values
 	addPreparedEnvToServer(fc, trackVarsArray)
@@ -209,7 +208,7 @@ func go_register_variables(threadIndex C.uintptr_t, trackVarsArray *C.zval) {
 //
 // Adapted from https://github.com/caddyserver/caddy/blob/master/modules/caddyhttp/reverseproxy/fastcgi/fastcgi.go
 // Copyright 2015 Matthew Holt and The Caddy Authors
-func splitPos(fc *FrankenPHPContext, path string) int {
+func splitPos(fc *frankenPHPContext, path string) int {
 	if len(fc.splitPath) == 0 {
 		return 0
 	}

+ 164 - 0
context.go

@@ -0,0 +1,164 @@
+package frankenphp
+
+import (
+	"context"
+	"net/http"
+	"os"
+	"strings"
+	"time"
+
+	"go.uber.org/zap"
+)
+
+// frankenPHPContext provides contextual information about the Request to handle.
+type frankenPHPContext struct {
+	documentRoot    string
+	splitPath       []string
+	env             PreparedEnv
+	logger          *zap.Logger
+	request         *http.Request
+	originalRequest *http.Request
+
+	docURI         string
+	pathInfo       string
+	scriptName     string
+	scriptFilename string
+
+	// Whether the request is already closed by us
+	isDone bool
+
+	responseWriter http.ResponseWriter
+
+	done      chan interface{}
+	startedAt time.Time
+}
+
+// fromContext extracts the frankenPHPContext from a context.
+func fromContext(ctx context.Context) (fctx *frankenPHPContext, ok bool) {
+	fctx, ok = ctx.Value(contextKey).(*frankenPHPContext)
+	return
+}
+
+// NewRequestWithContext creates a new FrankenPHP request context.
+func NewRequestWithContext(r *http.Request, opts ...RequestOption) (*http.Request, error) {
+	fc := &frankenPHPContext{
+		done:      make(chan interface{}),
+		startedAt: time.Now(),
+		request:   r,
+	}
+	for _, o := range opts {
+		if err := o(fc); err != nil {
+			return nil, err
+		}
+	}
+
+	if fc.logger == nil {
+		fc.logger = logger
+	}
+
+	if fc.documentRoot == "" {
+		if EmbeddedAppPath != "" {
+			fc.documentRoot = EmbeddedAppPath
+		} else {
+			var err error
+			if fc.documentRoot, err = os.Getwd(); err != nil {
+				return nil, err
+			}
+		}
+	}
+
+	if fc.splitPath == nil {
+		fc.splitPath = []string{".php"}
+	}
+
+	if fc.env == nil {
+		fc.env = make(map[string]string)
+	}
+
+	if splitPos := splitPos(fc, r.URL.Path); splitPos > -1 {
+		fc.docURI = r.URL.Path[:splitPos]
+		fc.pathInfo = r.URL.Path[splitPos:]
+
+		// Strip PATH_INFO from SCRIPT_NAME
+		fc.scriptName = strings.TrimSuffix(r.URL.Path, fc.pathInfo)
+
+		// Ensure the SCRIPT_NAME has a leading slash for compliance with RFC3875
+		// Info: https://tools.ietf.org/html/rfc3875#section-4.1.13
+		if fc.scriptName != "" && !strings.HasPrefix(fc.scriptName, "/") {
+			fc.scriptName = "/" + fc.scriptName
+		}
+	}
+
+	// SCRIPT_FILENAME is the absolute path of SCRIPT_NAME
+	fc.scriptFilename = sanitizedPathJoin(fc.documentRoot, fc.scriptName)
+
+	c := context.WithValue(r.Context(), contextKey, fc)
+
+	return r.WithContext(c), nil
+}
+
+func newDummyContext(requestPath string, opts ...RequestOption) (*frankenPHPContext, error) {
+	r, err := http.NewRequest(http.MethodGet, requestPath, nil)
+	if err != nil {
+		return nil, err
+	}
+
+	fr, err := NewRequestWithContext(r, opts...)
+	if err != nil {
+		return nil, err
+	}
+
+	fc, _ := fromContext(fr.Context())
+
+	return fc, nil
+}
+
+// closeContext sends the response to the client
+func (fc *frankenPHPContext) closeContext() {
+	if fc.isDone {
+		return
+	}
+
+	close(fc.done)
+	fc.isDone = true
+}
+
+// validate checks if the request should be outright rejected
+func (fc *frankenPHPContext) validate() bool {
+	if !strings.Contains(fc.request.URL.Path, "\x00") {
+		return true
+	}
+
+	fc.rejectBadRequest("Invalid request path")
+
+	return false
+}
+
+func (fc *frankenPHPContext) clientHasClosed() bool {
+	select {
+	case <-fc.request.Context().Done():
+		return true
+	default:
+		return false
+	}
+}
+
+// reject sends a response with the given status code and message
+func (fc *frankenPHPContext) reject(statusCode int, message string) {
+	if fc.isDone {
+		return
+	}
+
+	rw := fc.responseWriter
+	if rw != nil {
+		rw.WriteHeader(statusCode)
+		_, _ = rw.Write([]byte(message))
+		rw.(http.Flusher).Flush()
+	}
+
+	fc.closeContext()
+}
+
+func (fc *frankenPHPContext) rejectBadRequest(message string) {
+	fc.reject(http.StatusBadRequest, message)
+}

+ 42 - 110
frankenphp.c

@@ -70,26 +70,18 @@ frankenphp_config frankenphp_get_config() {
   };
 }
 
-typedef struct frankenphp_server_context {
-  bool has_main_request;
-  bool has_active_request;
-  bool worker_ready;
-  char *cookie_data;
-  bool finished;
-} frankenphp_server_context;
-
 bool should_filter_var = 0;
-__thread frankenphp_server_context *local_ctx = NULL;
 __thread uintptr_t thread_index;
+__thread bool is_worker_thread = false;
 __thread zval *os_environment = NULL;
 
 static void frankenphp_free_request_context() {
-  frankenphp_server_context *ctx = SG(server_context);
-
-  free(ctx->cookie_data);
-  ctx->cookie_data = NULL;
+  if (SG(request_info).cookie_data != NULL) {
+    free(SG(request_info).cookie_data);
+    SG(request_info).cookie_data = NULL;
+  }
 
-  /* Is freed via thread.Unpin() */
+  /* freed via thread.Unpin() */
   SG(request_info).auth_password = NULL;
   SG(request_info).auth_user = NULL;
   SG(request_info).request_method = NULL;
@@ -159,6 +151,17 @@ static void frankenphp_worker_request_shutdown() {
   zend_set_memory_limit(PG(memory_limit));
 }
 
+// shutdown the dummy request that starts the worker script
+bool frankenphp_shutdown_dummy_request(void) {
+  if (SG(server_context) == NULL) {
+    return false;
+  }
+
+  frankenphp_worker_request_shutdown();
+
+  return true;
+}
+
 PHPAPI void get_full_env(zval *track_vars_array) {
   go_getfullenv(thread_index, track_vars_array);
 }
@@ -227,10 +230,6 @@ static int frankenphp_worker_request_startup() {
       }
     }
 
-    /* Unfinish the request */
-    frankenphp_server_context *ctx = SG(server_context);
-    ctx->finished = false;
-
     /* TODO: store the list of modules to reload in a global module variable */
     const char **module_name;
     zend_module_entry *module;
@@ -255,20 +254,14 @@ PHP_FUNCTION(frankenphp_finish_request) { /* {{{ */
     RETURN_THROWS();
   }
 
-  frankenphp_server_context *ctx = SG(server_context);
-
-  if (ctx->finished) {
+  if (go_is_context_done(thread_index)) {
     RETURN_FALSE;
   }
 
   php_output_end_all();
   php_header();
 
-  if (ctx->has_active_request) {
-    go_frankenphp_finish_php_request(thread_index);
-  }
-
-  ctx->finished = true;
+  go_frankenphp_finish_php_request(thread_index);
 
   RETURN_TRUE;
 } /* }}} */
@@ -331,9 +324,8 @@ PHP_FUNCTION(frankenphp_request_headers) {
     RETURN_THROWS();
   }
 
-  frankenphp_server_context *ctx = SG(server_context);
   struct go_apache_request_headers_return headers =
-      go_apache_request_headers(thread_index, ctx->has_active_request);
+      go_apache_request_headers(thread_index);
 
   array_init_size(return_value, headers.r1);
 
@@ -407,9 +399,7 @@ PHP_FUNCTION(frankenphp_handle_request) {
   Z_PARAM_FUNC(fci, fcc)
   ZEND_PARSE_PARAMETERS_END();
 
-  frankenphp_server_context *ctx = SG(server_context);
-
-  if (!ctx->has_main_request) {
+  if (!is_worker_thread) {
     /* not a worker, throw an error */
     zend_throw_exception(
         spl_ce_RuntimeException,
@@ -417,22 +407,15 @@ PHP_FUNCTION(frankenphp_handle_request) {
     RETURN_THROWS();
   }
 
-  if (!ctx->worker_ready) {
-    /* Clean the first dummy request created to initialize the worker */
-    frankenphp_worker_request_shutdown();
-
-    ctx->worker_ready = true;
-  }
-
 #ifdef ZEND_MAX_EXECUTION_TIMERS
   /* Disable timeouts while waiting for a request to handle */
   zend_unset_timeout();
 #endif
 
-  bool request = go_frankenphp_worker_handle_request_start(thread_index);
+  bool has_request = go_frankenphp_worker_handle_request_start(thread_index);
   if (frankenphp_worker_request_startup() == FAILURE
       /* Shutting down */
-      || !request) {
+      || !has_request) {
     RETURN_FALSE;
   }
 
@@ -445,7 +428,7 @@ PHP_FUNCTION(frankenphp_handle_request) {
   }
 #endif
 
-  /* Call the PHP func */
+  /* Call the PHP func passed to frankenphp_handle_request() */
   zval retval = {0};
   fci.size = sizeof fci;
   fci.retval = &retval;
@@ -462,7 +445,6 @@ PHP_FUNCTION(frankenphp_handle_request) {
   }
 
   frankenphp_worker_request_shutdown();
-  ctx->has_active_request = false;
   go_frankenphp_finish_worker_request(thread_index);
 
   RETURN_TRUE;
@@ -526,44 +508,25 @@ static zend_module_entry frankenphp_module = {
     STANDARD_MODULE_PROPERTIES};
 
 static void frankenphp_request_shutdown() {
-  frankenphp_server_context *ctx = SG(server_context);
-
-  if (ctx->has_main_request && ctx->has_active_request) {
-    frankenphp_destroy_super_globals();
-  }
-
   php_request_shutdown((void *)0);
   frankenphp_free_request_context();
-
-  memset(local_ctx, 0, sizeof(frankenphp_server_context));
 }
 
-int frankenphp_update_server_context(
-    bool create, bool has_main_request, bool has_active_request,
-
-    const char *request_method, char *query_string, zend_long content_length,
-    char *path_translated, char *request_uri, const char *content_type,
-    char *auth_user, char *auth_password, int proto_num) {
-  frankenphp_server_context *ctx;
-
-  if (create) {
-    ctx = local_ctx;
+int frankenphp_update_server_context(bool is_worker_request,
 
-    ctx->worker_ready = false;
-    ctx->cookie_data = NULL;
-    ctx->finished = false;
+                                     const char *request_method,
+                                     char *query_string,
+                                     zend_long content_length,
+                                     char *path_translated, char *request_uri,
+                                     const char *content_type, char *auth_user,
+                                     char *auth_password, int proto_num) {
 
-    SG(server_context) = ctx;
-  } else {
-    ctx = (frankenphp_server_context *)SG(server_context);
-  }
+  SG(server_context) = (void *)1;
+  is_worker_thread = is_worker_request;
 
   // It is not reset by zend engine, set it to 200.
   SG(sapi_headers).http_response_code = 200;
 
-  ctx->has_main_request = has_main_request;
-  ctx->has_active_request = has_active_request;
-
   SG(request_info).auth_password = auth_password;
   SG(request_info).auth_user = auth_user;
   SG(request_info).request_method = request_method;
@@ -589,14 +552,6 @@ static int frankenphp_deactivate(void) {
 }
 
 static size_t frankenphp_ub_write(const char *str, size_t str_length) {
-  frankenphp_server_context *ctx = SG(server_context);
-
-  if (ctx->finished) {
-    /* TODO: maybe log a warning that we tried to write to a finished request?
-     */
-    return 0;
-  }
-
   struct go_ub_write_return result =
       go_ub_write(thread_index, (char *)str, str_length);
 
@@ -613,11 +568,6 @@ static int frankenphp_send_headers(sapi_headers_struct *sapi_headers) {
   }
 
   int status;
-  frankenphp_server_context *ctx = SG(server_context);
-
-  if (!ctx->has_active_request) {
-    return SAPI_HEADER_SEND_FAILED;
-  }
 
   if (SG(sapi_headers).http_status_line) {
     status = atoi((SG(sapi_headers).http_status_line) + 9);
@@ -629,37 +579,26 @@ static int frankenphp_send_headers(sapi_headers_struct *sapi_headers) {
     }
   }
 
-  go_write_headers(thread_index, status, &sapi_headers->headers);
+  bool success = go_write_headers(thread_index, status, &sapi_headers->headers);
+  if (success) {
+    return SAPI_HEADER_SENT_SUCCESSFULLY;
+  }
 
-  return SAPI_HEADER_SENT_SUCCESSFULLY;
+  return SAPI_HEADER_SEND_FAILED;
 }
 
 static void frankenphp_sapi_flush(void *server_context) {
-  frankenphp_server_context *ctx = (frankenphp_server_context *)server_context;
-
-  if (ctx && ctx->has_active_request && go_sapi_flush(thread_index)) {
+  if (go_sapi_flush(thread_index)) {
     php_handle_aborted_connection();
   }
 }
 
 static size_t frankenphp_read_post(char *buffer, size_t count_bytes) {
-  frankenphp_server_context *ctx = SG(server_context);
-
-  return ctx->has_active_request
-             ? go_read_post(thread_index, buffer, count_bytes)
-             : 0;
+  return go_read_post(thread_index, buffer, count_bytes);
 }
 
 static char *frankenphp_read_cookies(void) {
-  frankenphp_server_context *ctx = SG(server_context);
-
-  if (!ctx->has_active_request) {
-    return "";
-  }
-
-  ctx->cookie_data = go_read_cookies(thread_index);
-
-  return ctx->cookie_data;
+  return go_read_cookies(thread_index);
 }
 
 /* all variables with well defined keys can safely be registered like this */
@@ -817,10 +756,8 @@ static void frankenphp_register_variables(zval *track_vars_array) {
    * variables.
    */
 
-  frankenphp_server_context *ctx = SG(server_context);
-
   /* in non-worker mode we import the os environment regularly */
-  if (!ctx->has_main_request) {
+  if (!is_worker_thread) {
     get_full_env(track_vars_array);
     // php_import_environment_variables(track_vars_array);
     go_register_variables(thread_index, track_vars_array);
@@ -917,8 +854,6 @@ static void *php_thread(void *arg) {
 #endif
 #endif
 
-  local_ctx = malloc(sizeof(frankenphp_server_context));
-
   // loop until Go signals to stop
   char *scriptName = NULL;
   while ((scriptName = go_frankenphp_before_script_execution(thread_index))) {
@@ -932,9 +867,6 @@ static void *php_thread(void *arg) {
 
   go_frankenphp_on_thread_shutdown(thread_index);
 
-  free(local_ctx);
-  local_ctx = NULL;
-
   return NULL;
 }
 

+ 58 - 171
frankenphp.go

@@ -30,7 +30,6 @@ package frankenphp
 import "C"
 import (
 	"bytes"
-	"context"
 	"errors"
 	"fmt"
 	"io"
@@ -42,7 +41,6 @@ import (
 	"strings"
 	"sync"
 	"syscall"
-	"time"
 	"unsafe"
 
 	"go.uber.org/zap"
@@ -107,101 +105,6 @@ func (l syslogLevel) String() string {
 	}
 }
 
-// FrankenPHPContext provides contextual information about the Request to handle.
-type FrankenPHPContext struct {
-	documentRoot    string
-	splitPath       []string
-	env             PreparedEnv
-	logger          *zap.Logger
-	originalRequest *http.Request
-
-	docURI         string
-	pathInfo       string
-	scriptName     string
-	scriptFilename string
-
-	// Whether the request is already closed by us
-	closed sync.Once
-
-	responseWriter http.ResponseWriter
-	exitStatus     int
-
-	done      chan interface{}
-	startedAt time.Time
-}
-
-func clientHasClosed(r *http.Request) bool {
-	select {
-	case <-r.Context().Done():
-		return true
-	default:
-		return false
-	}
-}
-
-// NewRequestWithContext creates a new FrankenPHP request context.
-func NewRequestWithContext(r *http.Request, opts ...RequestOption) (*http.Request, error) {
-	fc := &FrankenPHPContext{
-		done:      make(chan interface{}),
-		startedAt: time.Now(),
-	}
-	for _, o := range opts {
-		if err := o(fc); err != nil {
-			return nil, err
-		}
-	}
-
-	if fc.documentRoot == "" {
-		if EmbeddedAppPath != "" {
-			fc.documentRoot = EmbeddedAppPath
-		} else {
-			var err error
-			if fc.documentRoot, err = os.Getwd(); err != nil {
-				return nil, err
-			}
-		}
-	}
-
-	if fc.splitPath == nil {
-		fc.splitPath = []string{".php"}
-	}
-
-	if fc.env == nil {
-		fc.env = make(map[string]string)
-	}
-
-	if fc.logger == nil {
-		fc.logger = getLogger()
-	}
-
-	if splitPos := splitPos(fc, r.URL.Path); splitPos > -1 {
-		fc.docURI = r.URL.Path[:splitPos]
-		fc.pathInfo = r.URL.Path[splitPos:]
-
-		// Strip PATH_INFO from SCRIPT_NAME
-		fc.scriptName = strings.TrimSuffix(r.URL.Path, fc.pathInfo)
-
-		// Ensure the SCRIPT_NAME has a leading slash for compliance with RFC3875
-		// Info: https://tools.ietf.org/html/rfc3875#section-4.1.13
-		if fc.scriptName != "" && !strings.HasPrefix(fc.scriptName, "/") {
-			fc.scriptName = "/" + fc.scriptName
-		}
-	}
-
-	// SCRIPT_FILENAME is the absolute path of SCRIPT_NAME
-	fc.scriptFilename = sanitizedPathJoin(fc.documentRoot, fc.scriptName)
-
-	c := context.WithValue(r.Context(), contextKey, fc)
-
-	return r.WithContext(c), nil
-}
-
-// FromContext extracts the FrankenPHPContext from a context.
-func FromContext(ctx context.Context) (fctx *FrankenPHPContext, ok bool) {
-	fctx, ok = ctx.Value(contextKey).(*FrankenPHPContext)
-	return
-}
-
 type PHPVersion struct {
 	MajorVersion   int
 	MinorVersion   int
@@ -343,11 +246,10 @@ func Init(options ...Option) error {
 		return err
 	}
 
-	regularRequestChan = make(chan *http.Request, totalThreadCount-workerThreadCount)
+	regularRequestChan = make(chan *frankenPHPContext, totalThreadCount-workerThreadCount)
 	regularThreads = make([]*phpThread, 0, totalThreadCount-workerThreadCount)
 	for i := 0; i < totalThreadCount-workerThreadCount; i++ {
-		thread := getInactivePHPThread()
-		convertToRegularThread(thread)
+		convertToRegularThread(getInactivePHPThread())
 	}
 
 	if err := initWorkers(opt.workers); err != nil {
@@ -389,19 +291,8 @@ func Shutdown() {
 	logger.Debug("FrankenPHP shut down")
 }
 
-func getLogger() *zap.Logger {
-	loggerMu.RLock()
-	defer loggerMu.RUnlock()
-
-	return logger
-}
-
-func updateServerContext(thread *phpThread, request *http.Request, create bool, isWorkerRequest bool) error {
-	fc, ok := FromContext(request.Context())
-	if !ok {
-		return InvalidRequestError
-	}
-
+func updateServerContext(thread *phpThread, fc *frankenPHPContext, isWorkerRequest bool) error {
+	request := fc.request
 	authUser, authPassword, ok := request.BasicAuth()
 	var cAuthUser, cAuthPassword *C.char
 	if ok && authPassword != "" {
@@ -438,12 +329,9 @@ func updateServerContext(thread *phpThread, request *http.Request, create bool,
 	}
 
 	cRequestUri := thread.pinCString(request.URL.RequestURI())
-	isBootingAWorkerScript := fc.responseWriter == nil
 
 	ret := C.frankenphp_update_server_context(
-		C.bool(create),
-		C.bool(isWorkerRequest || isBootingAWorkerScript),
-		C.bool(!isBootingAWorkerScript),
+		C.bool(isWorkerRequest || fc.responseWriter == nil),
 
 		cMethod,
 		cQueryString,
@@ -469,39 +357,36 @@ func ServeHTTP(responseWriter http.ResponseWriter, request *http.Request) error
 		return NotRunningError
 	}
 
-	if !requestIsValid(request, responseWriter) {
-		return nil
-	}
-
-	fc, ok := FromContext(request.Context())
+	fc, ok := fromContext(request.Context())
 	if !ok {
 		return InvalidRequestError
 	}
 
 	fc.responseWriter = responseWriter
 
+	if !fc.validate() {
+		return nil
+	}
+
 	// Detect if a worker is available to handle this request
 	if worker, ok := workers[fc.scriptFilename]; ok {
-		worker.handleRequest(request, fc)
+		worker.handleRequest(fc)
 		return nil
 	}
 
 	// If no worker was availabe send the request to non-worker threads
-	handleRequestWithRegularPHPThreads(request, fc)
+	handleRequestWithRegularPHPThreads(fc)
 
 	return nil
 }
 
-func maybeCloseContext(fc *FrankenPHPContext) {
-	fc.closed.Do(func() {
-		close(fc.done)
-	})
-}
-
 //export go_ub_write
 func go_ub_write(threadIndex C.uintptr_t, cBuf *C.char, length C.int) (C.size_t, C.bool) {
-	r := phpThreads[threadIndex].getActiveRequest()
-	fc, _ := FromContext(r.Context())
+	fc := phpThreads[threadIndex].getRequestContext()
+
+	if fc.isDone {
+		return 0, C.bool(true)
+	}
 
 	var writer io.Writer
 	if fc.responseWriter == nil {
@@ -523,28 +408,27 @@ func go_ub_write(threadIndex C.uintptr_t, cBuf *C.char, length C.int) (C.size_t,
 		fc.logger.Info(writer.(*bytes.Buffer).String())
 	}
 
-	return C.size_t(i), C.bool(clientHasClosed(r))
+	return C.size_t(i), C.bool(fc.clientHasClosed())
 }
 
 //export go_apache_request_headers
-func go_apache_request_headers(threadIndex C.uintptr_t, hasActiveRequest bool) (*C.go_string, C.size_t) {
+func go_apache_request_headers(threadIndex C.uintptr_t) (*C.go_string, C.size_t) {
 	thread := phpThreads[threadIndex]
+	fc := thread.getRequestContext()
 
-	if !hasActiveRequest {
+	if fc.responseWriter == nil {
 		// worker mode, not handling a request
-		mfc := thread.getActiveRequest().Context().Value(contextKey).(*FrankenPHPContext)
 
-		if c := mfc.logger.Check(zapcore.DebugLevel, "apache_request_headers() called in non-HTTP context"); c != nil {
-			c.Write(zap.String("worker", mfc.scriptFilename))
+		if c := logger.Check(zapcore.DebugLevel, "apache_request_headers() called in non-HTTP context"); c != nil {
+			c.Write(zap.String("worker", fc.scriptFilename))
 		}
 
 		return nil, 0
 	}
-	r := thread.getActiveRequest()
 
-	headers := make([]C.go_string, 0, len(r.Header)*2)
+	headers := make([]C.go_string, 0, len(fc.request.Header)*2)
 
-	for field, val := range r.Header {
+	for field, val := range fc.request.Header {
 		fd := unsafe.StringData(field)
 		thread.Pin(fd)
 
@@ -562,10 +446,10 @@ func go_apache_request_headers(threadIndex C.uintptr_t, hasActiveRequest bool) (
 	sd := unsafe.SliceData(headers)
 	thread.Pin(sd)
 
-	return sd, C.size_t(len(r.Header))
+	return sd, C.size_t(len(fc.request.Header))
 }
 
-func addHeader(fc *FrankenPHPContext, cString *C.char, length C.int) {
+func addHeader(fc *frankenPHPContext, cString *C.char, length C.int) {
 	parts := strings.SplitN(C.GoStringN(cString, length), ": ", 2)
 	if len(parts) != 2 {
 		if c := fc.logger.Check(zapcore.DebugLevel, "invalid header"); c != nil {
@@ -579,12 +463,11 @@ func addHeader(fc *FrankenPHPContext, cString *C.char, length C.int) {
 }
 
 //export go_write_headers
-func go_write_headers(threadIndex C.uintptr_t, status C.int, headers *C.zend_llist) {
-	r := phpThreads[threadIndex].getActiveRequest()
-	fc := r.Context().Value(contextKey).(*FrankenPHPContext)
+func go_write_headers(threadIndex C.uintptr_t, status C.int, headers *C.zend_llist) C.bool {
+	fc := phpThreads[threadIndex].getRequestContext()
 
-	if fc.responseWriter == nil {
-		return
+	if fc.isDone || fc.responseWriter == nil {
+		return C.bool(false)
 	}
 
 	current := headers.head
@@ -604,14 +487,18 @@ func go_write_headers(threadIndex C.uintptr_t, status C.int, headers *C.zend_lli
 			delete(h, k)
 		}
 	}
+
+	return C.bool(true)
 }
 
 //export go_sapi_flush
 func go_sapi_flush(threadIndex C.uintptr_t) bool {
-	r := phpThreads[threadIndex].getActiveRequest()
-	fc := r.Context().Value(contextKey).(*FrankenPHPContext)
+	fc := phpThreads[threadIndex].getRequestContext()
+	if fc == nil || fc.responseWriter == nil {
+		return false
+	}
 
-	if fc.responseWriter == nil || clientHasClosed(r) {
+	if fc.clientHasClosed() && !fc.isDone {
 		return true
 	}
 
@@ -626,13 +513,17 @@ func go_sapi_flush(threadIndex C.uintptr_t) bool {
 
 //export go_read_post
 func go_read_post(threadIndex C.uintptr_t, cBuf *C.char, countBytes C.size_t) (readBytes C.size_t) {
-	r := phpThreads[threadIndex].getActiveRequest()
+	fc := phpThreads[threadIndex].getRequestContext()
+
+	if fc.responseWriter == nil {
+		return 0
+	}
 
 	p := unsafe.Slice((*byte)(unsafe.Pointer(cBuf)), countBytes)
 	var err error
 	for readBytes < countBytes && err == nil {
 		var n int
-		n, err = r.Body.Read(p[readBytes:])
+		n, err = fc.request.Body.Read(p[readBytes:])
 		readBytes += C.size_t(n)
 	}
 
@@ -641,7 +532,7 @@ func go_read_post(threadIndex C.uintptr_t, cBuf *C.char, countBytes C.size_t) (r
 
 //export go_read_cookies
 func go_read_cookies(threadIndex C.uintptr_t) *C.char {
-	cookies := phpThreads[threadIndex].getActiveRequest().Header.Values("Cookie")
+	cookies := phpThreads[threadIndex].getRequestContext().request.Header.Values("Cookie")
 	cookie := strings.Join(cookies, "; ")
 	if cookie == "" {
 		return nil
@@ -656,7 +547,6 @@ func go_read_cookies(threadIndex C.uintptr_t) *C.char {
 
 //export go_log
 func go_log(message *C.char, level C.int) {
-	l := getLogger()
 	m := C.GoString(message)
 
 	var le syslogLevel
@@ -668,27 +558,32 @@ func go_log(message *C.char, level C.int) {
 
 	switch le {
 	case emerg, alert, crit, err:
-		if c := l.Check(zapcore.ErrorLevel, m); c != nil {
+		if c := logger.Check(zapcore.ErrorLevel, m); c != nil {
 			c.Write(zap.Stringer("syslog_level", syslogLevel(level)))
 		}
 
 	case warning:
-		if c := l.Check(zapcore.WarnLevel, m); c != nil {
+		if c := logger.Check(zapcore.WarnLevel, m); c != nil {
 			c.Write(zap.Stringer("syslog_level", syslogLevel(level)))
 		}
 
 	case debug:
-		if c := l.Check(zapcore.DebugLevel, m); c != nil {
+		if c := logger.Check(zapcore.DebugLevel, m); c != nil {
 			c.Write(zap.Stringer("syslog_level", syslogLevel(level)))
 		}
 
 	default:
-		if c := l.Check(zapcore.InfoLevel, m); c != nil {
+		if c := logger.Check(zapcore.InfoLevel, m); c != nil {
 			c.Write(zap.Stringer("syslog_level", syslogLevel(level)))
 		}
 	}
 }
 
+//export go_is_context_done
+func go_is_context_done(threadIndex C.uintptr_t) C.bool {
+	return C.bool(phpThreads[threadIndex].getRequestContext().isDone)
+}
+
 // ExecuteScriptCLI executes the PHP script passed as parameter.
 // It returns the exit status code of the script.
 func ExecuteScriptCLI(script string, args []string) int {
@@ -716,17 +611,9 @@ func freeArgs(argv []*C.char) {
 	}
 }
 
-// Ensure that the request path does not contain null bytes
-func requestIsValid(r *http.Request, rw http.ResponseWriter) bool {
-	if !strings.Contains(r.URL.Path, "\x00") {
-		return true
-	}
-	rejectRequest(rw, "Invalid request path")
-	return false
-}
+func executePHPFunction(functionName string) bool {
+	cFunctionName := C.CString(functionName)
+	defer C.free(unsafe.Pointer(cFunctionName))
 
-func rejectRequest(rw http.ResponseWriter, message string) {
-	rw.WriteHeader(http.StatusBadRequest)
-	_, _ = rw.Write([]byte(message))
-	rw.(http.Flusher).Flush()
+	return C.frankenphp_execute_php_function(cFunctionName) == 1
 }

+ 8 - 5
frankenphp.h

@@ -49,12 +49,15 @@ frankenphp_config frankenphp_get_config();
 int frankenphp_new_main_thread(int num_threads);
 bool frankenphp_new_php_thread(uintptr_t thread_index);
 
-int frankenphp_update_server_context(
-    bool create, bool has_main_request, bool has_active_request,
+bool frankenphp_shutdown_dummy_request(void);
+int frankenphp_update_server_context(bool is_worker_request,
 
-    const char *request_method, char *query_string, zend_long content_length,
-    char *path_translated, char *request_uri, const char *content_type,
-    char *auth_user, char *auth_password, int proto_num);
+                                     const char *request_method,
+                                     char *query_string,
+                                     zend_long content_length,
+                                     char *path_translated, char *request_uri,
+                                     const char *content_type, char *auth_user,
+                                     char *auth_password, int proto_num);
 int frankenphp_request_startup();
 int frankenphp_execute_script(char *file_name);
 

+ 21 - 0
phpmainthread_test.go

@@ -152,6 +152,27 @@ func TestAllCommonHeadersAreCorrect(t *testing.T) {
 		assert.True(t, ok, "header is not correctly capitalized: "+header)
 	}
 }
+func TestFinishBootingAWorkerScript(t *testing.T) {
+	logger = zap.NewNop()
+	_, err := initPHPThreads(1, 1, nil)
+	assert.NoError(t, err)
+
+	// boot the worker
+	worker := getDummyWorker("transition-worker-1.php")
+	convertToWorkerThread(phpThreads[0], worker)
+	phpThreads[0].state.waitFor(stateReady)
+
+	assert.NotNil(t, phpThreads[0].handler.(*workerThread).dummyContext)
+	assert.Nil(t, phpThreads[0].handler.(*workerThread).workerContext)
+	assert.False(
+		t,
+		phpThreads[0].handler.(*workerThread).isBootingScript,
+		"isBootingScript should be false after the worker thread is ready",
+	)
+
+	drainPHPThreads()
+	assert.Nil(t, phpThreads)
+}
 
 func getDummyWorker(fileName string) *worker {
 	if workers == nil {

+ 13 - 6
phpthread.go

@@ -4,7 +4,6 @@ package frankenphp
 // #include "frankenphp.h"
 import "C"
 import (
-	"net/http"
 	"runtime"
 	"sync"
 	"unsafe"
@@ -17,7 +16,7 @@ import (
 type phpThread struct {
 	runtime.Pinner
 	threadIndex  int
-	requestChan  chan *http.Request
+	requestChan  chan *frankenPHPContext
 	drainChan    chan struct{}
 	handlerMu    sync.Mutex
 	handler      threadHandler
@@ -30,13 +29,13 @@ type threadHandler interface {
 	name() string
 	beforeScriptExecution() string
 	afterScriptExecution(exitStatus int)
-	getActiveRequest() *http.Request
+	getRequestContext() *frankenPHPContext
 }
 
 func newPHPThread(threadIndex int) *phpThread {
 	return &phpThread{
 		threadIndex: threadIndex,
-		requestChan: make(chan *http.Request),
+		requestChan: make(chan *frankenPHPContext),
 		state:       newThreadState(),
 	}
 }
@@ -59,6 +58,7 @@ func (thread *phpThread) boot() {
 	if !C.frankenphp_new_php_thread(C.uintptr_t(thread.threadIndex)) {
 		logger.Panic("unable to create thread", zap.Int("threadIndex", thread.threadIndex))
 	}
+
 	thread.state.waitFor(stateInactive)
 }
 
@@ -103,8 +103,15 @@ func (thread *phpThread) transitionToNewHandler() string {
 	return thread.handler.beforeScriptExecution()
 }
 
-func (thread *phpThread) getActiveRequest() *http.Request {
-	return thread.handler.getActiveRequest()
+func (thread *phpThread) getRequestContext() *frankenPHPContext {
+	return thread.handler.getRequestContext()
+}
+
+func (thread *phpThread) name() string {
+	thread.handlerMu.Lock()
+	name := thread.handler.name()
+	thread.handlerMu.Unlock()
+	return name
 }
 
 // Pin a string that is not null-terminated

+ 7 - 7
request_options.go

@@ -11,7 +11,7 @@ import (
 )
 
 // RequestOption instances allow to configure a FrankenPHP Request.
-type RequestOption func(h *FrankenPHPContext) error
+type RequestOption func(h *frankenPHPContext) error
 
 var (
 	documentRootCache    sync.Map
@@ -26,7 +26,7 @@ var (
 // symlink is changed without PHP being restarted; enabling this
 // directive will set $_SERVER['DOCUMENT_ROOT'] to the real directory path.
 func WithRequestDocumentRoot(documentRoot string, resolveSymlink bool) RequestOption {
-	return func(o *FrankenPHPContext) (err error) {
+	return func(o *frankenPHPContext) (err error) {
 		v, ok := documentRootCache.Load(documentRoot)
 		if !ok {
 			// make sure file root is absolute
@@ -57,7 +57,7 @@ func WithRequestDocumentRoot(documentRoot string, resolveSymlink bool) RequestOp
 // WithRequestResolvedDocumentRoot is similar to WithRequestDocumentRoot
 // but doesn't do any checks or resolving on the path to improve performance.
 func WithRequestResolvedDocumentRoot(documentRoot string) RequestOption {
-	return func(o *FrankenPHPContext) error {
+	return func(o *frankenPHPContext) error {
 		o.documentRoot = documentRoot
 
 		return nil
@@ -75,7 +75,7 @@ func WithRequestResolvedDocumentRoot(documentRoot string) RequestOption {
 // which can be mitigated with use of a try_files-like behavior
 // that 404s if the FastCGI path info is not found.
 func WithRequestSplitPath(splitPath []string) RequestOption {
-	return func(o *FrankenPHPContext) error {
+	return func(o *frankenPHPContext) error {
 		o.splitPath = splitPath
 
 		return nil
@@ -100,7 +100,7 @@ func WithRequestEnv(env map[string]string) RequestOption {
 }
 
 func WithRequestPreparedEnv(env PreparedEnv) RequestOption {
-	return func(o *FrankenPHPContext) error {
+	return func(o *frankenPHPContext) error {
 		o.env = env
 
 		return nil
@@ -108,7 +108,7 @@ func WithRequestPreparedEnv(env PreparedEnv) RequestOption {
 }
 
 func WithOriginalRequest(r *http.Request) RequestOption {
-	return func(o *FrankenPHPContext) error {
+	return func(o *frankenPHPContext) error {
 		o.originalRequest = r
 
 		return nil
@@ -117,7 +117,7 @@ func WithOriginalRequest(r *http.Request) RequestOption {
 
 // WithRequestLogger sets the logger associated with the current request
 func WithRequestLogger(logger *zap.Logger) RequestOption {
-	return func(o *FrankenPHPContext) error {
+	return func(o *frankenPHPContext) error {
 		o.logger = logger
 
 		return nil

+ 3 - 3
scaling.go

@@ -31,7 +31,7 @@ const (
 )
 
 var (
-	scaleChan         chan *FrankenPHPContext
+	scaleChan         chan *frankenPHPContext
 	autoScaledThreads = []*phpThread{}
 	scalingMu         = new(sync.RWMutex)
 
@@ -47,7 +47,7 @@ func initAutoScaling(mainThread *phpMainThread) {
 	}
 
 	scalingMu.Lock()
-	scaleChan = make(chan *FrankenPHPContext)
+	scaleChan = make(chan *frankenPHPContext)
 	maxScaledThreads := mainThread.maxThreads - mainThread.numThreads
 	autoScaledThreads = make([]*phpThread, 0, maxScaledThreads)
 	scalingMu.Unlock()
@@ -134,7 +134,7 @@ func scaleRegularThread() {
 	autoScaledThreads = append(autoScaledThreads, thread)
 }
 
-func startUpscalingThreads(maxScaledThreads int, scale chan *FrankenPHPContext, done chan struct{}) {
+func startUpscalingThreads(maxScaledThreads int, scale chan *frankenPHPContext, done chan struct{}) {
 	for {
 		scalingMu.Lock()
 		scaledThreadCount := len(autoScaledThreads)

+ 1 - 5
threadinactive.go

@@ -1,9 +1,5 @@
 package frankenphp
 
-import (
-	"net/http"
-)
-
 // representation of a thread with no work assigned to it
 // implements the threadHandler interface
 // each inactive thread weighs around ~350KB
@@ -41,7 +37,7 @@ func (handler *inactiveThread) afterScriptExecution(exitStatus int) {
 	panic("inactive threads should not execute scripts")
 }
 
-func (handler *inactiveThread) getActiveRequest() *http.Request {
+func (handler *inactiveThread) getRequestContext() *frankenPHPContext {
 	return nil
 }
 

Some files were not shown because too many files changed in this diff