diff --git a/frankenphp.go b/frankenphp.go index b12f3f142..d5cae3ddf 100644 --- a/frankenphp.go +++ b/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) } } @@ -477,17 +487,21 @@ func maybeCloseContext(fc *FrankenPHPContext) { }) } +// go_execute_script Note: only called in cgi-mode +// //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()) if !ok { panic(InvalidRequestError) } - defer maybeCloseContext(fc) + defer func() { + maybeCloseContext(fc) + request.Context().Value(handleKey).(*handleList).FreeAll() + }() if err := updateServerContext(request, true, 0); err != nil { panic(err) @@ -698,12 +712,23 @@ func ExecuteScriptCLI(script string, args []string) int { cScript := C.CString(script) defer C.free(unsafe.Pointer(cScript)) + argc, argv := convertArgs(args) + defer freeArgs(argv) + + return int(C.frankenphp_execute_script_cli(cScript, argc, (**C.char)(unsafe.Pointer(&argv[0])))) +} + +func convertArgs(args []string) (C.int, []*C.char) { argc := C.int(len(args)) argv := make([]*C.char, argc) for i, arg := range args { argv[i] = C.CString(arg) - defer C.free(unsafe.Pointer(argv[i])) } + return argc, argv +} - return int(C.frankenphp_execute_script_cli(cScript, argc, (**C.char)(unsafe.Pointer(&argv[0])))) +func freeArgs(argv []*C.char) { + for _, arg := range argv { + C.free(unsafe.Pointer(arg)) + } } diff --git a/smartpointer.go b/smartpointer.go new file mode 100644 index 000000000..780b47b22 --- /dev/null +++ b/smartpointer.go @@ -0,0 +1,64 @@ +package frankenphp + +// #include +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() { + 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, 8), + } +} diff --git a/worker.go b/worker.go index 30d01f554..0510d3a6f 100644 --- a/worker.go +++ b/worker.go @@ -150,6 +150,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 +170,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 }