Fix memory leak

frankenphp.go

@@ -52,9 +52,11 @@ import (
 	//_ "github.com/ianlancetaylor/cgosymbolizer"
 )
 
-type key int
+type contextKeyStruct struct{}
+type handleKeyStruct struct{}
 
-var contextKey key
+var contextKey = contextKeyStruct{}
+var handleKey = handleKeyStruct{}
 
 var (
 	InvalidRequestError         = errors.New("not a FrankenPHP request")
@@ -191,7 +193,10 @@ func NewRequestWithContext(r *http.Request, opts ...RequestOption) (*http.Reques
 	// SCRIPT_FILENAME is the absolute path of SCRIPT_NAME
 	fc.scriptFilename = sanitizedPathJoin(fc.documentRoot, fc.scriptName)
 
-	return r.WithContext(context.WithValue(r.Context(), contextKey, fc)), nil
+	c := context.WithValue(r.Context(), contextKey, fc)
+	c = context.WithValue(c, handleKey, Handles())
+
+	return r.WithContext(c), nil
 }
 
 // FromContext extracts the FrankenPHPContext from a context.
@@ -402,9 +407,12 @@ func updateServerContext(request *http.Request, create bool, mrh C.uintptr_t) er
 
 	var rh cgo.Handle
 	if fc.responseWriter == nil {
-		mrh = C.uintptr_t(cgo.NewHandle(request))
+		h := cgo.NewHandle(request)
+		request.Context().Value(handleKey).(*HandleList).AddHandle(h)
+		mrh = C.uintptr_t(h)
 	} else {
 		rh = cgo.NewHandle(request)
+		request.Context().Value(handleKey).(*HandleList).AddHandle(rh)
 	}
 
 	ret := C.frankenphp_update_server_context(
@@ -467,7 +475,9 @@ func go_fetch_request() C.uintptr_t {
 		return 0
 
 	case r := <-requestChan:
-		return C.uintptr_t(cgo.NewHandle(r))
+		h := cgo.NewHandle(r)
+		r.Context().Value(handleKey).(*HandleList).AddHandle(h)
+		return C.uintptr_t(h)
 	}
 }
 
@@ -480,7 +490,6 @@ func maybeCloseContext(fc *FrankenPHPContext) {
 //export go_execute_script
 func go_execute_script(rh unsafe.Pointer) {
 	handle := cgo.Handle(rh)
-	defer handle.Delete()
 
 	request := handle.Value().(*http.Request)
 	fc, ok := FromContext(request.Context())

smartpointer.go

@@ -0,0 +1,69 @@
+package frankenphp
+
+// #include <stdlib.h>
+import "C"
+import (
+	"runtime/cgo"
+	"unsafe"
+)
+
+/*
+FrankenPHP is fairly complex because it shuffles handles/requests/contexts
+between C and Go. This simplifies the lifecycle management of per-request
+structures by allowing us to hold references until the end of the request
+and ensure they are always cleaned up.
+*/
+
+// PointerList A list of pointers that can be freed at a later time
+type PointerList struct {
+	Pointers []unsafe.Pointer
+}
+
+// HandleList A list of pointers that can be freed at a later time
+type HandleList struct {
+	Handles []cgo.Handle
+}
+
+// AddHandle Call when registering a handle for the very first time
+func (h *HandleList) AddHandle(handle cgo.Handle) {
+	h.Handles = append(h.Handles, handle)
+}
+
+// AddPointer Call when creating a request-level C pointer for the very first time
+func (p *PointerList) AddPointer(ptr unsafe.Pointer) {
+	p.Pointers = append(p.Pointers, ptr)
+}
+
+// FreeAll frees all C pointers
+func (p *PointerList) FreeAll() {
+	for _, ptr := range p.Pointers {
+		C.free(ptr)
+	}
+	p.Pointers = nil // To avoid dangling pointers
+}
+
+// FreeAll frees all handles
+func (h *HandleList) FreeAll() {
+	defer func() {
+		if err := recover(); err != nil {
+			getLogger().Warn("A handle was already deleted manually, indeterminate state")
+		}
+	}()
+	for _, p := range h.Handles {
+		p.Delete()
+	}
+}
+
+// Pointers Get a new list of pointers
+func Pointers() *PointerList {
+	return &PointerList{
+		Pointers: make([]unsafe.Pointer, 0),
+	}
+}
+
+// Handles Get a new list of handles
+func Handles() *HandleList {
+	return &HandleList{
+		Handles: make([]cgo.Handle, 0),
+	}
+}

worker.go

@@ -90,6 +90,9 @@ func startWorkers(fileName string, nbWorkers int, env map[string]string) error {
 						l.Error("unexpected termination, restarting", zap.String("worker", absFileName), zap.Int("exit_status", int(fc.exitStatus)))
 					}
 				} else {
+					// clean up the dummy request
+					r.Context().Value(handleKey).(*HandleList).FreeAll()
+					r = nil
 					break
 				}
 			}
@@ -150,6 +153,7 @@ func go_frankenphp_worker_handle_request_start(mrh C.uintptr_t) C.uintptr_t {
 	}
 
 	fc.currentWorkerRequest = cgo.NewHandle(r)
+	r.Context().Value(handleKey).(*HandleList).AddHandle(fc.currentWorkerRequest)
 
 	l.Debug("request handling started", zap.String("worker", fc.scriptFilename), zap.String("url", r.RequestURI))
 	if err := updateServerContext(r, false, mrh); err != nil {
@@ -169,8 +173,7 @@ func go_frankenphp_finish_request(mrh, rh C.uintptr_t, deleteHandle bool) {
 	fc := r.Context().Value(contextKey).(*FrankenPHPContext)
 
 	if deleteHandle {
-		rHandle.Delete()
-
+		r.Context().Value(handleKey).(*HandleList).FreeAll()
 		cgo.Handle(mrh).Value().(*http.Request).Context().Value(contextKey).(*FrankenPHPContext).currentWorkerRequest = 0
 	}