Skip to content

Set SA_ONSTACK in zend_sigaction - #9597

Merged
iluuu1994 merged 2 commits into
php:masterfrom
dunglas:feat/SA_ONSTACK
Sep 26, 2022
Merged

Set SA_ONSTACK in zend_sigaction #9597
iluuu1994 merged 2 commits into
php:masterfrom
dunglas:feat/SA_ONSTACK

Conversation

@dunglas

Copy link
Copy Markdown
Member

I'm currently working on a new SAPI for web servers written in Go.

Many virtual machines, including Go VM, depend on signals using SA_ONSTACK . This flag allows a thread to define a new alternate signal stack. Many argue that SA_ONSTACK should be a default, but it's not the case (yet).

This patch sets the SA_ONSTACK flag when PHP calls sigaction().

Python merged a similar patch (python/cpython#24730) in 2021 (Python 3.10+) for the same reasons, with no issues.

@Girgias

Copy link
Copy Markdown
Member

Shouldn't this target 8.2? master is currently 8.3-dev

@dunglas

Copy link
Copy Markdown
MemberAuthor

@Girgias, @krakjoe said on internals

Master should be targeted, no RFC is required, and the patch looks fine to me.

Your call :)

@devnexen

Copy link
Copy Markdown
Member

I definitely would like to see it at least for 8.3, @dunglas is doing the right things IMHO nor I have any problem seeing also into the 8.2 branch but should not we ask one of the release managers opinion just to be safe or is it ok ?

@iluuu1994iluuu1994 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM. I think master should be fine, unless the RMs think this should target an earlier branch.

@Girgias

Copy link
Copy Markdown
Member

@saundefined@adoy opinions about landing this in 8.2 or should this be deferred to 8.3?

@adoy

adoy commented Sep 26, 2022

Copy link
Copy Markdown
Member

Unless there is a very strong need to have this change in 8.2 I would recommend to target 8.3 since we already are in an RC phase.

@iluuu1994
iluuu1994 merged commit cd572ae into php:masterSep 26, 2022
@dunglas
dunglas deleted the feat/SA_ONSTACK branch October 13, 2022 13:57
lstrojny pushed a commit to php-amqp/php-amqp that referenced this pull request Jun 4, 2024
Sets SA_ONSTACK and switches from signal to sigaction (see php/php-src#9597)
Sign up for freeto 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.

5 participants

@dunglas@Girgias@devnexen@adoy@iluuu1994