NotificationsYou must be signed in to change notification settings
Fork9.7k
Star30.8k

[Messenger] do not listen to signals if the pcntl extension is missing#50963

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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Jump to bottom

Merged

nicolas-grekas merged 1 commit intosymfony:6.3fromxabbuh:pr-49539

Jul 13, 2023

Merged

[Messenger] do not listen to signals if the pcntl extension is missing#50963

nicolas-grekas merged 1 commit intosymfony:6.3fromxabbuh:pr-49539

Jul 13, 2023

Conversation

Copy link

Member

xabbuh commentedJul 13, 2023

Q	A
Branch?	6.3
Bug fix?	yes
New feature?	no
Deprecations?	no
Tickets
License	MIT
Doc PR

carsonbot added Status: Needs Review Bug Messenger labels

Jul 13, 2023

carsonbot added this to the6.3 milestone

Jul 13, 2023

Copy link

MemberAuthor

xabbuh commentedJul 13, 2023

caused by the changes done in#49539, previously we exited early when thepcntl_signal() function was missing

OskarStark approved these changes

Jul 13, 2023

View reviewed changes

carsonbot added Status: Reviewed and removed Status: Needs Review labels

Jul 13, 2023

Copy link

Member

nicolas-grekas commentedJul 13, 2023

Could the failing integration test be related?

Symfony\Component\Messenger\Bridge\Amqp\Tests\Transport\AmqpExtIntegrationTest::testItReceivesSignals
RuntimeException: Expected output never arrived. Got "" instead.

It's not on the latest 6.3 run.

xabbuh force-pushed thepr-49539 branch from38e0fc3 to8a998efCompare

July 13, 2023 10:34

Copy link

MemberAuthor

xabbuh commentedJul 13, 2023

I am looking into the failure

Copy link

Member

lyrixx commentedJul 13, 2023

There is a competitive PR

#50787

do not listen to signals if the pcntl extension is missing

dfc90f0

xabbuh force-pushed thepr-49539 branch from78b0497 todfc90f0Compare

July 13, 2023 12:24

Copy link

MemberAuthor

xabbuh commentedJul 13, 2023

@nicolas-grekas tests fixed

@lyrixx This PR fixes an issue when this listener is registered by the FrameworkBundle 5.4, but the pcntl extension is missing. I don't see how this is being fixed by the linked PR.