Skip to content

fix sigsev on bind permissions denied (#2250?) #2251

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
henderkes merged 4 commits into from
Mar 9, 2026
+4 −6
Merged

fix sigsev on bind permissions denied (#2250?) #2251

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
c179a5a
fix sigsev on bind permissions denied (#2250?)
henderkes Mar 8, 2026
da038e1
don't we set the context in Provision() already?
henderkes Mar 8, 2026
5bad616
RegisterWorkers and DrainWorkers are now never referenced
henderkes Mar 8, 2026
a27a12f
can't do that here, causes deadlock
henderkes Mar 8, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 3 additions & 5 deletions caddy/app.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -176,17 +176,15 @@ func (f *FrankenPHPApp) Start() error {
}

func (f *FrankenPHPApp) Stop() error {
ctx := caddy.ActiveContext()

if f.logger.Enabled(caddy.ActiveContext(), slog.LevelInfo) {
f.logger.LogAttrs(ctx, slog.LevelInfo, "FrankenPHP stopped 🐘")
if f.logger.Enabled(f.ctx, slog.LevelInfo) {
f.logger.LogAttrs(f.ctx, slog.LevelInfo, "FrankenPHP stopped 🐘")
Comment on lines +179 to +180
Copy link
Contributor Author

@henderkes henderkes Mar 8, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am not sure if we have to refetch the context here. If we do, I'll revert this of course, but in my testing this works.

Copy link
Member

@dunglas dunglas Mar 9, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks good to me.

TBH, I would only keep this change in this PR, other changes are unrelated to the issue and maybe touchy.

}

// attempt a graceful shutdown if caddy is exiting
// note: Exiting() is currently marked as 'experimental'
// https://github.com/caddyserver/caddy/blob/e76405d55058b0a3e5ba222b44b5ef00516116aa/caddy.go#L810
if caddy.Exiting() {
frankenphp.DrainWorkers()
frankenphp.Shutdown()
}

// reset the configuration so it doesn't bleed into later tests
Expand Down
2 changes: 1 addition & 1 deletion frankenphp.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -211,7 +211,7 @@ func calculateMaxThreads(opt *opt) (numWorkers int, _ error) {
return numWorkers, nil
}

if !maxThreadsIsSet && !numThreadsIsSet {
if !numThreadsIsSet {
if numWorkers >= maxProcs {
// Start at least as many threads as workers, and keep a free thread to handle requests in non-worker mode
opt.numThreads = numWorkers + 1
Expand Down