Continue after interrupt by signal

[borro]

In function gearman_wait you use poll (libgearman/universal.cc:316).
Why don’t you interrupt the gearman_wait job after signal arrives, and again start wait poll?
https://github.com/gearman/gearmand/blob/master/libgearman/universal.cc#L322

[bmeynell]

@borro - For the laymen that use libgearman, but who are not familiar with its inner-workings, etc., if you have the time, would you please consider adding a little high-level context, so all can understand the benefit of your proposal?

[borro]

@bmeynell
I use interrupts to correctly terminate my application.

1. Set pcntl_async_signals in true
2. Register handler for pcntl_signal
3. Start wait for Gearamn work (GearmanWorker::wait)
4. Send signal when GearmanWorker::wait work.

Result:
Signal handler call after end work GearmanWorker::wait.

Expected Result:
Signal dispatched whet GearmanWorker::wait work.

Run in cli (require php 7.1+, pcntl, pecl-gearman):

#!/usr/bin/env php

<?php

function debug($string, ...$args)
{
    printf('%s: %s', date('Y-m-d H:i:s'),sprintf($string, ...$args));
    echo PHP_EOL;
}

$quit = false;

pcntl_async_signals(true);

pcntl_alarm(1);

pcntl_signal(SIGINT, function ($signo, $signinfo) use (&$quit) {
    $quit = true;
    debug('Signo: %s, Signinfo: %s', $signo, print_r($signinfo, 1));
});
pcntl_signal(SIGALRM, function ($signo, $signinfo) use (&$quit) {
    $quit = true;
    debug('Signo: %s, Signinfo: %s', $signo, print_r($signinfo, 1));
});
debug('SIGINT, SIGALRM inited');

$worker = new \GearmanWorker();
$worker->addOptions(GEARMAN_WORKER_NON_BLOCKING);
$worker->addServer();
$worker->setTimeout(10000);

$worker->addFunction('test', function (\GearmanJob $arg) {
    debug('Gearman worker do %s with %s', $arg->functionName(), $arg->workload());
    return 1;
});

debug('Start while worker');
while (!$quit) {
    $worker->work();
    $returnCode = $worker->returnCode();
    if ($returnCode === GEARMAN_NO_JOBS) {
        debug('Wait for jobs');
        $worker->wait();
        continue;
    }
    if ($returnCode === GEARMAN_IO_WAIT) {
        continue;
    }
    if ($returnCode !== GEARMAN_SUCCESS) {
        debug('return_code: %s', $returnCode);
    }
}
debug('Stop while worker');

If $worker->wait(); replace on sleep(10); then script stops almost instantly.

[bmeynell]

@borro - i might be putting my foot in my mouth, but fwiw, long, long ago i was using PHP's signal handlers in tandem with sleep() and the latter simply did not sleep - it was simply ignored. there was an underlying low-level reason, but it escapes me now.

@brianlmoon @wcgallego - any thoughts?

[borro]

Now if you use signals and sleep, then sleep is interrupted by a signal. See block "Return Values" http://php.net/manual/en/function.sleep.php#refsect1-function.sleep-returnvalues

For example:

#!/usr/bin/env php
<?php
function debug($string, ...$args)
{
    printf('%s: %s', date('Y-m-d H:i:s'),sprintf($string, ...$args));
    echo PHP_EOL;
}

pcntl_async_signals(true); // ←if remove this line, then sleep exit by signal, but handler don't executed.
pcntl_alarm(1);
pcntl_signal(SIGINT, function ($signo, $signinfo) {
    debug('Signo: %s, Signinfo: %s', $signo, print_r($signinfo, 1));
});
pcntl_signal(SIGALRM, function ($signo, $signinfo) {
    debug('Signo: %s, Signinfo: %s', $signo, print_r($signinfo, 1));
});

debug('SIGINT, SIGALRM inited');

debug('Start sleep');
sleep(10);
debug('End of file');

Output:

$ php sleep.php

2019-02-28 22:28:35: SIGINT, SIGALRM inited
2019-02-28 22:28:35: Start sleep
2019-02-28 22:28:36: Signo: 14, Signinfo: Array // ←this line dont printed, if comment out pcntl_async_signals
(
    [signo] => 14
    [errno] => 0
    [code] => 128
)

2019-02-28 22:28:36: End of file

I hope that you will make the possibility of interruption by a signal in gearman_wait

[esabol]

If you want to improve the signal handling here, your best bet would be to come up with a patch and submit a pull request (PR). Be sure to include a test case (or extend an existing test) in the tests subdirectory that fails without your patch and passes with your patch. Then someone will do a a code review on your PR....

[borro]

@esabol I'm afraid, but I'm very bad C++ developer.
My knowledge was enough to find where all this is happening. I can only fix it by inserting return GEARMAN_IO_WAIT; instead of continue. I do this in local and it work.

[SpamapS]

borro, this is tricky stuff to do in a library because while your way may be more correct, there may also be people relying on this behavior. Ideally we introduce a mode flag that can be used to conditionally return the right thing, and then after some broad testing, maybe drop the old "broken" way.

I believe this can be a condition on the universal structure, but I have'nt looked at that code in quite some time.

Trust your instincts and submit a PR. We'll try to get it into generally consumable shape, and release it some day. :)

[borro]

I create PR #231