Skip to content

Commit

Permalink
Fix #1423
Browse files Browse the repository at this point in the history
  • Loading branch information
shelld3v authored Oct 25, 2024
1 parent 8f83e14 commit 57a9d5f
Showing 1 changed file with 3 additions and 1 deletion.
4 changes: 3 additions & 1 deletion lib/controller/controller.py
Original file line number Diff line number Diff line change
Expand Up @@ -205,7 +205,9 @@ def run(self) -> None:
self.requester = Requester()
if options["async_mode"]:
self.loop = asyncio.new_event_loop()
self.loop.add_signal_handler(signal.SIGINT, self.handle_pause)
# Credit: https://stackoverflow.com/a/54886771
signal.signal(signal.SIGINT, lambda *args: self.handle_pause())
signal.signal(signal.SIGTERM, lambda *args: self.handle_pause())

while options["urls"]:
url = options["urls"][0]
Expand Down

4 comments on commit 57a9d5f

@zrquan
Copy link
Contributor

@zrquan zrquan commented on 57a9d5f Oct 25, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We can keep add_signal_handler on Linux.

try:
    self.loop.add_signal_handler(signal.SIGINT, self.handle_pause)
except NotImplementedError:
    # Windows
    signal.signal(signal.SIGINT, self.handle_pause)
    signal.signal(signal.SIGTERM, self.handle_pause)

@shelld3v
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The "Windows" solution one actually works on Linux as well, I'm just investigating other problems, as well as if this can cause any bugs.

@shelld3v
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Had to close the PR to make sure Mauro wouldn't accidentally merge it lol

@zrquan
Copy link
Contributor

@zrquan zrquan commented on 57a9d5f Oct 27, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The "Windows" solution one actually works on Linux as well

Both solutions work for now, but I’d rather go with add_signal_handler since it's specifically designed for asyncio.

Unlike signal handlers registered using signal.signal(), a callback registered with this function is allowed to interact with the event loop.

Please sign in to comment.