Skip to content
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.

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

fix: SIGSEGV with env vars #1278

Merged
merged 2 commits into from
Dec 21, 2024
Merged

fix: SIGSEGV with env vars #1278

merged 2 commits into from
Dec 21, 2024

Conversation

Alliballibaba2
Copy link
Collaborator

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

frankenphp_getenv, /* getenv */

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)

@Alliballibaba2
Copy link
Collaborator Author

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

Copy link
Owner

@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
Owner

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.

3 participants