diff --git a/README.md b/README.md index 2e82f4f110..b582a23fe4 100644 --- a/README.md +++ b/README.md @@ -175,6 +175,7 @@ Go to `https://localhost`, and enjoy! - [Laravel integration](https://frankenphp.dev/docs/laravel/) - [Known issues](https://frankenphp.dev/docs/known-issues/) - [Demo app (Symfony) and benchmarks](https://github.com/dunglas/frankenphp-demo) +- [Use as a Go library](https://frankenphp.dev/docs/library/) - [Go library documentation](https://pkg.go.dev/github.com/dunglas/frankenphp) - [Contributing and debugging](https://frankenphp.dev/docs/contributing/) - [Internals (architecture overview)](docs/internals.md) diff --git a/caddy/admin_test.go b/caddy/admin_test.go index 27ece031f3..5ec45fa7c4 100644 --- a/caddy/admin_test.go +++ b/caddy/admin_test.go @@ -348,3 +348,49 @@ func TestAddModuleWorkerViaAdminApi(t *testing.T) { // Make a request to the worker to verify it's working tester.AssertGetResponse("http://localhost:"+testPort+"/worker-with-counter.php", http.StatusOK, "requests:1") } + +func TestRegisteredModuleWorkerPoolsMustBeCorrect(t *testing.T) { + tester := caddytest.NewTester(t) + initServer(t, tester, ` + { + skip_install_trust + admin localhost:2999 + + frankenphp { + num_threads 4 + worker ../testdata/worker-with-env.php 1 + } + } + + http://localhost:`+testPort+` { + route { + php { + root ../testdata + worker worker-with-counter.php 1 { + match /matched* + } + } + php { + root ../testdata + worker worker.php 1 + } + } + } + `, "caddyfile") + + debugState := getDebugState(t, tester) + + worker1Path, _ := fastabs.FastAbs("../testdata/worker-with-env.php") + worker2Path, _ := fastabs.FastAbs("../testdata/worker-with-counter.php") + worker3Path, _ := fastabs.FastAbs("../testdata/worker.php") + receivedThreadNames := make([]string, 0) + for _, thread := range debugState.ThreadDebugStates { + receivedThreadNames = append(receivedThreadNames, thread.Name) + } + + assert.Len(t, receivedThreadNames, 4, "expected 4 threads to be present") + assert.Contains(t, receivedThreadNames, "Regular PHP Thread", "expected a regular thread to be present") + assert.Contains(t, receivedThreadNames, "Worker PHP Thread - "+worker1Path, "expected global worker to be present") + assert.Contains(t, receivedThreadNames, "Worker PHP Thread - "+worker2Path, "expected module worker with \"match\" directive to be present") + assert.Contains(t, receivedThreadNames, "Worker PHP Thread - "+worker3Path, "expected module worker without \"match\" directive to be present") +} diff --git a/caddy/app.go b/caddy/app.go index e4593acc49..59c2e5e233 100644 --- a/caddy/app.go +++ b/caddy/app.go @@ -7,7 +7,6 @@ import ( "log/slog" "path/filepath" "strconv" - "strings" "sync" "time" @@ -60,10 +59,13 @@ type FrankenPHPApp struct { // EXPERIMENTAL: MaxRequests sets the maximum number of requests a PHP thread handles before restarting (0 = unlimited) MaxRequests int `json:"max_requests,omitempty"` - opts []frankenphp.Option - metrics frankenphp.Metrics - ctx context.Context - logger *slog.Logger + opts []frankenphp.Option + metrics frankenphp.Metrics + ctx context.Context + logger *slog.Logger + modules []*FrankenPHPModule + usedWorkerNames map[string]bool + httpApp *caddyhttp.App } var errIni = errors.New(`"php_ini" must be in the format: php_ini "" ""`) @@ -85,7 +87,8 @@ func (f *FrankenPHPApp) Provision(ctx caddy.Context) error { f.opts = make([]frankenphp.Option, 0, 7+len(options)) if httpApp, err := ctx.AppIfConfigured("http"); err == nil { - if httpApp.(*caddyhttp.App).Metrics != nil { + f.httpApp = httpApp.(*caddyhttp.App) + if f.httpApp.Metrics != nil { f.metrics = frankenphp.NewPrometheusMetrics(ctx.GetMetricsRegistry()) } } else { @@ -101,75 +104,9 @@ func (f *FrankenPHPApp) Provision(ctx caddy.Context) error { return nil } -func (f *FrankenPHPApp) generateUniqueModuleWorkerName(filepath string) string { - var i uint - filepath, _ = fastabs.FastAbs(filepath) - name := "m#" + filepath - -retry: - for _, wc := range f.Workers { - if wc.Name == name { - name = fmt.Sprintf("m#%s_%d", filepath, i) - i++ - - goto retry - } - } - - return name -} - -func (f *FrankenPHPApp) addModuleWorkers(workers ...workerConfig) ([]workerConfig, error) { - for i := range workers { - w := &workers[i] - - if frankenphp.EmbeddedAppPath != "" && filepath.IsLocal(w.FileName) { - w.FileName = filepath.Join(frankenphp.EmbeddedAppPath, w.FileName) - } - } - - // A php_server directive is provisioned once per route it's embedded in. Only the first embed - // registers its pools; later embeds reuse them by position, never touching other directives (#2477). - var registered []workerConfig - if len(workers) > 0 && workers[0].routeGroup != "" { - registered = f.moduleWorkersInRouteGroup(workers[0].routeGroup) - } - - for i := range workers { - if i < len(registered) { - workers[i].Name = registered[i].Name - continue - } - - f.registerModuleWorker(&workers[i]) - } - - return workers, nil -} - -func (f *FrankenPHPApp) registerModuleWorker(w *workerConfig) { - if w.Name == "" { - w.Name = f.generateUniqueModuleWorkerName(w.FileName) - } else if !strings.HasPrefix(w.Name, "m#") { - w.Name = "m#" + w.Name - } - - f.Workers = append(f.Workers, *w) -} - -// moduleWorkersInRouteGroup returns the registered workers of one directive, in registration order. -func (f *FrankenPHPApp) moduleWorkersInRouteGroup(routeGroup string) []workerConfig { - var group []workerConfig - for _, w := range f.Workers { - if w.routeGroup == routeGroup { - group = append(group, w) - } - } - - return group -} - func (f *FrankenPHPApp) Start() error { + defer f.reset() // reset after startup since the app persists across reloads + repl := caddy.NewReplacer() optionsMU.RLock() @@ -188,18 +125,19 @@ func (f *FrankenPHPApp) Start() error { frankenphp.WithMaxRequests(f.MaxRequests), ) + // register global workers for _, w := range f.Workers { - w.options = append(w.options, - frankenphp.WithWorkerEnv(w.Env), - frankenphp.WithWorkerWatchMode(w.Watch), - frankenphp.WithWorkerMaxFailures(w.MaxConsecutiveFailures), - frankenphp.WithWorkerMaxThreads(w.MaxThreads), - frankenphp.WithWorkerRequestOptions(w.requestOptions...), - ) - - f.opts = append(f.opts, frankenphp.WithWorkers(w.Name, repl.ReplaceKnown(w.FileName, ""), w.Num, w.options...)) + w.FileName = repl.ReplaceKnown(w.FileName, "") + w.Name = f.createUniqueWorkerName(w, "") + f.opts = append(f.opts, frankenphp.WithWorkers(w.Name, w.FileName, w.Num, w.toWorkerOptions()...)) + } + + if err := f.registerModules(repl); err != nil { + return err } + // if FrankenPHP is currently running, shut it down first + // this will happen in admin API reloads and caddy tests frankenphp.Shutdown() if err := frankenphp.Init(f.opts...); err != nil { return err @@ -220,20 +158,109 @@ func (f *FrankenPHPApp) Stop() error { frankenphp.Shutdown() } - // reset the configuration so it doesn't bleed into later tests + // reset global options + optionsMU.Lock() + options = nil + optionsMU.Unlock() + + return nil +} + +func (f *FrankenPHPApp) reset() { f.Workers = nil f.NumThreads = 0 f.MaxWaitTime = 0 f.MaxIdleTime = 0 f.MaxRequests = 0 + f.PhpIni = nil + f.modules = nil + f.opts = nil + f.ctx = nil + f.metrics = nil + f.usedWorkerNames = nil + f.httpApp = nil +} - optionsMU.Lock() - options = nil - optionsMU.Unlock() +// register workers and servers for "php" and "php_server" modules +func (f *FrankenPHPApp) registerModules(repl *caddy.Replacer) error { + modulesByIndex := make(map[int]*FrankenPHPModule) + for _, module := range f.modules { + if module.ServerIdx == 0 { + if err := f.registerModule(repl, module); err != nil { + return err + } + continue + } + + // modules with the same server_idx should share the same server instance + // example: the worker { match * } rule adds 2 "php" subroutes to the caddy handler + // the 2 handlers belong to the same "php_server" and must therefore share workers + if existingModule, ok := modulesByIndex[module.ServerIdx]; ok { + module.server = existingModule.server + continue + } + + modulesByIndex[module.ServerIdx] = module + if err := f.registerModule(repl, module); err != nil { + return err + } + } + + return nil +} + +// register a server instance and its workers for a single caddy module +func (f *FrankenPHPApp) registerModule(repl *caddy.Replacer, module *FrankenPHPModule) error { + serverName := f.resolveServerName(module) + server, err := frankenphp.NewServer(serverName, module.resolvedDocumentRoot, module.SplitPath, module.resolvedEnv, module.logger) + if err != nil { + return err + } + + module.server = server + f.opts = append(f.opts, frankenphp.WithServer(server)) + + for _, w := range module.Workers { + w.FileName = repl.ReplaceKnown(w.FileName, "") + w.Name = f.createUniqueWorkerName(w, serverName) + workerOptions := append(w.toWorkerOptions(), frankenphp.WithWorkerServerScope(server)) + f.opts = append(f.opts, frankenphp.WithWorkers(w.Name, w.FileName, w.Num, workerOptions...)) + } return nil } +// avoid name collisions for workers +// on collision, a name is first qualified with the server name +// (":") before falling back to a numeric postfix +func (f *FrankenPHPApp) createUniqueWorkerName(wc workerConfig, serverName string) string { + if f.usedWorkerNames == nil { + f.usedWorkerNames = make(map[string]bool) + } + + if wc.Name == "" { + wc.Name, _ = fastabs.FastAbs(wc.FileName) + } + + name := wc.Name + suffix := 0 + for { + if _, ok := f.usedWorkerNames[name]; !ok { + f.usedWorkerNames[name] = true + break + } + if serverName != "" { + name = serverName + ":" + wc.Name + serverName = "" + continue + } + suffix++ + name = fmt.Sprintf("%s_%d", wc.Name, suffix) + } + + return name +} + // UnmarshalCaddyfile implements caddyfile.Unmarshaler. func (f *FrankenPHPApp) UnmarshalCaddyfile(d *caddyfile.Dispenser) error { for d.Next() { @@ -344,9 +371,6 @@ func (f *FrankenPHPApp) UnmarshalCaddyfile(d *caddyfile.Dispenser) error { if frankenphp.EmbeddedAppPath != "" && filepath.IsLocal(wc.FileName) { wc.FileName = filepath.Join(frankenphp.EmbeddedAppPath, wc.FileName) } - if strings.HasPrefix(wc.Name, "m#") { - return d.Errf(`global worker names must not start with "m#": %q`, wc.Name) - } // check for duplicate workers for _, existingWorker := range f.Workers { if existingWorker.FileName == wc.FileName { diff --git a/caddy/config_test.go b/caddy/config_test.go index dbe6a82d58..04498ecf05 100644 --- a/caddy/config_test.go +++ b/caddy/config_test.go @@ -1,7 +1,7 @@ package caddy import ( - "strings" + "path/filepath" "testing" "time" @@ -10,97 +10,6 @@ import ( "github.com/stretchr/testify/require" ) -func TestPhpServerWorkerMatchNoDuplicatePools(t *testing.T) { - const config = ` - { - php { - worker { - file ../testdata/worker-with-env.php - num 2 - match /index.php/* - } - worker { - file ../testdata/worker-with-counter.php - num 1 - } - } - }` - - // a fresh copy per call mirrors the two module instances Caddy provisions for one directive - parseWorkers := func(routeGroup string) []workerConfig { - d := caddyfile.NewTestDispenser(config) - module := &FrankenPHPModule{} - require.NoError(t, module.UnmarshalCaddyfile(d)) - require.Len(t, module.Workers, 2, "block should parse to two workers") - for i := range module.Workers { - module.Workers[i].routeGroup = routeGroup - } - - return module.Workers - } - - app := &FrankenPHPApp{} - - first, err := app.addModuleWorkers(parseWorkers("g1")...) - require.NoError(t, err) - second, err := app.addModuleWorkers(parseWorkers("g1")...) - require.NoError(t, err) - - require.Len(t, app.Workers, 2, "each worker must be registered exactly once") - - for _, w := range app.Workers { - require.False(t, strings.HasSuffix(w.Name, "_0"), - "no _0-suffixed duplicate pool may exist, got %q", w.Name) - } - - // both embeds must resolve to the same pools for serve-time matching - require.Len(t, first, 2) - require.Len(t, second, 2) - for i := range first { - require.Equal(t, first[i].Name, second[i].Name, - "both routes must reference the same pool name for worker %d", i) - } -} - -func TestPhpServerSeparateDirectivesKeepDistinctPools(t *testing.T) { - worker := func(routeGroup string) workerConfig { - return workerConfig{FileName: "../testdata/worker-with-env.php", Num: 2, routeGroup: routeGroup} - } - - app := &FrankenPHPApp{} - site1, err := app.addModuleWorkers(worker("g1")) - require.NoError(t, err) - site2, err := app.addModuleWorkers(worker("g2")) - require.NoError(t, err) - - require.Len(t, app.Workers, 2, "identical workers from separate directives must stay separate pools") - require.NotEqual(t, site1[0].Name, site2[0].Name, "separate directives must not share a pool name") - require.True(t, strings.HasSuffix(site2[0].Name, "_0"), - "the second directive's pool must take a unique _0 name, got %q", site2[0].Name) -} - -func TestPhpServerEmbedReuseIsPositional(t *testing.T) { - embed := func() []workerConfig { - return []workerConfig{ - {FileName: "../testdata/worker-with-env.php", Num: 1, MatchPath: []string{"/x/*"}, routeGroup: "g1"}, - {FileName: "../testdata/worker-with-env.php", Num: 1, MatchPath: []string{"/x/*"}, routeGroup: "g1"}, - } - } - - app := &FrankenPHPApp{} - first, err := app.addModuleWorkers(embed()...) - require.NoError(t, err) - second, err := app.addModuleWorkers(embed()...) - require.NoError(t, err) - - require.Len(t, app.Workers, 2, "two identical workers in one block stay two pools, just as without the duplicate embed") - require.NotEqual(t, first[0].Name, first[1].Name, "the second identical worker must take its own _0 pool") - require.True(t, strings.HasSuffix(first[1].Name, "_0"), "got %q", first[1].Name) - for i := range first { - require.Equal(t, first[i].Name, second[i].Name, "the duplicate embed must reuse pools by position for worker %d", i) - } -} - func TestModuleRequestBodyTimeout(t *testing.T) { d := caddyfile.NewTestDispenser(` { @@ -198,7 +107,6 @@ func TestModuleWorkersDifferentNamesSucceed(t *testing.T) { // Parse the first configuration d1 := caddyfile.NewTestDispenser(configWithWorkerName1) - app := &FrankenPHPApp{} module1 := &FrankenPHPModule{} // Unmarshal the first configuration @@ -226,16 +134,6 @@ func TestModuleWorkersDifferentNamesSucceed(t *testing.T) { // Verify that no error was returned require.NoError(t, err, "Expected no error when two workers have different names") - - _, err = app.addModuleWorkers(module1.Workers...) - require.NoError(t, err, "Expected no error when adding the first module workers") - _, err = app.addModuleWorkers(module2.Workers...) - require.NoError(t, err, "Expected no error when adding the second module workers") - - // Verify that both workers were added - require.Len(t, app.Workers, 2, "Expected two workers in the app") - require.Equal(t, "m#test-worker-1", app.Workers[0].Name, "First worker should have the correct name") - require.Equal(t, "m#test-worker-2", app.Workers[1].Name, "Second worker should have the correct name") } func TestModuleWorkerWithEnvironmentVariables(t *testing.T) { @@ -324,7 +222,6 @@ func TestModuleWorkerWithCustomName(t *testing.T) { // Parse the configuration d := caddyfile.NewTestDispenser(configWithCustomName) module := &FrankenPHPModule{} - app := &FrankenPHPApp{} // Unmarshal the configuration err := module.UnmarshalCaddyfile(d) @@ -335,10 +232,40 @@ func TestModuleWorkerWithCustomName(t *testing.T) { // Verify that the worker was added to the module require.Len(t, module.Workers, 1, "Expected one worker to be added to the module") require.Equal(t, "../testdata/worker-with-env.php", module.Workers[0].FileName, "Worker should have the correct filename") +} - // Verify that the worker was added to app.Workers with the m# prefix - module.Workers, err = app.addModuleWorkers(module.Workers...) - require.NoError(t, err, "Expected no error when adding the worker to the app") - require.Equal(t, "m#custom-worker-name", module.Workers[0].Name, "Worker should have the custom name, prefixed with m#") - require.Equal(t, "m#custom-worker-name", app.Workers[0].Name, "Worker should have the custom name, prefixed with m#") +func TestCreateUniqueWorkerNames(t *testing.T) { + app := &FrankenPHPApp{} + filename := "../testdata/worker-with-env.php" + absFileName, _ := filepath.Abs(filename) + names := make([]string, 6) + for i := 0; i < 3; i++ { + names[i] = app.createUniqueWorkerName(workerConfig{ + FileName: filename, + Name: "custom-worker-name", + }, "") + names[i+3] = app.createUniqueWorkerName(workerConfig{ + FileName: filename, + }, "") + } + + require.Equal(t, "custom-worker-name", names[0]) + require.Equal(t, "custom-worker-name_1", names[1]) + require.Equal(t, "custom-worker-name_2", names[2]) + require.Equal(t, absFileName, names[3]) + require.Equal(t, absFileName+"_1", names[4]) + require.Equal(t, absFileName+"_2", names[5]) +} + +func TestCreateUniqueWorkerNamesQualifiedByServer(t *testing.T) { + app := &FrankenPHPApp{} + wc := workerConfig{FileName: "../testdata/worker-with-env.php", Name: "queue"} + + require.Equal(t, "queue", app.createUniqueWorkerName(wc, "one.example.com")) + // on collision, the name is qualified with the server name + require.Equal(t, "two.example.com:queue", app.createUniqueWorkerName(wc, "two.example.com")) + // when the qualified name is also taken, fall back to the numeric postfix + require.Equal(t, "queue_1", app.createUniqueWorkerName(wc, "two.example.com")) + // workers without a server keep the numeric postfix behavior + require.Equal(t, "queue_2", app.createUniqueWorkerName(wc, "")) } diff --git a/caddy/hotreload.go b/caddy/hotreload.go index 4b9c0ebe69..bec8b16fbd 100644 --- a/caddy/hotreload.go +++ b/caddy/hotreload.go @@ -49,7 +49,12 @@ func (f *FrankenPHPModule) configureHotReload(app *FrankenPHPApp) error { } app.opts = append(app.opts, frankenphp.WithHotReload(f.HotReload.Topic, f.mercureHub, f.HotReload.Watch)) - f.preparedEnv["FRANKENPHP_HOT_RELOAD\x00"] = "/.well-known/mercure?topic=" + url.QueryEscape(f.HotReload.Topic) + + // add the hot reload to the env variables + if f.Env == nil { + f.Env = make(map[string]string) + } + f.Env["FRANKENPHP_HOT_RELOAD"] = "/.well-known/mercure?topic=" + url.QueryEscape(f.HotReload.Topic) return nil } diff --git a/caddy/module.go b/caddy/module.go index 02d410155b..52b895e838 100644 --- a/caddy/module.go +++ b/caddy/module.go @@ -26,6 +26,7 @@ import ( // FrankenPHPModule represents the "php_server" and "php" directives in the Caddyfile // they are responsible for forwarding requests to FrankenPHP via "ServeHTTP" +// keep in mind that the module reference gets lost between Caddy parsing and Caddy provisioning // // example.com { // php_server { @@ -46,16 +47,19 @@ type FrankenPHPModule struct { Env map[string]string `json:"env,omitempty"` // Workers configures the worker scripts to start. Workers []workerConfig `json:"workers,omitempty"` - // RouteGroup is set automatically to pair the route embeds of one php_server directive (#2477). Do not set it manually. - RouteGroup string `json:"route_group,omitempty"` + // ServerIdx is the idx of the php_server this module belongs to + ServerIdx int `json:"server_idx,omitempty"` // RequestBodyTimeout is an idle timeout on request body reads: a stalled (slow POST) client is cut off while a steady upload of any size succeeds. Defaults to 60s when omitted; set to 0 to disable. RequestBodyTimeout *caddy.Duration `json:"request_body_timeout,omitempty"` - - resolvedDocumentRoot string - preparedEnv frankenphp.PreparedEnv - preparedEnvNeedsReplacement bool - logger *slog.Logger - requestOptions []frankenphp.RequestOption + // Name is the name of the php_server this module belongs to for logging purposes + Name string `json:"name,omitempty"` + + resolvedDocumentRoot string + resolvedEnv map[string]string + requestEnv frankenphp.PreparedEnv + requestOptions []frankenphp.RequestOption + server *frankenphp.Server + logger *slog.Logger } // CaddyModule returns the Caddy module information. @@ -69,6 +73,7 @@ func (FrankenPHPModule) CaddyModule() caddy.ModuleInfo { // Provision sets up the module. func (f *FrankenPHPModule) Provision(ctx caddy.Context) error { f.logger = ctx.Slogger() + app, err := ctx.App("frankenphp") if err != nil { return err @@ -83,7 +88,6 @@ func (f *FrankenPHPModule) Provision(ctx caddy.Context) error { f.assignMercureHub(ctx) - loggerOpt := frankenphp.WithRequestLogger(f.logger) for i, wc := range f.Workers { // make the file path absolute from the public directory // this can only be done if the root is defined inside php_server @@ -91,22 +95,13 @@ func (f *FrankenPHPModule) Provision(ctx caddy.Context) error { wc.FileName = filepath.Join(f.Root, wc.FileName) } - // Inherit environment variables from the parent php_server directive - if f.Env != nil { - wc.inheritEnv(f.Env) + if frankenphp.EmbeddedAppPath != "" && filepath.IsLocal(wc.FileName) { + wc.FileName = filepath.Join(frankenphp.EmbeddedAppPath, wc.FileName) } - wc.requestOptions = append(wc.requestOptions, loggerOpt) - wc.routeGroup = f.RouteGroup f.Workers[i] = wc } - workers, err := fapp.addModuleWorkers(f.Workers...) - if err != nil { - return err - } - f.Workers = workers - if f.Root == "" { if frankenphp.EmbeddedAppPath == "" { f.Root = "{http.vars.root}" @@ -141,11 +136,6 @@ func (f *FrankenPHPModule) Provision(ctx caddy.Context) error { f.ResolveRootSymlink = new(true) } - // Always pre-compute absolute file names for fallback matching - for i := range f.Workers { - f.Workers[i].absFileName, _ = fastabs.FastAbs(f.Workers[i].FileName) - } - if !needReplacement(f.Root) { root, err := fastabs.FastAbs(f.Root) if err != nil { @@ -166,41 +156,27 @@ func (f *FrankenPHPModule) Provision(ctx caddy.Context) error { if filepath.IsAbs(wc.FileName) { resolvedPath, _ := filepath.EvalSymlinks(wc.FileName) f.Workers[i].FileName = resolvedPath - f.Workers[i].absFileName = resolvedPath } } } - - // Pre-compute relative match paths for all workers (requires resolved document root) - docRootWithSep := f.resolvedDocumentRoot + string(filepath.Separator) - for i := range f.Workers { - if strings.HasPrefix(f.Workers[i].absFileName, docRootWithSep) { - f.Workers[i].matchRelPath = filepath.ToSlash(f.Workers[i].absFileName[len(f.resolvedDocumentRoot):]) - } - } - - f.requestOptions = append(f.requestOptions, frankenphp.WithRequestResolvedDocumentRoot(f.resolvedDocumentRoot)) } - if f.preparedEnv == nil { - f.preparedEnv = frankenphp.PrepareEnv(f.Env) + if err := f.configureHotReload(fapp); err != nil { + return err + } - for _, e := range f.preparedEnv { - if needReplacement(e) { - f.preparedEnvNeedsReplacement = true + f.resolvedEnv = make(map[string]string) // env variables that do not need replacement + f.requestEnv = make(map[string]string) // env variables that need replacement, e.g. {http.vars.root} - break - } + for k, e := range f.Env { + if needReplacement(e) { + f.requestEnv[k+"\x00"] = e // prepare the env with a null byte + } else { + f.resolvedEnv[k] = e } } - if !f.preparedEnvNeedsReplacement { - f.requestOptions = append(f.requestOptions, frankenphp.WithRequestPreparedEnv(f.preparedEnv)) - } - - if err := f.configureHotReload(fapp); err != nil { - return err - } + fapp.modules = append(fapp.modules, f) return nil } @@ -215,13 +191,13 @@ func (f *FrankenPHPModule) ServeHTTP(w http.ResponseWriter, r *http.Request, _ c ctx := r.Context() repl := ctx.Value(caddy.ReplacerCtxKey).(*caddy.Replacer) - documentRoot := f.resolvedDocumentRoot - opts := make([]frankenphp.RequestOption, 0, len(f.requestOptions)+4) opts = append(opts, f.requestOptions...) + opts = append(opts, frankenphp.WithOriginalRequest(new(ctx.Value(caddyhttp.OriginalRequestCtxKey).(http.Request)))) - if documentRoot == "" { - documentRoot = repl.ReplaceKnown(f.Root, "") + // if the root contains a caddy placeholder, it needs to be resolved here in the hot path + if f.resolvedDocumentRoot == "" { + documentRoot := repl.ReplaceKnown(f.Root, "") if documentRoot == "" && frankenphp.EmbeddedAppPath != "" { documentRoot = frankenphp.EmbeddedAppPath } @@ -232,37 +208,19 @@ func (f *FrankenPHPModule) ServeHTTP(w http.ResponseWriter, r *http.Request, _ c opts = append(opts, frankenphp.WithRequestDocumentRoot(documentRoot, false)) } - if f.preparedEnvNeedsReplacement { - env := make(frankenphp.PreparedEnv, len(f.Env)) - for k, v := range f.preparedEnv { + // all env variables that contain caddy placeholders need to be resolved here in the hot path + if len(f.requestEnv) > 0 { + env := make(frankenphp.PreparedEnv, len(f.requestEnv)) + for k, v := range f.requestEnv { env[k] = repl.ReplaceKnown(v, "") } opts = append(opts, frankenphp.WithRequestPreparedEnv(env)) } - workerName := "" - for _, w := range f.Workers { - if w.matchesPath(r, documentRoot) { - workerName = w.Name - break - } - } - - fr, err := frankenphp.NewRequestWithContext( - r, - append( - opts, - frankenphp.WithOriginalRequest(new(ctx.Value(caddyhttp.OriginalRequestCtxKey).(http.Request))), - frankenphp.WithWorkerName(workerName), - )..., - ) - - if err != nil { - return caddyhttp.Error(http.StatusInternalServerError, err) - } + err := f.server.ServeHTTP(w, r, opts...) - if err = frankenphp.ServeHTTP(w, fr); err != nil && !errors.As(err, &frankenphp.ErrRejected{}) { + if err != nil && !errors.As(err, &frankenphp.ErrRejected{}) { return caddyhttp.Error(http.StatusInternalServerError, err) } @@ -293,10 +251,8 @@ func (f *FrankenPHPModule) UnmarshalCaddyfile(d *caddyfile.Dispenser) error { } if f.Env == nil { f.Env = make(map[string]string) - f.preparedEnv = make(frankenphp.PreparedEnv) } f.Env[args[0]] = args[1] - f.preparedEnv[args[0]+"\x00"] = args[1] case "resolve_root_symlink": if !d.NextArg() { @@ -311,6 +267,12 @@ func (f *FrankenPHPModule) UnmarshalCaddyfile(d *caddyfile.Dispenser) error { } f.ResolveRootSymlink = &v + case "name": + if !d.NextArg() { + return d.ArgErr() + } + f.Name = d.Val() + case "worker": wc, err := unmarshalWorker(d) if err != nil { @@ -358,16 +320,22 @@ func (f *FrankenPHPModule) UnmarshalCaddyfile(d *caddyfile.Dispenser) error { return nil } +func (f *FrankenPHPModule) assignServerIdx(h httpcaddyfile.Helper) { + counter, _ := h.State["php_server_count"].(int) + f.ServerIdx = counter + 1 + h.State["php_server_count"] = counter + 1 +} + // parseCaddyfile unmarshals tokens from h into a new Middleware. func parseCaddyfile(h httpcaddyfile.Helper) (caddyhttp.MiddlewareHandler, error) { m := &FrankenPHPModule{} err := m.UnmarshalCaddyfile(h.Dispenser) + m.assignServerIdx(h) + return m, err } -const routeGroupStateKey = "frankenphp.worker_route_group_seq" - // parsePhpServer parses the php_server directive, which has a similar syntax // to the php_fastcgi directive. A line such as this: // @@ -400,12 +368,6 @@ func parsePhpServer(h httpcaddyfile.Helper) ([]httpcaddyfile.ConfigValue, error) return nil, h.ArgErr() } - // per-adaptation counter: identical for both embeds of this directive, distinct for every other - // (including separate snippet imports), and stable across re-adaptation since State resets each time - seq, _ := h.State[routeGroupStateKey].(int) - h.State[routeGroupStateKey] = seq + 1 - routeGroup := strconv.Itoa(seq) - // set up FrankenPHP phpsrv := FrankenPHPModule{} @@ -505,6 +467,9 @@ func parsePhpServer(h httpcaddyfile.Helper) ([]httpcaddyfile.ConfigValue, error) return nil, err } + // assign a unique index to the php server + phpsrv.assignServerIdx(h) + if frankenphp.EmbeddedAppPath != "" { if phpsrv.Root == "" { phpsrv.Root = filepath.Join(frankenphp.EmbeddedAppPath, defaultDocumentRoot) @@ -516,17 +481,15 @@ func parsePhpServer(h httpcaddyfile.Helper) ([]httpcaddyfile.ConfigValue, error) } } - phpsrv.RouteGroup = routeGroup - // set up a route list that we'll append to routes := caddyhttp.RouteList{} - // prepend routes from the 'worker match *' directives - routes = prependWorkerRoutes(routes, h, phpsrv, fsrv, disableFsrv) - // set the list of allowed path segments on which to split phpsrv.SplitPath = extensions + // prepend routes from the 'worker match *' directives + routes = prependWorkerRoutes(routes, h, phpsrv, fsrv, disableFsrv) + // if the index is turned off, we skip the redirect and try_files if indexFile != "off" { dirRedir := false diff --git a/caddy/serveridx_test.go b/caddy/serveridx_test.go new file mode 100644 index 0000000000..f5271acfb7 --- /dev/null +++ b/caddy/serveridx_test.go @@ -0,0 +1,66 @@ +package caddy + +import ( + "testing" + + "github.com/caddyserver/caddy/v2" + "github.com/caddyserver/caddy/v2/caddyconfig/httpcaddyfile" + "github.com/stretchr/testify/require" +) + +func TestAssignServerIdxIncrements(t *testing.T) { + h := httpcaddyfile.Helper{State: map[string]any{}} + m1 := &FrankenPHPModule{} + m2 := &FrankenPHPModule{} + + m1.assignServerIdx(h) + m2.assignServerIdx(h) + + require.Equal(t, 1, m1.ServerIdx) + require.Equal(t, 2, m2.ServerIdx) +} + +func TestRegisterModulesWithSameServerIdxShareOneServer(t *testing.T) { + app := &FrankenPHPApp{} + shared1 := &FrankenPHPModule{ServerIdx: 1, resolvedDocumentRoot: "../testdata"} + shared2 := &FrankenPHPModule{ServerIdx: 1, resolvedDocumentRoot: "../testdata"} + app.modules = []*FrankenPHPModule{shared1, shared2} + + require.NoError(t, app.registerModules(caddy.NewReplacer())) + + require.NotNil(t, shared1.server) + require.Same(t, shared1.server, shared2.server, "modules with the same server_idx must share one server instance") +} + +func TestRegisterModulesWithoutServerIdxGetOwnServers(t *testing.T) { + app := &FrankenPHPApp{} + auto1 := &FrankenPHPModule{resolvedDocumentRoot: "../testdata"} + auto2 := &FrankenPHPModule{resolvedDocumentRoot: "../testdata"} + indexed := &FrankenPHPModule{ServerIdx: 1, resolvedDocumentRoot: "../testdata"} + app.modules = []*FrankenPHPModule{auto1, indexed, auto2} + + require.NoError(t, app.registerModules(caddy.NewReplacer())) + + require.NotNil(t, auto1.server) + require.NotNil(t, auto2.server) + require.NotNil(t, indexed.server) + require.NotSame(t, auto1.server, auto2.server, "modules without server_idx must each get their own server") + require.NotSame(t, auto1.server, indexed.server) + require.NotSame(t, auto2.server, indexed.server) +} + +// regression test for the double-registration scenario: a module POSTed via +// the admin API with an explicit server_idx must not steal the server of a +// module that already registered under the same index; it joins it instead, +// and the first registered module defines the server configuration +func TestRegisterModulesFirstModuleWinsPerIdx(t *testing.T) { + app := &FrankenPHPApp{} + first := &FrankenPHPModule{ServerIdx: 2, resolvedDocumentRoot: "../testdata"} + second := &FrankenPHPModule{ServerIdx: 2, resolvedDocumentRoot: "../testdata/env"} + app.modules = []*FrankenPHPModule{first, second} + + require.NoError(t, app.registerModules(caddy.NewReplacer())) + + require.Same(t, first.server, second.server) + require.Len(t, app.opts, 1, "only one server must be registered for a shared index") +} diff --git a/caddy/servername.go b/caddy/servername.go new file mode 100644 index 0000000000..3ca25767fd --- /dev/null +++ b/caddy/servername.go @@ -0,0 +1,86 @@ +package caddy + +import ( + "github.com/caddyserver/caddy/v2/modules/caddyhttp" +) + +// resolveServerName picks a stable, human-friendly name for the php_server +// block represented by module, so workers, metrics and logs can be attributed +// to a block (e.g. "api.example.com") instead of an opaque index. Cascade: +// 1. First host of the enclosing route's host matcher. +// 2. First listener address of the http server containing the module. +// +// Returns "" when the http app is not available or the module cannot be +// located in any route tree. +func (f *FrankenPHPApp) resolveServerName(module *FrankenPHPModule) string { + if module.Name != "" { + return module.Name + } + + if f.httpApp == nil { + return "" + } + + for _, srv := range f.httpApp.Servers { + if !serverContainsHandler(srv, module) { + continue + } + if h := findHostInRoutes(srv.Routes, module); h != "" { + return h + } + if len(srv.Listen) > 0 { + return srv.Listen[0] + } + } + + return "" +} + +// findHostInRoutes walks routes (recursing into Subroute handlers) to locate +// the route that contains target, then returns the first host of that route's +// host matcher. Returns "" if no enclosing route or no host matcher is found. +func findHostInRoutes(routes caddyhttp.RouteList, target caddyhttp.MiddlewareHandler) string { + for _, route := range routes { + if !routeContainsHandler(route, target) { + continue + } + for _, mset := range route.MatcherSets { + for _, m := range mset { + hp, ok := m.(*caddyhttp.MatchHost) + if !ok || hp == nil || len(*hp) == 0 { + continue + } + return (*hp)[0] + } + } + } + + return "" +} + +func serverContainsHandler(srv *caddyhttp.Server, target caddyhttp.MiddlewareHandler) bool { + for _, route := range srv.Routes { + if routeContainsHandler(route, target) { + return true + } + } + + return false +} + +func routeContainsHandler(route caddyhttp.Route, target caddyhttp.MiddlewareHandler) bool { + for _, h := range route.Handlers { + if h == target { + return true + } + if sub, ok := h.(*caddyhttp.Subroute); ok { + for _, r := range sub.Routes { + if routeContainsHandler(r, target) { + return true + } + } + } + } + + return false +} diff --git a/caddy/servername_test.go b/caddy/servername_test.go new file mode 100644 index 0000000000..a27d068bb1 --- /dev/null +++ b/caddy/servername_test.go @@ -0,0 +1,64 @@ +package caddy + +import ( + "testing" + + "github.com/caddyserver/caddy/v2/modules/caddyhttp" + "github.com/stretchr/testify/require" +) + +func TestResolveServerName(t *testing.T) { + moduleWithHost := &FrankenPHPModule{} + moduleWithoutHost := &FrankenPHPModule{} + moduleInSubroute := &FrankenPHPModule{} + unroutedModule := &FrankenPHPModule{} + namedModule := &FrankenPHPModule{Name: "custom name"} + + host := caddyhttp.MatchHost{"api.example.com", "www.example.com"} + + app := &FrankenPHPApp{ + httpApp: &caddyhttp.App{ + Servers: map[string]*caddyhttp.Server{ + "srv0": { + Listen: []string{":8080"}, + Routes: caddyhttp.RouteList{ + { + MatcherSets: caddyhttp.MatcherSets{{&host}}, + Handlers: []caddyhttp.MiddlewareHandler{moduleWithHost}, + }, + { + Handlers: []caddyhttp.MiddlewareHandler{moduleWithoutHost}, + }, + { + Handlers: []caddyhttp.MiddlewareHandler{ + &caddyhttp.Subroute{ + Routes: caddyhttp.RouteList{ + {Handlers: []caddyhttp.MiddlewareHandler{moduleInSubroute}}, + }, + }, + }, + }, + }, + }, + }, + }, + } + + // first host of the enclosing route's host matcher wins + require.Equal(t, "api.example.com", app.resolveServerName(moduleWithHost)) + + // no host matcher: fall back to the first listener address + require.Equal(t, ":8080", app.resolveServerName(moduleWithoutHost)) + + // modules nested in subroutes are found too + require.Equal(t, ":8080", app.resolveServerName(moduleInSubroute)) + + // module not present in any route tree + require.Equal(t, "", app.resolveServerName(unroutedModule)) + + // no http app configured + require.Equal(t, "", (&FrankenPHPApp{}).resolveServerName(moduleWithHost)) + + // module with a name + require.Equal(t, "custom name", app.resolveServerName(namedModule)) +} diff --git a/caddy/workerconfig.go b/caddy/workerconfig.go index 0ade09fee3..42e1fc2c18 100644 --- a/caddy/workerconfig.go +++ b/caddy/workerconfig.go @@ -1,8 +1,6 @@ package caddy import ( - "net/http" - "path" "path/filepath" "strconv" @@ -10,7 +8,6 @@ import ( "github.com/caddyserver/caddy/v2/caddyconfig/caddyfile" "github.com/caddyserver/caddy/v2/modules/caddyhttp" "github.com/dunglas/frankenphp" - "github.com/dunglas/frankenphp/internal/fastabs" ) // workerConfig represents the "worker" directive in the Caddyfile @@ -25,7 +22,7 @@ import ( type workerConfig struct { mercureContext - // Name for the worker. Default: the filename for FrankenPHPApp workers, always prefixed with "m#" for FrankenPHPModule workers. + // Name for the worker. Default: the absolute path of the worker file, postfixed with a number if the name is already used. Name string `json:"name,omitempty"` // FileName sets the path to the worker script. FileName string `json:"file_name,omitempty"` @@ -44,9 +41,6 @@ type workerConfig struct { options []frankenphp.WorkerOption requestOptions []frankenphp.RequestOption - absFileName string - matchRelPath string // pre-computed relative URL path for fast matching - routeGroup string // identifies the php_server directive whose route embeds share this pool } func unmarshalWorker(d *caddyfile.Dispenser) (workerConfig, error) { @@ -162,41 +156,21 @@ func unmarshalWorker(d *caddyfile.Dispenser) (workerConfig, error) { return wc, nil } -func (wc *workerConfig) inheritEnv(env map[string]string) { - if wc.Env == nil { - wc.Env = make(map[string]string, len(env)) +func (wc *workerConfig) toWorkerOptions() []frankenphp.WorkerOption { + opts := []frankenphp.WorkerOption{ + frankenphp.WithWorkerEnv(wc.Env), + frankenphp.WithWorkerWatchMode(wc.Watch), + frankenphp.WithWorkerMaxFailures(wc.MaxConsecutiveFailures), + frankenphp.WithWorkerMaxThreads(wc.MaxThreads), + frankenphp.WithWorkerRequestOptions(wc.requestOptions...), } - for k, v := range env { - // do not overwrite existing environment variables - if _, exists := wc.Env[k]; !exists { - wc.Env[k] = v - } - } -} -func (wc *workerConfig) matchesPath(r *http.Request, documentRoot string) bool { - // try to match against a pattern if one is assigned - if len(wc.MatchPath) != 0 { - return (caddyhttp.MatchPath)(wc.MatchPath).Match(r) + // copy the caddy match logic and create a unique matcher function for this worker + // inject the matcher into frankenphp + if len(wc.MatchPath) > 0 { + matchFunc := caddyhttp.MatchPath(append([]string(nil), wc.MatchPath...)) + _ = matchFunc.Provision(caddy.Context{}) + opts = append(opts, frankenphp.WithWorkerMatcher(matchFunc.Match)) } - - // fast path: compare the request URL path against the pre-computed relative path - if wc.matchRelPath != "" { - reqPath := r.URL.Path - if reqPath == wc.matchRelPath { - return true - } - - // ensure leading slash for relative paths (see #2166) - if reqPath == "" || reqPath[0] != '/' { - reqPath = "/" + reqPath - } - - return path.Clean(reqPath) == wc.matchRelPath - } - - // fallback when documentRoot is dynamic (contains placeholders) - fullPath, _ := fastabs.FastAbs(filepath.Join(documentRoot, r.URL.Path)) - - return fullPath == wc.absFileName + return opts } diff --git a/cgi.go b/cgi.go index 7f31c36293..582cdbb2a4 100644 --- a/cgi.go +++ b/cgi.go @@ -92,13 +92,6 @@ func addKnownVariablesToServer(fc *frankenPHPContext, trackVarsArray *C.zval) { serverPort := reqPort contentLength := request.Header.Get("Content-Length") - var requestURI string - if fc.originalRequest != nil { - requestURI = fc.originalRequest.URL.RequestURI() - } else { - requestURI = fc.requestURI - } - phpSelf := fc.scriptName + fc.pathInfo C.frankenphp_register_server_vars(trackVarsArray, C.frankenphp_server_vars{ @@ -135,8 +128,8 @@ func addKnownVariablesToServer(fc *frankenPHPContext, trackVarsArray *C.zval) { server_protocol_len: C.size_t(len(request.Proto)), http_host: toUnsafeChar(request.Host), http_host_len: C.size_t(len(request.Host)), - request_uri: toUnsafeChar(requestURI), - request_uri_len: C.size_t(len(requestURI)), + request_uri: toUnsafeChar(fc.requestURI), + request_uri_len: C.size_t(len(fc.requestURI)), ssl_cipher: toUnsafeChar(sslCipher), ssl_cipher_len: C.size_t(len(sslCipher)), @@ -163,26 +156,28 @@ func addHeadersToServer(ctx context.Context, request *http.Request, trackVarsArr } } -// registerPreparedEnv exposes fc.env to getenv() before any PHP code runs. -func registerPreparedEnv(env PreparedEnv) { - size := C.size_t(len(env)) - for k, v := range env { - C.frankenphp_add_to_prepared_env(toUnsafeChar(k), C.size_t(len(k)-1), toUnsafeChar(v), C.size_t(len(v)), size) +// registerPreparedEnv exposes fc.env and fc.server.env to getenv() before any PHP code runs. +func registerPreparedEnv(fc *frankenPHPContext, preparedEnvLen int) { + for k, v := range fc.server.env { + C.frankenphp_add_to_prepared_env(toUnsafeChar(k), C.size_t(len(k)-1), toUnsafeChar(v), C.size_t(len(v)), C.size_t(preparedEnvLen)) + } + for k, v := range fc.env { + C.frankenphp_add_to_prepared_env(toUnsafeChar(k), C.size_t(len(k)-1), toUnsafeChar(v), C.size_t(len(v)), C.size_t(preparedEnvLen)) } } //export go_register_server_variables func go_register_server_variables(threadIndex C.uintptr_t, trackVarsArray *C.zval) { thread := phpThreads[threadIndex] - fc := thread.frankenPHPContext() + fc := thread.handler.frankenPHPContext() if fc.request != nil { addKnownVariablesToServer(fc, trackVarsArray) - addHeadersToServer(thread.context(), fc.request, trackVarsArray) + addHeadersToServer(fc.ctx, fc.request, trackVarsArray) } // The Prepared Environment is registered last and can overwrite any previous values - if len(fc.env) != 0 { + if len(fc.env) != 0 || len(fc.server.env) != 0 { C.frankenphp_merge_with_prepared_env(trackVarsArray) } } @@ -222,7 +217,13 @@ func splitCgiPath(fc *frankenPHPContext) { // TODO: is it possible to delay this and avoid saving everything in the context? // SCRIPT_FILENAME is the absolute path of SCRIPT_NAME fc.scriptFilename = sanitizedPathJoin(fc.documentRoot, fc.scriptName) - fc.worker = workersByPath[fc.scriptFilename] + + // see if a php_server worker or global worker matches the request path + // aka: root + request path == worker.filename + fc.worker = fc.server.workersByPath[fc.scriptFilename] + if fc.worker == nil { + fc.worker = globalWorkersByPath[fc.scriptFilename] + } } // splitPos returns the index where path should be split based on splitPath. @@ -284,15 +285,16 @@ func splitPos(path string, splitPath []string) int { //export go_update_request_info func go_update_request_info(threadIndex C.uintptr_t, info *C.sapi_request_info) *C.char { thread := phpThreads[threadIndex] - fc := thread.frankenPHPContext() + fc := thread.handler.frankenPHPContext() request := fc.request if request == nil { return nil } - if len(fc.env) != 0 { - registerPreparedEnv(fc.env) + preparedEnvLen := len(fc.env) + len(fc.server.env) + if preparedEnvLen != 0 { + registerPreparedEnv(fc, preparedEnvLen) } if m, ok := cStringHTTPMethods[request.Method]; ok { diff --git a/context.go b/context.go index c582c6e453..c5ed828a26 100644 --- a/context.go +++ b/context.go @@ -7,6 +7,7 @@ import ( "log/slog" "net/http" "os" + "path/filepath" "strconv" "strings" "time" @@ -16,13 +17,14 @@ import ( type frankenPHPContext struct { mercureContext - documentRoot string - splitPath []string - env PreparedEnv - logger *slog.Logger - request *http.Request - originalRequest *http.Request - worker *worker + ctx context.Context + documentRoot string + splitPath []string + env PreparedEnv + logger *slog.Logger + request *http.Request + worker *worker + server *Server // idle timeout per body read; zero disables it requestBodyTimeout time.Duration @@ -45,24 +47,6 @@ type frankenPHPContext struct { startedAt time.Time } -type contextHolder struct { - ctx context.Context - frankenPHPContext *frankenPHPContext -} - -// fromContext extracts the frankenPHPContext from a context. -func fromContext(ctx context.Context) (fctx *frankenPHPContext, ok bool) { - fctx, ok = ctx.Value(contextKey).(*frankenPHPContext) - return -} - -func newFrankenPHPContext() *frankenPHPContext { - return &frankenPHPContext{ - done: make(chan any), - startedAt: time.Now(), - } -} - // NewRequestWithContext creates a new FrankenPHP request context. // // FrankenPHP does not strip request headers whose name contains an underscore. @@ -74,8 +58,23 @@ func newFrankenPHPContext() *frankenPHPContext { // you explicitly need (and whitelist) them. The Caddy-based server and reverse // proxies such as nginx (underscores_in_headers off) already do this. func NewRequestWithContext(r *http.Request, opts ...RequestOption) (*http.Request, error) { - fc := newFrankenPHPContext() - fc.request = r + c := context.WithValue(r.Context(), contextKey, opts) + + return r.WithContext(c), nil +} + +func newContextFromRequest(request *http.Request, responseWriter http.ResponseWriter, s *Server, opts ...RequestOption) (*frankenPHPContext, error) { + fc := &frankenPHPContext{ + ctx: request.Context(), + done: make(chan any), + startedAt: time.Now(), + server: s, + splitPath: s.splitPath, + logger: s.logger.Load(), + request: request, + documentRoot: s.root, + responseWriter: responseWriter, + } for _, o := range opts { if err := o(fc); err != nil { @@ -83,8 +82,14 @@ func NewRequestWithContext(r *http.Request, opts ...RequestOption) (*http.Reques } } - if fc.logger == nil { - fc.logger = globalLogger + // assign a worker directly if it has a request matcher + if fc.worker == nil { + for _, w := range s.workersWithRequestMatcher { + if w.matchRequest(request) { + fc.worker = w + break + } + } } if fc.documentRoot == "" { @@ -98,32 +103,75 @@ func NewRequestWithContext(r *http.Request, opts ...RequestOption) (*http.Reques } } - splitCgiPath(fc) - - fc.requestURI = r.URL.RequestURI() + // if no originalRequest was passed, use the URI from the actual request + // when using Caddy's http module, the original unchanged uri will be used here + // request.URL is often already rewritten to match a PHP script path + if fc.requestURI == "" { + fc.requestURI = fc.request.URL.RequestURI() + } - c := context.WithValue(r.Context(), contextKey, fc) + splitCgiPath(fc) - return r.WithContext(c), nil + return fc, nil } -// newDummyContext creates a fake context from a request path -func newDummyContext(requestPath string, opts ...RequestOption) (*frankenPHPContext, error) { - r, err := http.NewRequestWithContext(globalCtx, http.MethodGet, requestPath, nil) +// newWorkerDummyContext creates a context for worker startup +func newWorkerDummyContext(w *worker) (*frankenPHPContext, error) { + r, err := http.NewRequestWithContext(globalCtx, http.MethodGet, filepath.Base(w.fileName), nil) if err != nil { return nil, err } - fr, err := NewRequestWithContext(r, opts...) - if err != nil { - return nil, err + fc := &frankenPHPContext{ + done: make(chan any), + ctx: r.Context(), + server: w.server, + request: r, + startedAt: time.Now(), + logger: globalLogger, + worker: w, } - fc, _ := fromContext(fr.Context()) + for _, o := range w.requestOptions { + if err := o(fc); err != nil { + return nil, err + } + } + + if fc.server == nil { + // global worker, not associated with a server + fc.server = fallbackServer + } + + splitCgiPath(fc) return fc, nil } +// newContextFromMessage creates a context from a message (external workers) +func newContextFromMessage(message any, rw http.ResponseWriter, ctx context.Context, w *worker) *frankenPHPContext { + fc := &frankenPHPContext{ + done: make(chan any), + startedAt: time.Now(), + server: w.server, + worker: w, + logger: globalLogger, + responseWriter: rw, + handlerParameters: message, + ctx: ctx, + } + + if fc.server == nil { + fc.server = fallbackServer + } + + if fc.ctx == nil { + fc.ctx = globalCtx + } + + return fc +} + // closeContext sends the response to the client func (fc *frankenPHPContext) closeContext() { if fc.isDone { @@ -158,11 +206,11 @@ func (fc *frankenPHPContext) validate() error { func (fc *frankenPHPContext) clientHasClosed() bool { if fc.request == nil { - return false + return false // not in HTTP context } select { - case <-fc.request.Context().Done(): + case <-fc.ctx.Done(): return true default: return false diff --git a/debugstate.go b/debugstate.go index 62f3f9f7a0..ea9f3e364f 100644 --- a/debugstate.go +++ b/debugstate.go @@ -80,13 +80,8 @@ func threadDebugState(thread *phpThread) ThreadDebugState { return s } - if fc.originalRequest == nil { - s.CurrentURI = fc.requestURI - s.CurrentMethod = fc.request.Method - } else { - s.CurrentURI = fc.originalRequest.URL.RequestURI() - s.CurrentMethod = fc.originalRequest.Method - } + s.CurrentURI = fc.requestURI + s.CurrentMethod = fc.request.Method if !fc.startedAt.IsZero() { s.RequestStartedAt = fc.startedAt.UnixMilli() diff --git a/docs/config.md b/docs/config.md index 9f5e1b19e2..281f05dc75 100644 --- a/docs/config.md +++ b/docs/config.md @@ -187,13 +187,14 @@ php_server [] { root # Sets the root folder to the site. Default: `root` directive. split_path # Sets the substrings for splitting the URI into two parts. The first matching substring will be used to split the "path info" from the path. The first piece is suffixed with the matching substring and will be assumed as the actual resource (CGI script) name. The second piece will be set to PATH_INFO for the script to use. Default: `.php` resolve_root_symlink false # Disables resolving the `root` directory to its actual value by evaluating a symbolic link, if one exists (enabled by default). + name # Sets the name for this server, used to attribute workers, metrics and logs. Default: the first host matcher of the enclosing route, or the first listener address. env # Sets an extra environment variable to the given value. Can be specified more than once for multiple environment variables. file_server off # Disables the built-in file_server directive. request_body_timeout # Sets an idle timeout on request body reads: a stalled (slow POST) client is cut off while a steady upload of any size succeeds. Default: 60s. Set to 0 to disable. worker { # Creates a worker specific to this server. Can be specified more than once for multiple workers. file # Sets the path to the worker script, can be relative to the php_server root num # Sets the number of PHP threads to start, defaults to 2x the number of available - name # Sets the name for the worker, used in logs and metrics. Default: absolute path of worker file. Always starts with m# when defined in a php_server block. + name # Sets the name for the worker, used in logs and metrics. Default: absolute path of worker file. Postfixed with a number if name is already in use. watch # Sets the path to watch for file changes. Can be specified more than once for multiple paths. env # Sets an extra environment variable to the given value. Can be specified more than once for multiple environment variables. Environment variables for this worker are also inherited from the php_server parent, but can be overwritten here. match # match the worker to a path pattern. Overrides try_files and can only be used in the php_server directive. diff --git a/docs/library.md b/docs/library.md new file mode 100644 index 0000000000..a78b2ff8c9 --- /dev/null +++ b/docs/library.md @@ -0,0 +1,98 @@ +--- +title: Use FrankenPHP as a Go library +description: Embed PHP in any Go program with the FrankenPHP library, serve PHP scripts through net/http, and scope workers to server instances. +--- + +# Use FrankenPHP as a Go library + +FrankenPHP is not only a Caddy module: it can be embedded as a library in any Go program to execute PHP scripts with `net/http`. + +The compilation requirements are the same as for [building FrankenPHP from source](compile.md): a PHP built with the embed SAPI and ZTS enabled, and the matching CGO flags. + +## Getting started + +Create a `Server`, register it while initializing FrankenPHP, and use it as an `http.Handler`: + +```go +// Minimal FrankenPHP library usage +package main + +import ( + "log" + "net/http" + + "github.com/dunglas/frankenphp" +) + +func main() { + server, err := frankenphp.NewServer("", "public/", nil, nil, nil) + if err != nil { + log.Fatal(err) + } + + if err := frankenphp.Init(frankenphp.WithServer(server)); err != nil { + log.Fatal(err) + } + defer frankenphp.Shutdown() + + http.HandleFunc("/", func(w http.ResponseWriter, r *http.Request) { + if err := server.ServeHTTP(w, r); err != nil { + http.Error(w, err.Error(), http.StatusInternalServerError) + } + }) + log.Fatal(http.ListenAndServe(":8080", nil)) +} +``` + +`NewServer()` takes a human-readable name used to attribute workers, metrics and logs to the server (defaults to `server_` at registration when empty), the document root, the split path suffixes (defaults to `[".php"]`), environment variables made available to every request, and a `*slog.Logger` (defaults to the global logger). + +`Init()` starts the PHP runtime and must be called exactly once before serving requests; `Shutdown()` stops it. Calling `Server.ServeHTTP()` before `Init()` or after `Shutdown()` returns `ErrNotRunning`. + +## Multiple servers + +Several servers can be registered at once, each with its own document root, environment and logger. This mirrors what multiple `php_server` blocks do in a Caddyfile: + +```go +// Registering two servers with separate document roots +api, _ := frankenphp.NewServer("api", "api/public/", nil, nil, nil) +admin, _ := frankenphp.NewServer("admin", "admin/public/", nil, nil, nil) + +err := frankenphp.Init( + frankenphp.WithServer(api), + frankenphp.WithServer(admin), +) +``` + +Requests served through `api.ServeHTTP()` only see the configuration (and, see below, the workers) of that server. + +## Workers + +[Worker scripts](worker.md) are declared with `WithWorkers()`. A worker can be scoped to a server with `WithWorkerServerScope()`: only requests handled by this server instance will reach the worker. Requests are matched by script path, or by a custom matcher registered with `WithWorkerMatcher()`: + +```go +// Scoping workers to a server +server, _ := frankenphp.NewServer("", "public/", nil, nil, nil) + +err := frankenphp.Init( + frankenphp.WithServer(server), + frankenphp.WithWorkers("app", "public/index.php", 4, + frankenphp.WithWorkerServerScope(server), + ), + frankenphp.WithWorkers("api", "public/api.php", 2, + frankenphp.WithWorkerServerScope(server), + frankenphp.WithWorkerMatcher(func(r *http.Request) bool { + return strings.HasPrefix(r.URL.Path, "/api/") + }), + ), +) +``` + +Workers declared without a server scope are global: they match by file path on any server. + +## Per-request options + +`Server.ServeHTTP()` accepts `RequestOption`s to override the server configuration for a single request, e.g. `WithRequestDocumentRoot()`, `WithRequestSplitPath()`, `WithRequestEnv()` or `WithRequestLogger()`. + +## Compatibility with the pre-Server API + +The package-level `frankenphp.ServeHTTP()` function keeps working without registering any server: requests prepared with `frankenphp.NewRequestWithContext()` are executed on an internal fallback server carrying the global configuration. New code should prefer explicit `Server` instances. diff --git a/frankenphp.go b/frankenphp.go index ad2dedc42a..8b2a70efb8 100644 --- a/frankenphp.go +++ b/frankenphp.go @@ -37,7 +37,7 @@ import ( "time" "unsafe" // debug on Linux - //_ "github.com/ianlancetaylor/cgosymbolizer" + // _ "github.com/ianlancetaylor/cgosymbolizer" ) type contextKeyStruct struct{} @@ -48,7 +48,7 @@ var ( ErrInvalidPHPVersion = errors.New("FrankenPHP is only compatible with PHP 8.2+") ErrMainThreadCreation = errors.New("error creating the main thread") ErrScriptExecution = errors.New("error during PHP script execution") - ErrNotRunning = errors.New("FrankenPHP is not running. For proper configuration visit: https://frankenphp.dev/docs/config/#caddyfile-config") + ErrNotRunning = errors.New("server is not registered, you must first call frankenphp.Init() with the WithServer() option") ErrInvalidRequestPath = ErrRejected{"invalid request path", http.StatusBadRequest} ErrInvalidContentLengthHeader = ErrRejected{"invalid Content-Length header", http.StatusBadRequest} @@ -57,7 +57,7 @@ var ( contextKey = contextKeyStruct{} serverHeader = []string{"FrankenPHP"} - isRunning bool + isRunning atomic.Bool onServerShutdown []func() // Set default values to make Shutdown() idempotent @@ -240,10 +240,9 @@ func calculateMaxThreads(opt *opt) (numWorkers int, _ error) { // Init starts the PHP runtime and the configured workers. func Init(options ...Option) error { - if isRunning { + if !isRunning.CompareAndSwap(false, true) { return ErrAlreadyStarted } - isRunning = true // Ignore all SIGPIPE signals to prevent weird issues with systemd: https://github.com/php/frankenphp/issues/1020 // Docker/Moby has a similar hack: https://github.com/moby/moby/blob/d828b032a87606ae34267e349bf7f7ccb1f6495a/cmd/dockerd/docker.go#L87-L90 @@ -284,6 +283,8 @@ func Init(options ...Option) error { maxIdleTime = opt.maxIdleTime } + registerServers(opt.servers) + workerThreadCount, err := calculateMaxThreads(opt) if err != nil { Shutdown() @@ -321,7 +322,7 @@ func Init(options ...Option) error { // reused across reloads so queued requests aren't orphaned on a stale channel if regularRequestChan == nil { - regularRequestChan = make(chan contextHolder) + regularRequestChan = make(chan *frankenPHPContext) } regularThreads = make([]*phpThread, 0, opt.numThreads-workerThreadCount) for i := 0; i < opt.numThreads-workerThreadCount; i++ { @@ -365,7 +366,7 @@ func Init(options ...Option) error { // Shutdown stops the workers and the PHP runtime. func Shutdown() { - if !isRunning { + if !isRunning.Load() { return } @@ -376,6 +377,7 @@ func Shutdown() { drainWatchers() drainPHPThreads() + unregisterServers() metrics.Shutdown() @@ -384,7 +386,7 @@ func Shutdown() { _ = os.RemoveAll(EmbeddedAppPath) } - isRunning = false + isRunning.Store(false) if globalLogger.Enabled(globalCtx, slog.LevelDebug) { globalLogger.LogAttrs(globalCtx, slog.LevelDebug, "FrankenPHP shut down") } @@ -394,43 +396,20 @@ func Shutdown() { // ServeHTTP executes a PHP script according to the given context. func ServeHTTP(responseWriter http.ResponseWriter, request *http.Request) error { - h := responseWriter.Header() - if h["Server"] == nil { - h["Server"] = serverHeader - } - - if !isRunning { - return ErrNotRunning - } - ctx := request.Context() - fc, ok := fromContext(ctx) - - ch := contextHolder{ctx, fc} + opts, ok := ctx.Value(contextKey).([]RequestOption) if !ok { return ErrInvalidRequest } - fc.responseWriter = responseWriter - - if err := fc.validate(); err != nil { - return err - } - - // Detect if a worker is available to handle this request - if fc.worker != nil { - return fc.worker.handleRequest(ch) - } - - // If no worker was available, send the request to non-worker threads - return handleRequestWithRegularPHPThreads(ch) + return fallbackServer.ServeHTTP(responseWriter, request, opts...) } //export go_ub_write func go_ub_write(threadIndex C.uintptr_t, cBuf *C.char, length C.size_t) (C.size_t, C.bool) { thread := phpThreads[threadIndex] - fc := thread.frankenPHPContext() + fc := thread.handler.frankenPHPContext() if fc.isDone { return 0, C.bool(true) @@ -445,27 +424,14 @@ func go_ub_write(threadIndex C.uintptr_t, cBuf *C.char, length C.size_t) (C.size writer = fc.responseWriter } - var ctx context.Context - i, e := writer.Write(unsafe.Slice((*byte)(unsafe.Pointer(cBuf)), length)) - if e != nil { - ctx = thread.context() - - if fc.logger.Enabled(ctx, slog.LevelWarn) { - fc.logger.LogAttrs(ctx, slog.LevelWarn, "write error", slog.Any("error", e)) - } + if e != nil && fc.logger.Enabled(fc.ctx, slog.LevelWarn) { + fc.logger.LogAttrs(fc.ctx, slog.LevelWarn, "write error", slog.Any("error", e)) } - if fc.responseWriter == nil { + if fc.responseWriter == nil && fc.logger.Enabled(fc.ctx, slog.LevelInfo) { // probably starting a worker script, log the output - - if ctx == nil { - ctx = thread.context() - } - - if fc.logger.Enabled(ctx, slog.LevelInfo) { - fc.logger.LogAttrs(ctx, slog.LevelInfo, writer.(*bytes.Buffer).String()) - } + fc.logger.LogAttrs(fc.ctx, slog.LevelInfo, writer.(*bytes.Buffer).String()) } return C.size_t(i), C.bool(fc.clientHasClosed()) @@ -474,14 +440,13 @@ func go_ub_write(threadIndex C.uintptr_t, cBuf *C.char, length C.size_t) (C.size //export go_apache_request_headers func go_apache_request_headers(threadIndex C.uintptr_t) (*C.go_string, C.size_t) { thread := phpThreads[threadIndex] - ctx := thread.context() - fc := thread.frankenPHPContext() + fc := thread.handler.frankenPHPContext() if fc.responseWriter == nil { // worker mode, not handling a request - if globalLogger.Enabled(ctx, slog.LevelDebug) { - globalLogger.LogAttrs(ctx, slog.LevelDebug, "apache_request_headers() called in non-HTTP context", slog.String("worker", fc.worker.name)) + if fc.logger.Enabled(fc.ctx, slog.LevelDebug) { + fc.logger.LogAttrs(fc.ctx, slog.LevelDebug, "apache_request_headers() called in non-HTTP context", slog.String("worker", fc.worker.name)) } return nil, 0 @@ -510,11 +475,11 @@ func go_apache_request_headers(threadIndex C.uintptr_t) (*C.go_string, C.size_t) return sd, C.size_t(len(fc.request.Header)) } -func addHeader(ctx context.Context, fc *frankenPHPContext, h *C.sapi_header_struct) { +func addHeader(fc *frankenPHPContext, h *C.sapi_header_struct) { key, val := splitRawHeader(h.header, int(h.header_len)) if key == "" { - if fc.logger.Enabled(ctx, slog.LevelDebug) { - fc.logger.LogAttrs(ctx, slog.LevelDebug, "invalid header", slog.String("header", C.GoStringN(h.header, C.int(h.header_len)))) + if fc.logger.Enabled(fc.ctx, slog.LevelDebug) { + fc.logger.LogAttrs(fc.ctx, slog.LevelDebug, "invalid header", slog.String("header", C.GoStringN(h.header, C.int(h.header_len)))) } return @@ -556,7 +521,7 @@ func splitRawHeader(rawHeader *C.char, length int) (string, string) { //export go_write_headers func go_write_headers(threadIndex C.uintptr_t, status C.int, headers *C.zend_llist) C.bool { thread := phpThreads[threadIndex] - fc := thread.frankenPHPContext() + fc := thread.handler.frankenPHPContext() if fc == nil { return C.bool(false) } @@ -574,7 +539,7 @@ func go_write_headers(threadIndex C.uintptr_t, status C.int, headers *C.zend_lli for current != nil { h := (*C.sapi_header_struct)(unsafe.Pointer(&(current.data))) - addHeader(thread.context(), fc, h) + addHeader(fc, h) current = current.next } @@ -583,10 +548,9 @@ func go_write_headers(threadIndex C.uintptr_t, status C.int, headers *C.zend_lli // go panics on invalid status code // https://github.com/golang/go/blob/9b8742f2e79438b9442afa4c0a0139d3937ea33f/src/net/http/server.go#L1162 if goStatus < 100 || goStatus > 999 { - ctx := thread.context() - if globalLogger.Enabled(ctx, slog.LevelWarn) { - globalLogger.LogAttrs(ctx, slog.LevelWarn, "Invalid response status code", slog.Int("status_code", goStatus)) + if fc.logger.Enabled(fc.ctx, slog.LevelWarn) { + fc.logger.LogAttrs(fc.ctx, slog.LevelWarn, "Invalid response status code", slog.Int("status_code", goStatus)) } goStatus = 500 @@ -608,7 +572,7 @@ func go_write_headers(threadIndex C.uintptr_t, status C.int, headers *C.zend_lli //export go_sapi_flush func go_sapi_flush(threadIndex C.uintptr_t) bool { thread := phpThreads[threadIndex] - fc := thread.frankenPHPContext() + fc := thread.handler.frankenPHPContext() if fc == nil { return false } @@ -625,10 +589,8 @@ func go_sapi_flush(threadIndex C.uintptr_t) bool { fc.responseController = http.NewResponseController(fc.responseWriter) } if err := fc.responseController.Flush(); err != nil { - ctx := thread.context() - - if globalLogger.Enabled(ctx, slog.LevelWarn) { - globalLogger.LogAttrs(ctx, slog.LevelWarn, "the current responseWriter is not a flusher, if you are not using a custom build, please report this issue", slog.Any("error", err)) + if fc.logger.Enabled(fc.ctx, slog.LevelWarn) { + fc.logger.LogAttrs(fc.ctx, slog.LevelWarn, "the current responseWriter is not a flusher, if you are not using a custom build, please report this issue", slog.Any("error", err)) } } @@ -637,7 +599,7 @@ 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) { - fc := phpThreads[threadIndex].frankenPHPContext() + fc := phpThreads[threadIndex].handler.frankenPHPContext() if fc.responseWriter == nil { return 0 @@ -678,7 +640,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 { - request := phpThreads[threadIndex].frankenPHPContext().request + request := phpThreads[threadIndex].handler.frankenPHPContext().request if request == nil { return nil } @@ -695,23 +657,24 @@ func go_read_cookies(threadIndex C.uintptr_t) *C.char { return C.CString(cookie) } +// getLogger returns the logger and context safely even if phpThreads have not been created yet func getLogger(threadIndex C.uintptr_t) (*slog.Logger, context.Context) { - ctxHolder := phpThreads[threadIndex] - if ctxHolder == nil { + if threadIndex >= C.uintptr_t(len(phpThreads)) { return globalLogger, globalCtx } - ctx := ctxHolder.context() - if ctxHolder.handler == nil { - return globalLogger, ctx + thread := phpThreads[threadIndex] + if thread == nil || thread.handler == nil { + return globalLogger, globalCtx } - fCtx := ctxHolder.frankenPHPContext() - if fCtx == nil || fCtx.logger == nil { - return globalLogger, ctx + fc := thread.handler.frankenPHPContext() + if fc == nil { + return globalLogger, globalCtx } - return fCtx.logger, ctx + // logger and context must always be defined on fc + return fc.logger, fc.ctx } //export go_log @@ -779,7 +742,7 @@ func mapToAttr(input map[string]any) []slog.Attr { //export go_is_context_done func go_is_context_done(threadIndex C.uintptr_t) C.bool { - return C.bool(phpThreads[threadIndex].frankenPHPContext().isDone) + return C.bool(phpThreads[threadIndex].handler.frankenPHPContext().isDone) } //export go_schedule_opcache_reset @@ -818,7 +781,7 @@ func resetGlobals() { globalLogger = slog.Default() workers = nil workersByName = nil - workersByPath = nil + globalWorkersByPath = nil watcherIsEnabled = false maxIdleTime = defaultMaxIdleTime maxRequestsPerThread = 0 diff --git a/frankenphp_test.go b/frankenphp_test.go index 409e644634..f9bee9e94e 100644 --- a/frankenphp_test.go +++ b/frankenphp_test.go @@ -37,6 +37,8 @@ import ( "github.com/stretchr/testify/require" ) +var testDataDir = "" + type testOptions struct { workerScript string watch []string @@ -148,6 +150,9 @@ func TestMain(m *testing.M) { os.Exit(1) } + cwd, _ := os.Getwd() + testDataDir = cwd + strings.Clone("/testdata/") + os.Exit(m.Run()) } @@ -230,8 +235,6 @@ func TestPathInfo_worker(t *testing.T) { testPathInfo(t, &testOptions{workerScript: "server-variable.php"}) } func testPathInfo(t *testing.T, opts *testOptions) { - cwd, _ := os.Getwd() - testDataDir := cwd + strings.Clone("/testdata/") path := strings.Clone("/server-variable.php/pathinfo") runTest(t, func(_ func(http.ResponseWriter, *http.Request), _ *httptest.Server, i int) { diff --git a/llms.txt b/llms.txt index adef1048ee..e0989d6bad 100644 --- a/llms.txt +++ b/llms.txt @@ -25,6 +25,7 @@ This index points AI agents and crawlers at the canonical English documentation. - [FrankenPHP Worker Mode](https://frankenphp.dev/docs/worker/): supervisor configuration, restart policies, and integration with Symfony Runtime and Laravel Octane. - [Extension Workers](https://frankenphp.dev/docs/extension-workers/): dedicated PHP thread pool from a Go extension for queues, schedulers, and event listeners. - [Writing PHP Extensions in Go](https://frankenphp.dev/docs/extensions/): extension generator, types API, calling PHP from Go, classes, methods, and constants. +- [Use as a Go library](https://frankenphp.dev/docs/library/): embed PHP in any Go program, serve scripts through net/http, and scope workers to server instances. ## Frameworks diff --git a/mercure.go b/mercure.go index d7cf33609e..821b057915 100644 --- a/mercure.go +++ b/mercure.go @@ -20,12 +20,11 @@ type mercureContext struct { //export go_mercure_publish func go_mercure_publish(threadIndex C.uintptr_t, topics *C.struct__zval_struct, data *C.zend_string, private bool, id, typ *C.zend_string, retry uint64) (generatedID *C.zend_string, error C.short) { thread := phpThreads[threadIndex] - ctx := thread.context() - fc := thread.frankenPHPContext() + fc := thread.handler.frankenPHPContext() if fc.mercureHub == nil { - if fc.logger.Enabled(ctx, slog.LevelError) { - fc.logger.LogAttrs(ctx, slog.LevelError, "No Mercure hub configured") + if fc.logger.Enabled(fc.ctx, slog.LevelError) { + fc.logger.LogAttrs(fc.ctx, slog.LevelError, "No Mercure hub configured") } return nil, 1 @@ -39,7 +38,7 @@ func go_mercure_publish(threadIndex C.uintptr_t, topics *C.struct__zval_struct, Type: GoString(unsafe.Pointer(typ)), }, Private: private, - Debug: fc.logger.Enabled(ctx, slog.LevelDebug), + Debug: fc.logger.Enabled(fc.ctx, slog.LevelDebug), } zvalType := C.zval_get_type(topics) @@ -49,8 +48,8 @@ func go_mercure_publish(threadIndex C.uintptr_t, topics *C.struct__zval_struct, case C.IS_ARRAY: ts, err := GoPackedArray[string](unsafe.Pointer(*(**C.zend_array)(unsafe.Pointer(&topics.value[0])))) if err != nil { - if fc.logger.Enabled(ctx, slog.LevelError) { - fc.logger.LogAttrs(ctx, slog.LevelError, "invalid topics type", slog.Any("error", err)) + if fc.logger.Enabled(fc.ctx, slog.LevelError) { + fc.logger.LogAttrs(fc.ctx, slog.LevelError, "invalid topics type", slog.Any("error", err)) } return nil, 1 @@ -62,9 +61,9 @@ func go_mercure_publish(threadIndex C.uintptr_t, topics *C.struct__zval_struct, panic("invalid topics type") } - if err := fc.mercureHub.Publish(ctx, u); err != nil { - if fc.logger.Enabled(ctx, slog.LevelError) { - fc.logger.LogAttrs(ctx, slog.LevelError, "Unable to publish Mercure update", slog.Any("error", err)) + if err := fc.mercureHub.Publish(fc.ctx, u); err != nil { + if fc.logger.Enabled(fc.ctx, slog.LevelError) { + fc.logger.LogAttrs(fc.ctx, slog.LevelError, "Unable to publish Mercure update", slog.Any("error", err)) } return nil, 2 diff --git a/options.go b/options.go index a9cd2a2630..6a66d85dc4 100644 --- a/options.go +++ b/options.go @@ -4,6 +4,7 @@ import ( "context" "fmt" "log/slog" + "net/http" "time" ) @@ -32,6 +33,7 @@ type opt struct { maxWaitTime time.Duration maxIdleTime time.Duration maxRequests int + servers []*Server } type workerOpt struct { @@ -44,12 +46,14 @@ type workerOpt struct { env PreparedEnv requestOptions []RequestOption watch []string + matchRequest func(*http.Request) bool maxConsecutiveFailures int extensionWorkers *extensionWorkers onThreadReady func(int) onThreadShutdown func(int) onServerStartup func() onServerShutdown func() + server *Server } // WithContext sets the main context to use. @@ -212,6 +216,26 @@ func WithWorkerWatchMode(watch []string) WorkerOption { } } +// WithWorkerMatcher sets a request matcher for this worker +// if the matcher returns true, the worker will be used to handle the request +// if no request matcher is set, matching happens only by path (filename == root + request path) +func WithWorkerMatcher(matcherFunc func(*http.Request) bool) WorkerOption { + return func(w *workerOpt) error { + w.matchRequest = matcherFunc + return nil + } +} + +// WithWorkerServerScope scopes the worker to a server instance. +// Only requests that are handled by the server instance will reach the worker. +func WithWorkerServerScope(s *Server) WorkerOption { + return func(w *workerOpt) error { + w.server = s + + return nil + } +} + // WithWorkerMaxFailures sets the maximum number of consecutive failures before panicking func WithWorkerMaxFailures(maxFailures int) WorkerOption { return func(w *workerOpt) error { @@ -265,3 +289,13 @@ func withExtensionWorkers(w *extensionWorkers) WorkerOption { return nil } } + +// WithServer starts FrankenPHP with the given Server instance. +// After registering, it will be possible to call Server.ServeHTTP() +func WithServer(s *Server) Option { + return func(o *opt) error { + o.servers = append(o.servers, s) + + return nil + } +} diff --git a/phpmainthread_test.go b/phpmainthread_test.go index 8ee5746505..3ae65e68b9 100644 --- a/phpmainthread_test.go +++ b/phpmainthread_test.go @@ -92,7 +92,7 @@ func TestTransitionAThreadBetween2DifferentWorkers(t *testing.T) { // try all possible handler transitions // takes around 200ms and is supposed to force race conditions func TestTransitionThreadsWhileDoingRequests(t *testing.T) { - t.Cleanup(Shutdown) + setupGlobals(t) var ( isDone atomic.Bool @@ -236,8 +236,10 @@ func TestFinishBootingAWorkerScript(t *testing.T) { convertToWorkerThread(phpThreads[0], worker) phpThreads[0].state.WaitFor(state.Ready) - assert.NotNil(t, phpThreads[0].handler.(*workerThread).dummyContext) - assert.Nil(t, phpThreads[0].handler.(*workerThread).workerContext) + dummyFC := phpThreads[0].handler.(*workerThread).dummyFrankenPHPContext + assert.NotNil(t, dummyFC) + assert.NotNil(t, dummyFC.ctx) + assert.Nil(t, phpThreads[0].handler.(*workerThread).workerFrankenPHPContext) assert.False( t, phpThreads[0].handler.(*workerThread).isBootingScript, @@ -249,27 +251,27 @@ func TestFinishBootingAWorkerScript(t *testing.T) { } func TestReturnAnErrorIf2WorkersHaveTheSameFileName(t *testing.T) { + resetGlobals() workers = []*worker{} workersByName = map[string]*worker{} - workersByPath = map[string]*worker{} + globalWorkersByPath = map[string]*worker{} w, err1 := newWorker(workerOpt{fileName: testDataPath + "/index.php"}) assert.NoError(t, err1) workers = append(workers, w) workersByName[w.name] = w - workersByPath[w.fileName] = w + globalWorkersByPath[w.fileName] = w _, err2 := newWorker(workerOpt{fileName: testDataPath + "/index.php"}) assert.Error(t, err2, "two workers cannot have the same filename") } func TestReturnAnErrorIf2ModuleWorkersHaveTheSameName(t *testing.T) { + resetGlobals() workers = []*worker{} workersByName = map[string]*worker{} - workersByPath = map[string]*worker{} w, err1 := newWorker(workerOpt{fileName: testDataPath + "/index.php", name: "workername"}) assert.NoError(t, err1) workers = append(workers, w) workersByName[w.name] = w - workersByPath[w.fileName] = w _, err2 := newWorker(workerOpt{fileName: testDataPath + "/hello.php", name: "workername"}) assert.Error(t, err2, "two workers cannot have the same name") } @@ -313,9 +315,9 @@ func allPossibleTransitions(worker1Path string, worker2Path string) []func(*phpT thread.boot() } }, - func(thread *phpThread) { convertToWorkerThread(thread, workersByPath[worker1Path]) }, + func(thread *phpThread) { convertToWorkerThread(thread, globalWorkersByPath[worker1Path]) }, convertToInactiveThread, - func(thread *phpThread) { convertToWorkerThread(thread, workersByPath[worker2Path]) }, + func(thread *phpThread) { convertToWorkerThread(thread, globalWorkersByPath[worker2Path]) }, convertToInactiveThread, } } @@ -381,3 +383,22 @@ func testThreadCalculationError(t *testing.T, o *opt) { _, err := calculateMaxThreads(o) assert.Error(t, err, "configuration must error") } + +func TestContextAndLoggerMustNotBeNil(t *testing.T) { + log, ctx := getLogger(0) + assert.NotNil(t, log, "logger is defined if all threads are inactive") + assert.NotNil(t, ctx, "context is defined if all threads are inactive") + + fc := newContextFromMessage(nil, nil, nil, &worker{}) + assert.NotNil(t, fc.logger, "logger is defined for message context") + assert.NotNil(t, fc.ctx, "context is defined for message context") + + r := httptest.NewRequest("GET", "http://localhost/index.php", nil) + fc, _ = newContextFromRequest(r, nil, fallbackServer) + assert.NotNil(t, fc.logger, "logger is defined for request context") + assert.NotNil(t, fc.ctx, "context is defined for request context") + + fc, _ = newWorkerDummyContext(&worker{}) + assert.NotNil(t, fc.logger, "logger is defined for worker dummy context") + assert.NotNil(t, fc.ctx, "context is defined for worker dummy context") +} diff --git a/phpthread.go b/phpthread.go index 106ab2101b..39ee24b5d4 100644 --- a/phpthread.go +++ b/phpthread.go @@ -4,7 +4,6 @@ package frankenphp // #include "frankenphp.h" import "C" import ( - "context" "log/slog" "runtime" "sync" @@ -19,7 +18,7 @@ import ( type phpThread struct { runtime.Pinner threadIndex int - requestChan chan contextHolder + requestChan chan *frankenPHPContext drainChan chan struct{} handlerMu sync.RWMutex handler threadHandler @@ -39,7 +38,6 @@ type threadHandler interface { name() string beforeScriptExecution() string afterScriptExecution(exitStatus int) - context() context.Context frankenPHPContext() *frankenPHPContext // drain is a hook called by drainWorkerThreads right before drainChan is // closed. Handlers that need to wake up a thread parked in a blocking C @@ -52,7 +50,7 @@ type threadHandler interface { func newPHPThread(threadIndex int) *phpThread { return &phpThread{ threadIndex: threadIndex, - requestChan: make(chan contextHolder), + requestChan: make(chan *frankenPHPContext), state: state.NewThreadState(), } } @@ -177,19 +175,6 @@ func (thread *phpThread) transitionToNewHandler() string { return thread.handler.beforeScriptExecution() } -func (thread *phpThread) frankenPHPContext() *frankenPHPContext { - return thread.handler.frankenPHPContext() -} - -func (thread *phpThread) context() context.Context { - if thread.handler == nil { - // handler can be nil when using opcache.preload - return globalCtx - } - - return thread.handler.context() -} - func (thread *phpThread) name() string { thread.handlerMu.RLock() defer thread.handlerMu.RUnlock() diff --git a/requestoptions.go b/requestoptions.go index e6df428945..8414fb557d 100644 --- a/requestoptions.go +++ b/requestoptions.go @@ -84,6 +84,19 @@ 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, error) { + if err := normalizeSplitPath(splitPath); err != nil { + return nil, err + } + + return func(o *frankenPHPContext) error { + o.splitPath = splitPath + + return nil + }, nil +} + +// normalize split path in-place to lowercase ASCII characters +func normalizeSplitPath(splitPath []string) error { var b strings.Builder for i, split := range splitPath { @@ -92,7 +105,7 @@ func WithRequestSplitPath(splitPath []string) (RequestOption, error) { for j := 0; j < len(split); j++ { c := split[j] if c >= utf8.RuneSelf { - return nil, ErrInvalidSplitPath + return ErrInvalidSplitPath } if 'A' <= c && c <= 'Z' { @@ -106,11 +119,7 @@ func WithRequestSplitPath(splitPath []string) (RequestOption, error) { b.Reset() } - return func(o *frankenPHPContext) error { - o.splitPath = splitPath - - return nil - }, nil + return nil } type PreparedEnv = map[string]string @@ -165,7 +174,7 @@ func ensurePreparedEnv(env PreparedEnv) PreparedEnv { func WithOriginalRequest(r *http.Request) RequestOption { return func(o *frankenPHPContext) error { - o.originalRequest = r + o.requestURI = r.URL.RequestURI() return nil } @@ -176,6 +185,10 @@ func WithRequestLogger(logger *slog.Logger) RequestOption { return func(o *frankenPHPContext) error { o.logger = logger + if o.logger == nil { + o.logger = globalLogger // fall back to global logger + } + return nil } } diff --git a/scaling.go b/scaling.go index 82f2b6c2d7..e4edc0c846 100644 --- a/scaling.go +++ b/scaling.go @@ -35,15 +35,15 @@ var ( ) func initAutoScaling(mainThread *phpMainThread) { + if mainThread.maxThreads <= mainThread.numThreads { + return + } + // reused across reloads so queued requests aren't orphaned on a stale channel if scaleChan == nil { scaleChan = make(chan *frankenPHPContext) } - if mainThread.maxThreads <= mainThread.numThreads { - return - } - done := mainThread.done mstate := mainThread.state diff --git a/scaling_test.go b/scaling_test.go index f899e7679e..d2784a8eeb 100644 --- a/scaling_test.go +++ b/scaling_test.go @@ -48,7 +48,7 @@ func TestScaleAWorkerThreadUpAndDown(t *testing.T) { autoScaledThread := phpThreads[2] // scale up - scaleWorkerThread(workersByPath[workerPath], mainThread.done, mainThread.state) + scaleWorkerThread(globalWorkersByPath[workerPath], mainThread.done, mainThread.state) assert.Equal(t, state.Ready, autoScaledThread.state.Get()) // on down-scale, the thread will be marked as inactive diff --git a/server.go b/server.go new file mode 100644 index 0000000000..84f7acb66f --- /dev/null +++ b/server.go @@ -0,0 +1,160 @@ +package frankenphp + +import ( + "fmt" + "log/slog" + "net/http" + "strconv" + "sync/atomic" + + "github.com/dunglas/frankenphp/internal/fastabs" +) + +// Server represents a preconfigured server block +// requests and workers can be scoped to a Server +type Server struct { + idx int + name string + root string + splitPath []string + env PreparedEnv + workers []*worker + workersByPath map[string]*worker + workersWithRequestMatcher []*worker + workerOpts []workerOpt + + // registered while FrankenPHP runs with this server; read by concurrent + // ServeHTTP calls while Init()/Shutdown() flip it, hence atomic + isRegistered atomic.Bool + + // atomic for the same reason: the fallback server's logger is replaced + // at registration time while in-flight requests may read it + logger atomic.Pointer[slog.Logger] +} + +var ( + servers []*Server + fallbackServer = newFallbackServer() +) + +func newFallbackServer() *Server { + s := &Server{ + idx: -1, + workersByPath: make(map[string]*worker), + env: make(map[string]string), + } + s.logger.Store(globalLogger) + + return s +} + +func registerServers(newServers []*Server) { + servers = newServers + fallbackServer.logger.Store(globalLogger) + fallbackServer.isRegistered.Store(true) + for i, s := range servers { + s.idx = i + if s.name == "" { + s.name = "server_" + strconv.Itoa(i) + } + s.isRegistered.Store(true) + } +} + +func unregisterServers() { + fallbackServer.isRegistered.Store(false) + for _, server := range servers { + server.isRegistered.Store(false) + } +} + +// NewServer creates a Server that can be registered via WithServer(). +// name is a human-readable identifier used to attribute workers, metrics +// and logs to this server; when empty, it defaults to the index the +// server gets at registration time. +func NewServer(name, root string, splitPath []string, env map[string]string, logger *slog.Logger) (*Server, error) { + root, err := fastabs.FastAbs(root) + if err != nil { + return nil, err + } + + if err := normalizeSplitPath(splitPath); err != nil { + return nil, err + } + + s := &Server{ + name: name, + root: root, + splitPath: splitPath, + env: PrepareEnv(env), + workersByPath: make(map[string]*worker), + workerOpts: make([]workerOpt, 0), + } + + if logger == nil { + logger = globalLogger + } + s.logger.Store(logger) + + if len(s.splitPath) == 0 { + s.splitPath = []string{".php"} + } + + if s.env == nil { + s.env = PrepareEnv(nil) + } + + return s, nil +} + +// Name returns the human-readable name of the server. +// It is empty until registration if none was passed to NewServer(). +func (s *Server) Name() string { + return s.name +} + +func (s *Server) addWorker(w *worker) error { + s.workers = append(s.workers, w) + if w.matchRequest != nil { + s.workersWithRequestMatcher = append(s.workersWithRequestMatcher, w) + return nil + } + + if _, exists := s.workersByPath[w.fileName]; exists { + return fmt.Errorf("two workers in a server cannot have the same filename: %q", w.fileName) + } + s.workersByPath[w.fileName] = w + + return nil +} + +// ServeHTTP executes a PHP script on the registered server. +// The request will be scoped to the server instance that was registered via WithServer(). +// Otherwise, it is equivalent to calling ServeHTTP. +func (s *Server) ServeHTTP(responseWriter http.ResponseWriter, request *http.Request, opts ...RequestOption) error { + if !s.isRegistered.Load() { + return ErrNotRunning + } + + h := responseWriter.Header() + if h["Server"] == nil { + h["Server"] = serverHeader + } + + fc, err := newContextFromRequest(request, responseWriter, s, opts...) + if err != nil { + return err + } + + if err := fc.validate(); err != nil { + return err + } + + // Handle request with a worker if one is assigned + if fc.worker != nil { + return fc.worker.handleRequest(fc) + } + + // If no worker was available, send the request to non-worker threads + return handleRequestWithRegularPHPThreads(fc) +} diff --git a/server_test.go b/server_test.go new file mode 100644 index 0000000000..00db5ea199 --- /dev/null +++ b/server_test.go @@ -0,0 +1,191 @@ +package frankenphp_test + +import ( + "io" + "net/http" + "net/http/httptest" + "path/filepath" + "strings" + "testing" + + "github.com/dunglas/frankenphp" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func initServers(t *testing.T, opts ...frankenphp.Option) { + t.Helper() + t.Cleanup(frankenphp.Shutdown) + require.NoError(t, frankenphp.Init(opts...)) +} + +func serverRequest(t *testing.T, server *frankenphp.Server, req *http.Request) (string, *http.Response) { + t.Helper() + + w := httptest.NewRecorder() + require.NoError(t, server.ServeHTTP(w, req)) + + resp := w.Result() + body, err := io.ReadAll(resp.Body) + require.NoError(t, err) + + return string(body), resp +} + +func serverGet(t *testing.T, server *frankenphp.Server, url string) string { + t.Helper() + + body, _ := serverRequest(t, server, httptest.NewRequest(http.MethodGet, url, nil)) + + return body +} + +func TestServer(t *testing.T) { + t.Run("idx", func(t *testing.T) { + server1, _ := frankenphp.NewServer("", testDataDir, nil, map[string]string{"PHP_SERVER_IDX_1": "1"}, nil) + server2, _ := frankenphp.NewServer("", testDataDir, nil, map[string]string{"PHP_SERVER_IDX_2": "2"}, nil) + + initServers(t, frankenphp.WithServer(server1), frankenphp.WithServer(server2)) + + body1 := serverGet(t, server1, "http://example.com/server-variable.php") + body2 := serverGet(t, server2, "http://example.com/server-variable.php") + + assert.Contains(t, body1, "[PHP_SERVER_IDX_1] => 1") + assert.Contains(t, body2, "[PHP_SERVER_IDX_2] => 2") + assert.NotContains(t, body1, "[PHP_SERVER_IDX_2]") + assert.NotContains(t, body2, "[PHP_SERVER_IDX_1]") + }) + + t.Run("name", func(t *testing.T) { + named, _ := frankenphp.NewServer("api", testDataDir, nil, nil, nil) + unnamed, _ := frankenphp.NewServer("", testDataDir, nil, nil, nil) + + initServers(t, frankenphp.WithServer(named), frankenphp.WithServer(unnamed)) + + assert.Equal(t, "api", named.Name()) + // an empty name defaults to the server index at registration + assert.Equal(t, "server_1", unnamed.Name()) + }) + + t.Run("root", func(t *testing.T) { + server, _ := frankenphp.NewServer("", testDataDir, nil, nil, nil) + initServers(t, frankenphp.WithServer(server)) + + body := serverGet(t, server, "http://example.com/server-globals.php") + + expectedRoot := filepath.Clean(strings.TrimSuffix(testDataDir, string(filepath.Separator))) + assert.Contains(t, body, "DOCUMENT_ROOT: "+expectedRoot+"\n") + }) + + t.Run("env", func(t *testing.T) { + server, _ := frankenphp.NewServer("", testDataDir, nil, map[string]string{"TEST_123": "123"}, nil) + initServers(t, frankenphp.WithServer(server)) + + body := serverGet(t, server, "http://example.com/server-variable.php") + + assert.Contains(t, body, "[TEST_123] => 123") + }) + + t.Run("split_path", func(t *testing.T) { + server, _ := frankenphp.NewServer("", testDataDir, []string{".custom"}, nil, nil) + initServers(t, frankenphp.WithServer(server)) + + body := serverGet(t, server, "http://example.com/split-path.custom/pathinfo") + + assert.Contains(t, body, "PATH_INFO: /pathinfo\n") + assert.Contains(t, body, "SCRIPT_NAME: /split-path.custom\n") + assert.Contains(t, body, "PHP_SELF: /split-path.custom/pathinfo\n") + }) + + t.Run("workers_by_path_and_request_matcher", func(t *testing.T) { + server1, _ := frankenphp.NewServer("", testDataDir, nil, nil, nil) + server2, _ := frankenphp.NewServer("", testDataDir, nil, nil, nil) + initServers( + t, + frankenphp.WithServer(server1), + frankenphp.WithServer(server2), + frankenphp.WithWorkers("counter", testDataDir+"worker-with-counter.php", 1, frankenphp.WithWorkerServerScope(server1)), + frankenphp.WithWorkers("match", testDataDir+"worker-with-counter.php", 1, + frankenphp.WithWorkerServerScope(server2), + frankenphp.WithWorkerMatcher(func(r *http.Request) bool { + return strings.HasPrefix(r.URL.Path, "/match/") + }), + ), + ) + + body1 := serverGet(t, server1, "http://example.com/worker-with-counter.php") + body2 := serverGet(t, server1, "http://example.com/worker-with-counter.php") + body3 := serverGet(t, server2, "http://example.com/match/anything") + body4 := serverGet(t, server2, "http://example.com/match/anything") + body5 := serverGet(t, server2, "http://example.com/index.php") + body6 := serverGet(t, server1, "http://example.com/match/anything") + + assert.Equal(t, "requests:1", body1, "could not access the worker by path on server 1") + assert.Equal(t, "requests:2", body2, "could not access the worker by path on server 1") + assert.Equal(t, "requests:1", body3, "could not access the worker by matcher on server 2") + assert.Equal(t, "requests:2", body4, "could not access the worker by matcher on server 2") + assert.Contains(t, body5, "I am by birth a Genevese (i not set)", "could not access the worker by path on server 2") + assert.Contains(t, body6, "Failed opening required", "worker is scoped to server 1, so should not be found on server 2") + }) + + t.Run("worker_env_inheritance", func(t *testing.T) { + server, _ := frankenphp.NewServer("", testDataDir, nil, map[string]string{ + "FROM_SERVER_ENV": "original", + "FROM_WORKER_ENV": "overridden", + }, nil) + initServers( + t, + frankenphp.WithPhpIni(map[string]string{"variables_order": "EGPCS"}), + frankenphp.WithServer(server), + frankenphp.WithWorkers( + "env", + testDataDir+"env/env.php", + 1, + frankenphp.WithWorkerServerScope(server), + frankenphp.WithWorkerEnv(map[string]string{ + "FROM_WORKER_ENV": "original", + }), + ), + ) + + body := serverGet(t, server, "http://example.com/env/env.php?keys[]=FROM_SERVER_ENV&keys[]=FROM_WORKER_ENV") + + assert.Equal( + t, + "FROM_SERVER_ENV=original,FROM_WORKER_ENV=original", + body, + "should contain the server env and not override the worker env", + ) + }) + + t.Run("error_on_duplicate_worker_filenames", func(t *testing.T) { + t.Cleanup(frankenphp.Shutdown) + + server, _ := frankenphp.NewServer("", testDataDir, nil, nil, nil) + err := frankenphp.Init( + frankenphp.WithServer(server), + frankenphp.WithWorkers("worker1", testDataDir+"worker-with-counter.php", 1, frankenphp.WithWorkerServerScope(server)), + frankenphp.WithWorkers("worker2", testDataDir+"worker-with-counter.php", 1, frankenphp.WithWorkerServerScope(server)), + ) + + assert.Error(t, err) + assert.Contains(t, err.Error(), "two workers in a server cannot have the same filename") + }) + + t.Run("error_on_missing_registration", func(t *testing.T) { + server, _ := frankenphp.NewServer("", testDataDir, nil, nil, nil) + + assert.ErrorIs(t, server.ServeHTTP(nil, nil), frankenphp.ErrNotRunning) + }) + + t.Run("server_logger", func(t *testing.T) { + logger, buf := newTestLogger(t) + server, _ := frankenphp.NewServer("", testDataDir, nil, nil, logger) + initServers(t, frankenphp.WithServer(server)) + + _ = serverGet(t, server, "http://example.com/log-frankenphp_log.php") + _ = serverGet(t, server, "http://example.com/log-frankenphp_log.php") + + assert.Contains(t, buf.String(), "some error message") + }) +} diff --git a/testdata/split-path.custom b/testdata/split-path.custom new file mode 100644 index 0000000000..836a9a4512 --- /dev/null +++ b/testdata/split-path.custom @@ -0,0 +1,26 @@ + 0 { + for k, v := range o.server.env { + if _, exists := o.env[k]; !exists { + o.env[k] = v + } + } + } + o.env["FRANKENPHP_WORKER\x00"] = "1" + w := &worker{ name: o.name, fileName: absFileName, + matchRequest: o.matchRequest, requestOptions: o.requestOptions, num: o.num, maxThreads: o.maxThreads, - requestChan: make(chan contextHolder), + requestChan: make(chan *frankenPHPContext), threads: make([]*phpThread, 0, o.num), - allowPathMatching: allowPathMatching, maxConsecutiveFailures: o.maxConsecutiveFailures, onThreadReady: o.onThreadReady, onThreadShutdown: o.onThreadShutdown, + server: o.server, } w.configureMercure(&o) @@ -215,7 +227,7 @@ func (worker *worker) isAtThreadLimit() bool { return atMaxThreads } -func (worker *worker) handleRequest(ch contextHolder) error { +func (worker *worker) handleRequest(fc *frankenPHPContext) error { metrics.StartWorkerRequest(worker.name) runtime.Gosched() @@ -225,10 +237,10 @@ func (worker *worker) handleRequest(ch contextHolder) error { worker.threadMutex.RLock() for _, thread := range worker.threads { select { - case thread.requestChan <- ch: + case thread.requestChan <- fc: worker.threadMutex.RUnlock() - <-ch.frankenPHPContext.done - metrics.StopWorkerRequest(worker.name, time.Since(ch.frankenPHPContext.startedAt)) + <-fc.done + metrics.StopWorkerRequest(worker.name, time.Since(fc.startedAt)) return nil default: @@ -249,22 +261,22 @@ func (worker *worker) handleRequest(ch contextHolder) error { } select { - case worker.requestChan <- ch: + case worker.requestChan <- fc: worker.queuedRequests.Add(-1) metrics.DequeuedWorkerRequest(worker.name) - <-ch.frankenPHPContext.done - metrics.StopWorkerRequest(worker.name, time.Since(ch.frankenPHPContext.startedAt)) + <-fc.done + metrics.StopWorkerRequest(worker.name, time.Since(fc.startedAt)) return nil - case workerScaleChan <- ch.frankenPHPContext: + case workerScaleChan <- fc: // the request has triggered scaling, continue to wait for a thread case <-timeoutChan(time.Duration(maxWaitTime.Load())): // the request has timed out stalling worker.queuedRequests.Add(-1) metrics.DequeuedWorkerRequest(worker.name) - metrics.StopWorkerRequest(worker.name, time.Since(ch.frankenPHPContext.startedAt)) + metrics.StopWorkerRequest(worker.name, time.Since(fc.startedAt)) - ch.frankenPHPContext.reject(ErrMaxWaitTimeExceeded) + fc.reject(ErrMaxWaitTimeExceeded) return ErrMaxWaitTimeExceeded } diff --git a/workerextension.go b/workerextension.go index 82b74631a7..814eea1565 100644 --- a/workerextension.go +++ b/workerextension.go @@ -45,13 +45,8 @@ func (w *extensionWorkers) NumThreads() int { // EXPERIMENTAL: SendMessage sends a message to the worker and waits for a response. func (w *extensionWorkers) SendMessage(ctx context.Context, message any, rw http.ResponseWriter) (any, error) { - fc := newFrankenPHPContext() - fc.logger = globalLogger - fc.worker = w.internalWorker - fc.responseWriter = rw - fc.handlerParameters = message - - err := w.internalWorker.handleRequest(contextHolder{context.WithValue(ctx, contextKey, fc), fc}) + fc := newContextFromMessage(message, rw, ctx, w.internalWorker) + err := w.internalWorker.handleRequest(fc) return fc.handlerReturn, err }