Skip to content

[Console] Signal does not work with Question #38820

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

Closed
lyrixx opened this issue Oct 26, 2020 · 4 comments · Fixed by #38845
Closed

[Console] Signal does not work with Question #38820

lyrixx opened this issue Oct 26, 2020 · 4 comments · Fixed by #38845

Comments

@lyrixx
Copy link
Member

lyrixx commented Oct 26, 2020

Symfony version(s) affected: 5.2.x

I broke symfony, sorry !

Description

Signal handling does not work well with question!

How to reproduce

Use bin/console make:controller and then press CTRL+C
=> it does not work anymore until you press enter

Possible Solution

Empty this list https://github.com/symfony/symfony/blob/5.x/src/Symfony/Component/Console/Application.php#L94
Most of the time, we don't care about signal. It's only useful for long running process. But since we use this list before running the command, the "Signal To Symfony Event" feature will become hard to implement. We may register signal listener if the command implement a certain insterface.

The other solution would be to not use fgets but stream_select()

Additional context

I created a simple reproducer:

<?php

pcntl_signal(SIGINT, function () {
    dump('CAUGHT');
});
pcntl_async_signals(true);


$stream = \STDIN;
$what = fgets($stream, 4096);

dump('AFTER');

=>

php test.php 
^C^C^C^C
^ "CAUGHT"
^ "CAUGHT"
^ "CAUGHT"
^ "CAUGHT"
^ "AFTER"

As you can see, I pressed 4 times CTRL+C and it did not worked until I pressed enter

@lyrixx lyrixx added the Bug label Oct 26, 2020
@lyrixx lyrixx changed the title [Console] Signal does not work with Quest [Console] Signal does not work with Question Oct 26, 2020
@lyrixx lyrixx added the Console label Oct 26, 2020
@chalasr
Copy link
Member

chalasr commented Oct 26, 2020

Good catch. Not subscribing to any signal by default makes sense to me 👍

@lyrixx
Copy link
Member Author

lyrixx commented Oct 26, 2020

@chalasr, but if we do that, the whole system with Symfony Event could not work anymore. What about my proposal ?

@chalasr
Copy link
Member

chalasr commented Oct 27, 2020

@lyrixx Sorry, I missed that part. I see now, enabling the feature from the command (typically worker) sounds good.

Could we rely on SignalableCommandInterface to automatically subscribe to default signals? Or do we need another one?

@lyrixx
Copy link
Member Author

lyrixx commented Oct 27, 2020

IMHO, we need another one, because one may want to use only event, without bothering with handling manually the signal.

I'll propose a PR soon

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants