Forward signal to process

[joelvh]

Command doesn't forward signals to Horizon process to allow Horizon to shut down properly.

Fixes #16

[sdapkus]

Still not working for me. Same behaviour. On each change, new worker is started and the previous one is left running.

[joelvh]

@sdapkus does the process that's left running eventually die?

[joelvh]

@sdapkus I've added a wait call to see if that helps. If those processes eventually die, then maybe there's just a delay. Try the latest update on this PR.

What version of PHP are you using?

[sdapkus]

I'm getting same error in both commits that were made on Sep 26, 2023

sail artisan horizon:watch

   INFO  Starting Horizon and will restart it when any files change...  

PHP Fatal error:  Error installing signal handler for 9 in /var/www/html/vendor/symfony/console/SignalRegistry/SignalRegistry.php on line 37

   Symfony\Component\ErrorHandler\Error\FatalError 

  Error installing signal handler for 9

  at vendor/symfony/console/SignalRegistry/SignalRegistry.php:37
     33▕         }
     34▕ 
     35▕         $this->signalHandlers[$signal][] = $signalHandler;
     36▕ 
  ➜  37▕         pcntl_signal($signal, $this->handle(...));
     38▕     }
     39▕ 
     40▕     public static function isSupported(): bool
     41▕     {

   Whoops\Exception\ErrorException 

  Error installing signal handler for 9

  at vendor/symfony/console/SignalRegistry/SignalRegistry.php:37
     33▕         }
     34▕ 
     35▕         $this->signalHandlers[$signal][] = $signalHandler;
     36▕ 
  ➜  37▕         pcntl_signal($signal, $this->handle(...));
     38▕     }
     39▕ 
     40▕     public static function isSupported(): bool
     41▕     {

      +1 vendor frames 

  2   [internal]:0
      Whoops\Run::handleShutdown()

PHP Fatal error:  Error installing signal handler for 9 in Unknown on line 0

I'm trying this in Laravel sail, here is PHP version:

$ php -v
PHP 8.2.10 (cli) (built: Sep  2 2023 06:59:22) (NTS)
Copyright (c) The PHP Group
Zend Engine v4.2.10, Copyright (c) Zend Technologies
    with Zend OPcache v8.2.10, Copyright (c), by Zend Technologies
    with Xdebug v3.2.1, Copyright (c) 2002-2023, by Derick Rethans

On first commit, I've tried waiting for about two minutes after reload, worker from first horizon instance was still running. Maybe process status output will be helpful, you can see that multiple supervisors are running. I've did three file changes.

sail@cbeea79f3596:/var/www/html$ ps -aux
USER       PID %CPU %MEM    VSZ   RSS TTY      STAT START   TIME COMMAND
root         1  0.0  1.0  32240 25720 ?        Ss   14:46   0:01 /usr/bin/python3 /usr/bin/supervisord -c /etc/supervisor/conf.d/supervisord.conf
sail        16  0.1  2.8 271104 71252 ?        S    14:46   0:01 /usr/bin/php -d variables_order=EGPCS /var/www/html/artisan serve --host=0.0.0.0 --port=80
sail        19  0.0  1.7 260412 42652 ?        S    14:46   0:00 /usr/bin/php8.2 -S 0.0.0.0:80 /var/www/html/vendor/laravel/framework/src/Illuminate/Foundation/Console/../resources/server.php
sail       600  0.0  0.1   4136  3200 pts/1    Ss   15:01   0:00 bash
sail      1042  0.5  2.8 271104 71064 pts/0    Ss+  15:05   0:00 php artisan horizon:watch
sail      1053  0.5  2.8 271100 71112 pts/0    S+   15:05   0:00 php artisan horizon
sail      1055  1.0  1.8 893304 45848 pts/0    Sl+  15:05   0:00 /usr/bin/node /var/www/html/vendor/spatie/file-system-watcher/bin/file-watcher.js [["\/var\/www\/html\/app","\/var\/www\/html\/config","\/var\/www\/html\/databas
sail      1078  0.6  2.8 271104 71096 pts/0    S+   15:05   0:00 /usr/bin/php8.2 artisan horizon:supervisor cbeea79f3596-lfup:supervisor-1 redis --workers-name=default --balance=auto --max-processes=3 --min-processes=1 --nice=
sail      1088  0.6  2.8 271104 70892 pts/0    S+   15:05   0:00 /usr/bin/php8.2 artisan horizon:work redis --name=default --supervisor=cbeea79f3596-lfup:supervisor-1 --backoff=0 --max-time=0 --max-jobs=0 --memory=128 --queue=
sail      1119  0.7  2.8 271100 71056 pts/0    S+   15:05   0:00 php artisan horizon
sail      1124  0.7  2.8 271104 71008 pts/0    S+   15:05   0:00 /usr/bin/php8.2 artisan horizon:supervisor cbeea79f3596-lgEX:supervisor-1 redis --workers-name=default --balance=auto --max-processes=3 --min-processes=1 --nice=
sail      1131  0.6  2.8 271104 71248 pts/0    S+   15:05   0:00 /usr/bin/php8.2 artisan horizon:work redis --name=default --supervisor=cbeea79f3596-lgEX:supervisor-1 --backoff=0 --max-time=0 --max-jobs=0 --memory=128 --queue=
sail      1179  0.0  0.0   2308  1280 pts/0    S+   15:05   0:00 sh -c php artisan horizon
sail      1180  1.2  2.8 271100 71240 pts/0    S+   15:05   0:00 php artisan horizon
sail      1185  1.3  2.8 271104 71240 pts/0    S+   15:05   0:00 /usr/bin/php8.2 artisan horizon:supervisor cbeea79f3596-tiAP:supervisor-1 redis --workers-name=default --balance=auto --max-processes=3 --min-processes=1 --nice=
sail      1192  1.2  2.8 271104 71240 pts/0    S+   15:05   0:00 /usr/bin/php8.2 artisan horizon:work redis --name=default --supervisor=cbeea79f3596-tiAP:supervisor-1 --backoff=0 --max-time=0 --max-jobs=0 --memory=128 --queue=
sail      1224  0.0  0.0   6412  2432 pts/1    R+   15:06   0:00 ps -aux

[joelvh]

Thanks @sdapkus. Unfortunately, I am not reproducing the same issue. I am also not using this package directly anymore, so I'm not sure I can help much more here.

I would recommend adding debugging to this command to see what you can learn about how processes are managed in your situation and try some other approaches to fixing this issue. Alternatively, you could revert my changes and use the node-based solution that was present before#16 was merged.

[joelvh]

@freekmurze FWIW, the changes in this PR will improve the existing solution. However, you may choose to revert #16 instead.

[sdapkus]

I'm not really familiar with signals, but trying to think out loud. For me it feels that you tried to subscribe to a signals that horizon:watch are getting, in other words you tried to subscribe to signals that you manually do in terminal while horizon:watch is running.

What I'm trying to say, that your $this-trap() callback is only called, when horizon:watch is terminated, killed or smth else is done. On refresh, we are trying to kill horizon command, that you actually not trapping.

[joelvh]

@sdapkus that's right. that was my observation too, that the signal update I made is to kill the child process when you kill the whole thing. I don't know why calling stop is not actually stopping your processes on restart. You could try adding output in the Process class in your vendor folder to see what the process status is or if something else is going on.

The wait that I added ought to wait for that first process to die since that's what Process::run(...) uses to wait for the process to die too.

You can also test this outside of Sail to see if it's specific to the Docker environment.

[sdapkus]

But that's the problem, you added trap on horizon:watch, but not on horizon command that is called by watch command.

Because of that on restart, we are not killing anything, and only kill processes on horizon:watch kill

[joelvh]

@sdapkus what i was trying to say is the trap I added is only for handling when you kill the watch command itself. There isn't a signal that needs to be passed to the horizon process when restarting because stop sends the signal to the process for us (if you look at the source code of stop).

That should handle the signal already, but something doesn't work for you.

Did you try the other suggestions (e.g. run this outside of Sail)?

Otherwise, I recommend you open a PR to revert the changes I made in #16 and go back to what worked for you. I don't use this command anymore -- I use a generic version of this command that doesn't have the problems you have.

[LukeAbell]

Removing SIGKILL from the list of signals fixes the Error installing signal handler for 9 error for me.

[freekmurze]

Thanks!