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

support using SIGALARM instead of SIGPROF for Go compatibility #9738

Closed
wants to merge 1 commit into from

Conversation

dunglas
Copy link
Contributor

@dunglas dunglas commented Oct 13, 2022

Go uses SIGPROF for internal purposes and can send this signal to the C code. PHP also uses it for the timeout system. This causes issues when embedding PHP in Go programs.

For some platforms, when SIGPROF isn't available, PHP already uses SIGALARM instead. This patch adds a new compilation flag to force using SIGALARM. This fixes issues with Go.

Related: #9597, #9672.

@krakjoe
Copy link
Member

krakjoe commented Oct 13, 2022

Is it going to be normal for the PHP build that your program interacts with to be compiled with special options that make it compatible, or is it the case that your go program needs to interact with php builds it finds in the wild ?

I expect the latter is true, and that makes me question the usefulness of this patch ?

@dunglas
Copy link
Contributor Author

dunglas commented Oct 13, 2022

@krakjoe in my case, I use a custom build of PHP and I embed it in the binary produced by Go (static compilation), so special options are ok for this use case.

That being said, I think that a better fix would be to ensure that signal handlers do nothing if executed on threads not owned by PHP (as Go does for the threads it doesn't own). #9649 (comment)
I'll work on a patch as soon as possible.

@dunglas
Copy link
Contributor Author

dunglas commented Mar 4, 2023

This patch is no longer relevant, #10141 (which uses real-time signals) fixes this interoperability issue with Go.

@dunglas dunglas closed this Mar 4, 2023
@dunglas dunglas deleted the feat/no-sigprof branch March 4, 2023 10:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants