Skip to content

Refactor pcntl_sigprocmask()/pcntl_sigwaitinfo()/pcntl_sigtimedwait() #11860

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

Merged
merged 8 commits into from
Nov 16, 2023

Conversation

Girgias
Copy link
Member

@Girgias Girgias commented Aug 2, 2023

Also add some validations which throw ValueError/TypeErrors.

Besides, found a bug in which in certain cases it wouldn't return false on failure, I think this should be fixed but probably just for master/PHP 8.3?

Still need to write the UPGRADING bit

@Girgias
Copy link
Member Author

Girgias commented Aug 2, 2023

pcntl_sigprocmask() Should also get those amendments.

@Girgias Girgias force-pushed the pcntl-refactoring branch 2 times, most recently from 5122ac0 to 38f6734 Compare August 3, 2023 12:24
bool(false)
sigtimedwait with invalid arguments
Error triggered
int(-1)
Copy link
Member Author

Choose a reason for hiding this comment

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

Really this issue should have been caught way before...

@Girgias Girgias changed the title Refactor pcntl_sigwaitinfo()/pcntl_sigtimedwait() Refactor pcntl_sigprocmask()/pcntl_sigwaitinfo()/pcntl_sigtimedwait() Aug 4, 2023
@Girgias Girgias requested a review from devnexen August 4, 2023 17:25
@Girgias
Copy link
Member Author

Girgias commented Sep 6, 2023

I forgot about this PR...

@bukka this is kinda bugfixing some of the return values but is also adding Value and Type errors, is this too late to include in 8.3 considering it has hit RC phase?

Copy link
Member

@devnexen devnexen left a comment

Choose a reason for hiding this comment

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

LGTM

@bukka
Copy link
Member

bukka commented Sep 7, 2023

Yeah it is a bug fix but a BC breaking bug fix so I think it needs to go to master only. As I mentioned elsewhere I don't think we should introduce any changes to UPGRADING in RC stage. It's just too late and we want users to start migrating their applications and test PHP 8.3 on them. So we should not introduce any further BC break that might impact application migration.

@Girgias Girgias merged commit 1bdb0fd into php:master Nov 16, 2023
@Girgias Girgias deleted the pcntl-refactoring branch November 16, 2023 00:40
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.

3 participants