Skip to content

cli: override default signal handlers #4190

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 1 commit into from
Nov 20, 2021
Merged

cli: override default signal handlers #4190

merged 1 commit into from
Nov 20, 2021

Conversation

bastimeyer
Copy link
Member

  • Don't raise a KeyboardInterrupt until streamlink_cli has been fully
    initialized. This prevents a stack call from being printed to stderr
    when streamlink_cli gets interrupted early.
  • Restore default signal handlers once streamlink_cli has finished its
    initialization, so that KeyboardInterrupt exceptions can be caught to
    perform clean up tasks.
  • Restore default signal handlers in test immediately after importing
    streamlink_cli once as early as possible, to be able to cancel the
    test runners regularly via a KeyboardInterrupt.
  • Refactor CLI logging tests, which test the log output of the
    initialization and need to stop execution at some point.

resolves #4188

Both the regular entry script and invocation via python -m streamlink{,_cli} work.

$ /usr/bin/timeout --preserve-status --signal=SIGINT .1s streamlink twitch.tv/bobross
$ echo $?
130

$ /usr/bin/timeout --preserve-status --signal=SIGINT 1s streamlink twitch.tv/bobross
[cli][info] Found matching plugin twitch for URL twitch.tv/bobross
Interrupted! Exiting...
$ echo $?
130

This does not fix a KeyboardInterrupt from being raised and its call stack being printed to stderr if the user sends a SIGINT signal to the process even before the signal override happens. The signal overrides happen as early as possible, but python has to load parts of its standard library first, so there's a small time window here where we don't have any control.

There's also a secondary time window in the streamlink_cli.main.main code, as it restores the default signal handlers but then only catches KeyboardInterrupt exceptions in some cases. That shouldn't be too important though.

@bastimeyer bastimeyer added the CLI label Nov 20, 2021
@bastimeyer
cli: override default signal handlers
fa0b753 
- Don't raise a KeyboardInterrupt until streamlink_cli has been fully
  initialized. This prevents a stack call from being printed to stderr
  when streamlink_cli gets interrupted early.
- Restore default signal handlers once streamlink_cli has finished its
  initialization, so that KeyboardInterrupt exceptions can be caught to
  perform clean up tasks.
- Restore default signal handlers in test immediately after importing
  streamlink_cli once as early as possible, to be able to cancel the
  test runners regularly via a KeyboardInterrupt.
- Refactor CLI logging tests, which test the log output of the
  initialization and need to stop execution at some point.
@bastimeyer bastimeyer force-pushed the cli/override-default-signals branch from 2886892 to fa0b753 Compare November 20, 2021 13:29
@bastimeyer bastimeyer mentioned this pull request Nov 20, 2021
Closed
3 tasks
@gravyboat gravyboat merged commit 6ef5605 into streamlink:master Nov 20, 2021
@mk-pmb
Copy link

mk-pmb commented Nov 20, 2021

Thank you for fixing it! 👍

@bastimeyer bastimeyer deleted the cli/override-default-signals branch November 20, 2021 19:13
Billy2011 added a commit to Billy2011/streamlink-27 that referenced this pull request Nov 21, 2021
@Billy2011
[PATCH] cli: override default signal handlers (streamlink#4190)
f973956 
- Don't raise a KeyboardInterrupt until streamlink_cli has been fully
  initialized. This prevents a stack call from being printed to stderr
  when streamlink_cli gets interrupted early.
- Restore default signal handlers once streamlink_cli has finished its
  initialization, so that KeyboardInterrupt exceptions can be caught to
  perform clean up tasks.
- Restore default signal handlers in test immediately after importing
  streamlink_cli once as early as possible, to be able to cancel the
  test runners regularly via a KeyboardInterrupt.
- Refactor CLI logging tests, which test the log output of the
  initialization and need to stop execution at some point.
Billy2011 added a commit to Billy2011/streamlink-27 that referenced this pull request Nov 21, 2021
@Billy2011
[PATCH] cli: override default signal handlers (streamlink#4190)
b6a9ade 
- Don't raise a KeyboardInterrupt until streamlink_cli has been fully
  initialized. This prevents a stack call from being printed to stderr
  when streamlink_cli gets interrupted early.
- Restore default signal handlers once streamlink_cli has finished its
  initialization, so that KeyboardInterrupt exceptions can be caught to
  perform clean up tasks.
- Restore default signal handlers in test immediately after importing
  streamlink_cli once as early as possible, to be able to cancel the
  test runners regularly via a KeyboardInterrupt.
- Refactor CLI logging tests, which test the log output of the
  initialization and need to stop execution at some point.
Billy2011 added a commit to Billy2011/streamlink-27 that referenced this pull request Nov 21, 2021
@Billy2011
[PATCH] cli: override default signal handlers (streamlink#4190)
dfeb79e 
- Don't raise a KeyboardInterrupt until streamlink_cli has been fully
  initialized. This prevents a stack call from being printed to stderr
  when streamlink_cli gets interrupted early.
- Restore default signal handlers once streamlink_cli has finished its
  initialization, so that KeyboardInterrupt exceptions can be caught to
  perform clean up tasks.
- Restore default signal handlers in test immediately after importing
  streamlink_cli once as early as possible, to be able to cancel the
  test runners regularly via a KeyboardInterrupt.
- Refactor CLI logging tests, which test the log output of the
  initialization and need to stop execution at some point.
Billy2011 added a commit to Billy2011/streamlink-27 that referenced this pull request Nov 22, 2021
@Billy2011
[PATCH] cli: override default signal handlers (streamlink#4190)
8e9e7e3 
- Don't raise a KeyboardInterrupt until streamlink_cli has been fully
  initialized. This prevents a stack call from being printed to stderr
  when streamlink_cli gets interrupted early.
- Restore default signal handlers once streamlink_cli has finished its
  initialization, so that KeyboardInterrupt exceptions can be caught to
  perform clean up tasks.
- Restore default signal handlers in test immediately after importing
  streamlink_cli once as early as possible, to be able to cancel the
  test runners regularly via a KeyboardInterrupt.
- Refactor CLI logging tests, which test the log output of the
  initialization and need to stop execution at some point.
Billy2011 added a commit to Billy2011/streamlink-27 that referenced this pull request Nov 22, 2021
@Billy2011
[PATCH] cli: override default signal handlers (streamlink#4190)
9a93a27 
- Don't raise a KeyboardInterrupt until streamlink_cli has been fully
  initialized. This prevents a stack call from being printed to stderr
  when streamlink_cli gets interrupted early.
- Restore default signal handlers once streamlink_cli has finished its
  initialization, so that KeyboardInterrupt exceptions can be caught to
  perform clean up tasks.
- Restore default signal handlers in test immediately after importing
  streamlink_cli once as early as possible, to be able to cancel the
  test runners regularly via a KeyboardInterrupt.
- Refactor CLI logging tests, which test the log output of the
  initialization and need to stop execution at some point.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
CLI
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Brief exit message for KeyboardInterrupt
3 participants
@bastimeyer @mk-pmb @gravyboat