Browse Source

fix: race condition on shutdown

Kévin Dunglas 1 year ago
parent
commit
df976c1708
3 changed files with 16 additions and 14 deletions
  1. 11 7
      frankenphp.go
  2. 0 3
      frankenphp_test.go
  3. 5 4
      worker.go

+ 11 - 7
frankenphp.go

@@ -54,6 +54,7 @@ var (
 	ScriptExecutionError        = errors.New("error during PHP script execution")
 
 	requestChan chan *http.Request
+	done        chan struct{}
 	shutdownWG  sync.WaitGroup
 
 	loggerMu sync.RWMutex
@@ -274,6 +275,7 @@ func Init(options ...Option) error {
 	}
 
 	shutdownWG.Add(1)
+	done = make(chan struct{})
 	requestChan = make(chan *http.Request)
 
 	if C.frankenphp_init(C.int(opt.numThreads)) != 0 {
@@ -295,7 +297,7 @@ func Init(options ...Option) error {
 // Shutdown stops the workers and the PHP runtime.
 func Shutdown() {
 	stopWorkers()
-	close(requestChan)
+	close(done)
 	shutdownWG.Wait()
 	requestChan = nil
 
@@ -410,8 +412,9 @@ func ServeHTTP(responseWriter http.ResponseWriter, request *http.Request) error
 		rc = v.(chan *http.Request)
 	}
 
-	if rc != nil {
-		rc <- request
+	select {
+	case <-done:
+	case rc <- request:
 		<-fc.done
 	}
 
@@ -420,12 +423,13 @@ func ServeHTTP(responseWriter http.ResponseWriter, request *http.Request) error
 
 //export go_fetch_request
 func go_fetch_request() C.uintptr_t {
-	r, ok := <-requestChan
-	if !ok {
+	select {
+	case <-done:
 		return 0
-	}
 
-	return C.uintptr_t(cgo.NewHandle(r))
+	case r := <-requestChan:
+		return C.uintptr_t(cgo.NewHandle(r))
+	}
 }
 
 func maybeCloseContext(fc *FrankenPHPContext) {

+ 0 - 3
frankenphp_test.go

@@ -525,13 +525,10 @@ func testFlush(t *testing.T, opts *testOptions) {
 func TestTimeout_module(t *testing.T) {
 	testTimeout(t, &testOptions{})
 }
-
 func TestTimeout_worker(t *testing.T) {
 	testTimeout(t, &testOptions{workerScript: "timeout.php"})
 }
 func testTimeout(t *testing.T, opts *testOptions) {
-	t.Skip("config-dependant")
-
 	config := frankenphp.Config()
 	if !config.ZendMaxExecutionTimers {
 		t.Skip("Zend Timer is not enabled")

+ 5 - 4
worker.go

@@ -108,7 +108,6 @@ func startWorkers(fileName string, nbWorkers int) error {
 func stopWorkers() {
 	workersRequestChans.Range(func(k, v any) bool {
 		workersRequestChans.Delete(k)
-		close(v.(chan *http.Request))
 
 		return true
 	})
@@ -135,12 +134,14 @@ func go_frankenphp_worker_handle_request_start(mrh C.uintptr_t) C.uintptr_t {
 	l := getLogger()
 
 	l.Debug("waiting for request", zap.String("worker", fc.Env["SCRIPT_FILENAME"]))
-	r, ok := <-rc
-	if !ok {
-		// channel closed, server is shutting down
+
+	var r *http.Request
+	select {
+	case <-done:
 		l.Debug("shutting down", zap.String("worker", fc.Env["SCRIPT_FILENAME"]))
 
 		return 0
+	case r = <-rc:
 	}
 
 	fc.currentWorkerRequest = cgo.NewHandle(r)