Skip to content

Conversation

@ghost
Copy link

@ghost ghost commented Dec 20, 2024

This is another attempt at fixing #1269
I still couldn't reproduce the issue, but here is my best guess:

Some PHP extensions probably access environment variables when starting the main thread. In a past PR, we changed it so all environment is accessed via go
https://github.com/dunglas/frankenphp/blob/0fc6ccc5ceaee0855f2a5c24e7b064d25d589d3f/frankenphp.c#L777

So we need to make sure the phpThreads slice is initialized before starting the main thread, so it can pin to the first phpThread (threadIndex on the main thread defaults to 0)

@ghost
Copy link
Author

ghost commented Dec 20, 2024

I also changed it to explicitly check for a nil return on unsafe.StringData(s) (in case that' somehow the issue)

Copy link
Member

@dunglas dunglas left a comment

Choose a reason for hiding this comment

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

LGTM

@dunglas dunglas merged commit 92f9534 into main Dec 21, 2024
54 of 55 checks passed
@dunglas dunglas deleted the fix/env-var-sigsev branch December 21, 2024 01:38
@dunglas
Copy link
Member

dunglas commented Dec 21, 2024

Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants