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

[10.x] We dont want Symfony to catch pcntl signal #47725

Merged
merged 1 commit into from
Jul 12, 2023

Conversation

ChristopheBorcard
Copy link
Contributor

Simple solution to fix issue #47724

@ChristopheBorcard ChristopheBorcard changed the title We dont want Symfony to catch pcntl signal [10.x] We dont want Symfony to catch pcntl signal Jul 12, 2023
@taylorotwell
Copy link
Member

Please explain thoroughly how it fixes the issue.

@ChristopheBorcard
Copy link
Contributor Author

ChristopheBorcard commented Jul 12, 2023

When using the Symfony Event Dispatcher it will register for signal has you can see here

And PHP pcntl-signal are not cumulative so il will replace the one registered here

But if we empty the list of signal to listen by symfony pre-defined here we make sure we don't override the signal management already in place in Laravel

@taylorotwell
Copy link
Member

I dunno - this feels like it could just break even more stuff - I'm very tempted to just revert the entire Symfony Event mess from the PR that started this.

@ChristopheBorcard
Copy link
Contributor Author

I'm very tempted to just revert the entire Symfony Event mess from the PR that started this.

This is the solution we've temporarily applied to correct the problem.

But I wanted to propose a solution without going back. But it seems riskier too.

@taylorotwell taylorotwell merged commit 3cd9bcb into laravel:10.x Jul 12, 2023
16 of 17 checks passed
@func0der
Copy link

func0der commented Jan 10, 2024

I dunno - this feels like it could just break even more stuff - I'm very tempted to just revert the entire Symfony Event mess from the PR that started this.

I think you were on the right track with this, @taylorotwell
From what I have found spending half my day trying to debug job workers, it is a problem in combination with Docker, IDEs and queue:listen.
In my example it is PHPStorm, but I figure it would be any other IDE, too.

Steps that should reproduce this:

I just a assume a working queue configuration here.

  1. Configure your IDE with a php remote interpreter from a docker container. It should use run, not exec.
  2. Start php artisan queue:listen via the IDE running configurations
  3. Check docker ps for the newly created container running artisan
  4. Try to stop the process/container from your IDE ("stop" button or similar).
  5. Check docker ps for the container from 3. It lingers around even though it should have vanished.

This is likely caused by the fact that queue:listen calls queue:work --once which does not register the signal handling, because it uses runNextJob instead of daemon (https://github.com/laravel/framework/blob/10.x/src/Illuminate/Queue/Console/WorkCommand.php#L137). - Offtopic: This would have been much easier to find if the function call was a literal call and not a string -.

Why this does not happen if do stuff like docker run .... php artisan queue:listen in bash, I can only guess. Maybe the system intervenes here kills the surrounding docker process and does not directly interact with the php process like the IDE does.

Long story short: We lost the ability to properly interact with the subprocess created by queue:listen and potentially other commands, that spawn sub-processes.

I am not sure how to proceed with this, since I am pretty new to Laravel. Maybe someone can give some insights into this and/or give some pointers.

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.

None yet

3 participants