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

Add Process.on_interrupt #13034

Merged

Conversation

HertzDevil
Copy link
Contributor

@HertzDevil HertzDevil commented Jan 31, 2023

Resolves #12224. The "interrupt" corresponds to Ctrl+C and Ctrl+Break for Windows console applications. The following will now work:

# press Ctrl+C or Ctrl+Break 5 times to exit

count = 5
Process.on_interrupt do
  print(count -= 1)
  exit if count == 0
end

while true
  print '.'
  sleep 1
end

It also means that specs will abort gracefully when interrupted, whether from a spec executable or via crystal spec.

The following does not work:

Process.on_interrupt { print '.' }

while true
  # `STDIN.gets` is synchronous, and the outcome depends on the signal plus the handler:
  #
  # * Ctrl+C, custom handler: `x == nil`, and the handler is not called
  # * Ctrl+Break, custom handler: `x == nil`, and the handler is not called
  # * Ctrl+C, interrupts ignored: `x == nil`
  # * Ctrl+Break, interrupts ignored: `x == nil`, but the process exits briefly afterwards anyway
  # * default handling: process exits before `STDIN.gets` returns
  x = STDIN.gets
end

Additionally, the bin\crystal.bat and bin\crystal.ps1 wrappers do not use some kind of exec (PowerShell 7.3 added Switch-Process but it is unavailable on Windows itself!), so if a custom interrupt handler returns normally without exiting the process, control returns to the console but the process will continue running in the background. This isn't a problem for crystal spec as specs are run inside an at_exit.

On Windows, this interrupt handler is run on a fresh thread every time an interrupt arrives, so some form of synchronization is required. Originally the plan was to integrate this into the event loop, but the current implementation uses an atomic counter instead, and the listener loop fiber on the main thread simply polls for any outstanding interrupts every 50 milliseconds. This seems to be a good compromise because e.g. IO::FileDescriptor#flock_* does the same thing.

@HertzDevil HertzDevil marked this pull request as ready for review February 2, 2023 22:23
src/spec.cr Outdated Show resolved Hide resolved
Comment on lines +561 to 565
{% if flag?(:win32) %}
Crystal::System::Process.start_interrupt_loop
{% else %}
Signal.setup_default_handlers
{% end %}
Copy link
Member

Choose a reason for hiding this comment

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

I suppose these handlers could also be placed in a portable interface? But this can be added later.

src/process.cr Outdated Show resolved Hide resolved
src/process.cr Outdated Show resolved Hide resolved
src/process.cr Outdated Show resolved Hide resolved
Comment on lines +27 to +28
# NOTE: `Process.on_interrupt` is preferred over `Signal::INT.trap`, as the
# former also works on Windows.
Copy link
Member

Choose a reason for hiding this comment

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

We can leave it out of the scope of this PR, but would it be feasible to implement Signal::INT.trap as Process.on_interrupt on Windows?

HertzDevil and others added 2 commits February 3, 2023 08:47
Co-authored-by: Johannes Müller <straightshoota@gmail.com>
@straight-shoota straight-shoota added this to the 1.8.0 milestone Feb 3, 2023
@straight-shoota straight-shoota merged commit 8853f73 into crystal-lang:master Feb 4, 2023
@HertzDevil HertzDevil deleted the feature/process-on_interrupt branch February 6, 2023 13:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Platform-independent interrupt handlers
2 participants