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

Signals ignored after signal taps are closed #1450

Open
jkramer opened this issue Jan 29, 2018 · 6 comments
Open

Signals ignored after signal taps are closed #1450

jkramer opened this issue Jan 29, 2018 · 6 comments
Labels
signals tests needed Issue is generally resolved but tests were not written yet

Comments

@jkramer
Copy link
Contributor

jkramer commented Jan 29, 2018

The Problem

When adding a signal handler using a signal tap or react/whenever block and removing it/leaving the react block, signals are still ignored.

Expected Behavior

Default behavior/signal handlers should be restored.

Actual Behavior

Even though the custom handler defined in the tap/react block is not executed anymore, signals are completely ignored afterwards.

Steps to Reproduce

use NativeCall;
sub kill(int32, int32) is native {*};
my $tap = signal(SIGINT).tap: { say "sigint" };
kill($*PID, SIGINT);
$tap.close;
say "not listening to C-c anymore, trying C-c to kill this process now";
kill($*PID, SIGINT);
say "still alive";

Alternatively:

use NativeCall;
sub kill(int32, int32) is native {*};
react {
  whenever signal(SIGINT) {
    say "sigint";
    done;
  }
}

say "after react";
kill($*PID, SIGINT);
say "still alive";

Environment

@lizmat
Copy link
Contributor

lizmat commented Jan 30, 2018

Perhaps this can be solved by making sure that there is always a tap on signal(SIGINT) that does a nqp::exit(0) if there are no other taps active?

@jkramer
Copy link
Contributor Author

jkramer commented Jan 30, 2018

Would exit code 0 be correct in this case? I googled a bit and it seems like 130 is commonly used as exit code when killed by SIGINT, however I'm not sure if this is something that has to be done explicitly be the interrupted program or by the shell it's been running in. It already seems to be default behavior when not messing with signals:

$ perl6 -e 'sleep 5'
^C
$ echo $?           
130

Versus:

$ perl6 -e 'signal(SIGINT).tap: { exit(0) }; sleep 5'
^C
$ echo $?                                            
0

@jnthn
Copy link
Member

jnthn commented Jan 30, 2018

The cancel part of this function table needs to be implemented.

@jstuder-gh
Copy link
Contributor

This issue was resolved with commits db010b8 and a31579c. I don't know if there is any reasonable way of testing this, but if not it can probably be closed.

@AlexDaniel AlexDaniel added the tests needed Issue is generally resolved but tests were not written yet label Mar 14, 2018
@zoffixznet
Copy link
Contributor

Well, I wrote the test, that would've worked, but I can't get Proc::Async to flush stdout.

Opened #1700 for that that also contains the test more or less in workable form, once flushing is figured out.

@zoffixznet
Copy link
Contributor

Actually, #1700 was a brainfart, BUT, I then got stuck on #1701 -- Proc::Async.kill apparently only working once.

So, once that's resolved, this test should cover the bug:

subtest 'default signal handlers reinstalled after signal taps are closed' => {
    # because of Windows issues, shove the program into the file and run that
    # https://rt.perl.org/Ticket/Display.html?id=132258
    my $program := make-temp-file content => 
        $*OUT.out-buffer = False;
        react {
            whenever signal(SIGINT) {
                say 'CAUGHT';
                done
            }
            say 'READY';
        }

        say 'PASS';
        await Promise.in: 10;
        say 'FAIL';
    ;
    with Proc::Async.new: :out, :err, $*EXECUTABLE, $program.absolute {
        my ($out, $err) = '', '';
        .stdout.tap: { $out ~= $_ };
        .stderr.tap: { $err ~= $_ };
        my $prom := .start;
        await .ready;

        repeat { sleep .01 } until $out.contains: 'READY'; # started listening
        .kill: SIGINT; # should be caught
        repeat { sleep .01 } until $out.contains: 'CAUGHT'; # await catching
        .kill: SIGINT; # should go through standard handler

        is (await $prom).signal, Signal::SIGINT, 'killed with right signal';
        is $err, '', 'nothing on STDERR';
        cmp-ok $out, '~~', {
            .contains: <CAUGHT PASS>.all & none 'FAIL'
        }, 'STDOUT contains only the stuff we expect';
    }
}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
signals tests needed Issue is generally resolved but tests were not written yet
Projects
None yet
Development

No branches or pull requests

7 participants