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

improve Process#on_interrupt #13654

Closed
wants to merge 11 commits into from
12 changes: 12 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,3 +1,15 @@
# 1.9.1 (2023-07-17)

### Bugfixes

#### stdlib

- *(serialization)* Fix `Serializable` with converter parsing `null` value ([#13656](https://github.com/crystal-lang/crystal/pull/13656), thanks @straight-shoota)

#### compiler

- *(codegen)* Fix generated cc command for cross compile ([#13661](https://github.com/crystal-lang/crystal/pull/13661), thanks @fnordfish)

# 1.9.0 (2023-07-11)
### Breaking changes

Expand Down
24 changes: 24 additions & 0 deletions spec/std/json/serializable_spec.cr
Original file line number Diff line number Diff line change
Expand Up @@ -212,6 +212,20 @@ class JSONAttrWithTimeEpoch
property value : Time
end

class JSONAttrNilableWithTimeEpoch
include JSON::Serializable

@[JSON::Field(converter: Time::EpochConverter)]
property value : Time?
end

class JSONAttrDefaultWithTimeEpoch
include JSON::Serializable

@[JSON::Field(converter: Time::EpochConverter)]
property value : Time = Time.unix(0)
end

class JSONAttrWithTimeEpochMillis
include JSON::Serializable

Expand Down Expand Up @@ -791,6 +805,16 @@ describe "JSON mapping" do
end
end

it "converter with null value (#13655)" do
JSONAttrNilableWithTimeEpoch.from_json(%({"value": null})).value.should be_nil
JSONAttrNilableWithTimeEpoch.from_json(%({"value":1459859781})).value.should eq Time.unix(1459859781)
end

it "converter with default value" do
JSONAttrDefaultWithTimeEpoch.from_json(%({"value": null})).value.should eq Time.unix(0)
JSONAttrDefaultWithTimeEpoch.from_json(%({"value":1459859781})).value.should eq Time.unix(1459859781)
end

it "uses Time::EpochConverter" do
string = %({"value":1459859781})
json = JSONAttrWithTimeEpoch.from_json(string)
Expand Down
61 changes: 0 additions & 61 deletions spec/std/kernel_spec.cr
Original file line number Diff line number Diff line change
Expand Up @@ -294,64 +294,3 @@ describe "hardware exception" do
error.should contain("Stack overflow")
end
end

private def compile_and_run_exit_handler(&block : Process -> _)
with_tempfile("source_file") do |source_file|
File.write(source_file, <<-CRYSTAL)
at_exit { print "Exiting" }
print "."
STDOUT.flush
sleep
CRYSTAL
output = nil
compile_file(source_file) do |executable_file|
error = IO::Memory.new
process = Process.new executable_file, output: :pipe, error: error

spawn do
process.output.read_byte
block.call(process)
output = process.output.gets_to_end
end

status = process.wait
{status, output, error.to_s}
end
end
end

describe "default interrupt handlers", tags: %w[slow] do
# TODO: Implementation for sending console control commands on Windows.
# So long this behaviour can only be tested manually.
#
# ```
# lib LibC
# fun GenerateConsoleCtrlEvent(dwCtrlEvent : DWORD, dwProcessGroupId : DWORD) : BOOL
# end

# at_exit { print "Exiting"; }
# print "."
# STDOUT.flush
# LibC.GenerateConsoleCtrlEvent(LibC::CTRL_C_EVENT, 0)
# sleep
# ```
{% unless flag?(:windows) %}
it "handler for SIGINT" do
status, output, _ = compile_and_run_exit_handler(&.signal(Signal::INT))
output.should eq "Exiting"
status.inspect.should eq "Process::Status[130]"
end

it "handler for SIGTERM" do
status, output, _ = compile_and_run_exit_handler(&.terminate)
output.should eq "Exiting"
status.inspect.should eq "Process::Status[143]"
end
{% end %}

it "no handler for SIGKILL" do
status, output, _ = compile_and_run_exit_handler(&.terminate(graceful: false))
output.should eq ""
status.inspect.should eq {{ flag?(:unix) ? "Process::Status[Signal::KILL]" : "Process::Status[1]" }}
end
end
24 changes: 24 additions & 0 deletions spec/std/yaml/serializable_spec.cr
Original file line number Diff line number Diff line change
Expand Up @@ -221,6 +221,20 @@ class YAMLAttrWithTimeEpoch
property value : Time
end

class YAMLAttrNilableWithTimeEpoch
include YAML::Serializable

@[YAML::Field(converter: Time::EpochConverter)]
property value : Time?
end

class YAMLAttrDefaultWithTimeEpoch
include YAML::Serializable

@[YAML::Field(converter: Time::EpochConverter)]
property value : Time = Time.unix(0)
end

class YAMLAttrWithTimeEpochMillis
include YAML::Serializable

Expand Down Expand Up @@ -871,6 +885,16 @@ describe "YAML::Serializable" do
end
end

it "converter with null value (#13655)" do
YAMLAttrNilableWithTimeEpoch.from_yaml(%({"value": null})).value.should be_nil
YAMLAttrNilableWithTimeEpoch.from_yaml(%({"value":1459859781})).value.should eq Time.unix(1459859781)
end

it "converter with default value" do
YAMLAttrDefaultWithTimeEpoch.from_yaml(%({"value": null})).value.should eq Time.unix(0)
YAMLAttrDefaultWithTimeEpoch.from_yaml(%({"value":1459859781})).value.should eq Time.unix(1459859781)
end

it "uses Time::EpochConverter" do
string = %({"value":1459859781})
yaml = YAMLAttrWithTimeEpoch.from_yaml(string)
Expand Down
2 changes: 1 addition & 1 deletion src/VERSION
Original file line number Diff line number Diff line change
@@ -1 +1 @@
1.9.0
1.9.1
7 changes: 4 additions & 3 deletions src/compiler/crystal/compiler.cr
Original file line number Diff line number Diff line change
Expand Up @@ -323,8 +323,9 @@ module Crystal

target_machine.emit_obj_to_file llvm_mod, output_filename
end

_, command, args = linker_command(program, [output_filename], output_filename, nil)
object_names = [output_filename]
output_filename = output_filename.rchop(unit.object_extension)
_, command, args = linker_command(program, object_names, output_filename, nil)
print_command(command, args)
end

Expand Down Expand Up @@ -684,7 +685,7 @@ module Crystal
getter original_name
getter llvm_mod
getter? reused_previous_compilation = false
@object_extension : String
getter object_extension : String

def initialize(@compiler : Compiler, program : Program, @name : String,
@llvm_mod : LLVM::Module, @output_dir : String, @bc_flags_changed : Bool)
Expand Down
12 changes: 9 additions & 3 deletions src/crystal/system/process.cr
Original file line number Diff line number Diff line change
Expand Up @@ -56,9 +56,6 @@ struct Crystal::System::Process
# thread.
# def self.start_interrupt_loop

# Trap interrupt to exit normally with `at_exit` handlers being executed.
# def self.setup_default_interrupt_handler

# Whether the process identified by *pid* is still registered in the system.
# def self.exists?(pid : Int) : Bool

Expand All @@ -80,6 +77,15 @@ struct Crystal::System::Process

# Changes the root directory for the current process.
# def self.chroot(path : String)

enum Interrupt
# sigint, ctrl-c or ctrl-break events
USER_SIGNALLED
# sighup or closed event
TERMINAL_DISCONNECTED
# sigterm, logoff or shutdown events
SESSION_ENDED
end
end

module Crystal::System
Expand Down
30 changes: 22 additions & 8 deletions src/crystal/system/unix/process.cr
Original file line number Diff line number Diff line change
Expand Up @@ -58,8 +58,28 @@ struct Crystal::System::Process
raise RuntimeError.from_errno("kill") if ret < 0
end

def self.on_interrupt(&handler : ->) : Nil
::Signal::INT.trap { |_signal| handler.call }
def self.on_interrupt(&handler : Interrupt ->) : Nil
sig_handler = Proc(::Signal, Nil).new do |signal|
int_type = case signal
when .int?
Interrupt::USER_SIGNALLED
when .hup?
Interrupt::TERMINAL_DISCONNECTED
when .term?
Interrupt::SESSION_ENDED
else
Interrupt::USER_SIGNALLED
end
handler.call int_type

# ignore prevents system defaults and clears registered interrupts
# hence we need to re-register
signal.ignore
Process.on_interrupt &handler
end
::Signal::INT.trap &sig_handler
::Signal::HUP.trap &sig_handler
::Signal::TERM.trap &sig_handler
end

def self.ignore_interrupts! : Nil
Expand All @@ -74,12 +94,6 @@ struct Crystal::System::Process
# do nothing; `Crystal::System::Signal.start_loop` takes care of this
end

def self.setup_default_interrupt_handlers
# Status 128 + signal number indicates process exit was caused by the signal.
::Signal::INT.trap { ::exit 128 + ::Signal::INT.value }
::Signal::TERM.trap { ::exit 128 + ::Signal::TERM.value }
end

def self.exists?(pid)
ret = LibC.kill(pid, 0)
if ret == 0
Expand Down
22 changes: 13 additions & 9 deletions src/crystal/system/win32/process.cr
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ struct Crystal::System::Process
@@interrupt_count = Crystal::AtomicSemaphore.new
@@win32_interrupt_handler : LibC::PHANDLER_ROUTINE?
@@setup_interrupt_handler = Atomic::Flag.new
@@last_interrupt = Interrupt::USER_SIGNALLED

def initialize(process_info)
@pid = process_info.dwProcessId
Expand Down Expand Up @@ -103,7 +104,16 @@ struct Crystal::System::Process
def self.on_interrupt(&@@interrupt_handler : ->) : Nil
restore_interrupts!
@@win32_interrupt_handler = handler = LibC::PHANDLER_ROUTINE.new do |event_type|
next 0 unless event_type.in?(LibC::CTRL_C_EVENT, LibC::CTRL_BREAK_EVENT)
@@last_interrupt = case event_type
when LibC::CTRL_C_EVENT, LibC::CTRL_BREAK_EVENT
Interrupt::USER_SIGNALLED
when LibC::CTRL_CLOSE_EVENT
Interrupt::TERMINAL_DISCONNECTED
when LibC::CTRL_LOGOFF_EVENT, LibC::CTRL_SHUTDOWN_EVENT
Interrupt::SESSION_ENDED
else
next 0
end
@@interrupt_count.signal
1
end
Expand Down Expand Up @@ -136,8 +146,9 @@ struct Crystal::System::Process

if handler = @@interrupt_handler
non_nil_handler = handler # if handler is closured it will also have the Nil type
int_type = @@last_interrupt
spawn do
non_nil_handler.call
non_nil_handler.call int_type
rescue ex
ex.inspect_with_backtrace(STDERR)
STDERR.puts("FATAL: uncaught exception while processing interrupt handler, exiting")
Expand All @@ -149,13 +160,6 @@ struct Crystal::System::Process
end
end

def self.setup_default_interrupt_handlers
on_interrupt do
# Exit code 3 indicates `std::abort` was called which corresponds to the interrupt handler
::exit 3
end
end

def self.exists?(pid)
handle = LibC.OpenProcess(LibC::PROCESS_QUERY_INFORMATION, 0, pid)
return false unless handle
Expand Down
10 changes: 8 additions & 2 deletions src/json/serialization.cr
Original file line number Diff line number Diff line change
Expand Up @@ -219,8 +219,14 @@ module JSON
{% for name, value in properties %}
when {{value[:key]}}
begin
{% if (value[:has_default] && !value[:nilable]) || value[:root] %}
next if pull.read_null?
{% if value[:has_default] || value[:nilable] || value[:root] %}
if pull.read_null?
{% if value[:nilable] %}
%var{name} = nil
%found{name} = true
{% end %}
next
end
{% end %}

%var{name} =
Expand Down
1 change: 0 additions & 1 deletion src/kernel.cr
Original file line number Diff line number Diff line change
Expand Up @@ -569,7 +569,6 @@ end
{% else %}
Crystal::System::Signal.setup_default_handlers
{% end %}
Crystal::System::Process.setup_default_interrupt_handlers

# load debug info on start up of the program is executed with CRYSTAL_LOAD_DEBUG_INFO=1
# this will make debug info available on print_frame that is used by Crystal's segfault handler
Expand Down
7 changes: 5 additions & 2 deletions src/lib_c/x86_64-windows-msvc/c/consoleapi.cr
Original file line number Diff line number Diff line change
Expand Up @@ -14,8 +14,11 @@ lib LibC
fun GetConsoleCP : DWORD
fun GetConsoleOutputCP : DWORD

CTRL_C_EVENT = 0
CTRL_BREAK_EVENT = 1
CTRL_C_EVENT = 0
CTRL_BREAK_EVENT = 1
CTRL_CLOSE_EVENT = 2
CTRL_LOGOFF_EVENT = 5
CTRL_SHUTDOWN_EVENT = 6

alias PHANDLER_ROUTINE = DWORD -> BOOL

Expand Down
20 changes: 4 additions & 16 deletions src/process.cr
Original file line number Diff line number Diff line change
Expand Up @@ -58,10 +58,6 @@ class Process
# * On Unix-like systems, this traps `SIGINT`.
# * On Windows, this captures <kbd>Ctrl</kbd> + <kbd>C</kbd> and
# <kbd>Ctrl</kbd> + <kbd>Break</kbd> signals sent to a console application.
#
# The default interrupt handler calls `::exit` to ensure `at_exit` handlers
# execute. It returns a platform-specific status code indicating an interrupt
# (`130` on Unix, `3` on Windows).
def self.on_interrupt(&handler : ->) : Nil
Crystal::System::Process.on_interrupt(&handler)
end
Expand All @@ -76,14 +72,8 @@ class Process
end

# Restores default handling of interrupt requests.
#
# The default interrupt handler calls `::exit` to ensure `at_exit` handlers
# execute. It returns a platform-specific status code indicating an interrupt
# (`130` on Unix, `3` on Windows).
def self.restore_interrupts! : Nil
Crystal::System::Process.restore_interrupts!

Crystal::System::Process.setup_default_interrupt_handlers
end

# Returns `true` if the process identified by *pid* is valid for
Expand Down Expand Up @@ -311,12 +301,10 @@ class Process
end
end

{% if flag?(:unix) %}
# :nodoc:
def initialize(pid : LibC::PidT)
@process_info = Crystal::System::Process.new(pid)
end
{% end %}
# :nodoc:
def initialize(pid : LibC::PidT)
@process_info = Crystal::System::Process.new(pid)
end

# Sends *signal* to this process.
#
Expand Down
Loading