From 95af602c46b5ed68999492c401e7f56a020038b4 Mon Sep 17 00:00:00 2001 From: Quinton Miller Date: Thu, 5 Sep 2024 15:34:21 +0800 Subject: [PATCH 01/36] Emulate non-blocking `STDIN` console on Windows (#14947) --- src/crystal/system/file_descriptor.cr | 9 +++ src/crystal/system/win32/file_descriptor.cr | 64 ++++++++++++++++++++- src/crystal/system/win32/iocp.cr | 35 ++++++++--- src/crystal/system/win32/process.cr | 2 +- src/io/file_descriptor.cr | 8 +++ src/lib_c/x86_64-windows-msvc/c/ioapiset.cr | 8 +++ 6 files changed, 115 insertions(+), 11 deletions(-) diff --git a/src/crystal/system/file_descriptor.cr b/src/crystal/system/file_descriptor.cr index 0652ed56e52a..481e00982e25 100644 --- a/src/crystal/system/file_descriptor.cr +++ b/src/crystal/system/file_descriptor.cr @@ -22,6 +22,15 @@ module Crystal::System::FileDescriptor # Also used in `IO::FileDescriptor#finalize`. # def file_descriptor_close + # Returns `true` or `false` if this file descriptor pretends to block or not + # to block the caller thread regardless of the underlying internal file + # descriptor's implementation. Returns `nil` if nothing needs to be done, i.e. + # `#blocking` is identical to `#system_blocking?`. + # + # Currently used by console STDIN on Windows. + private def emulated_blocking? : Bool? + end + private def system_read(slice : Bytes) : Int32 event_loop.read(self, slice) end diff --git a/src/crystal/system/win32/file_descriptor.cr b/src/crystal/system/win32/file_descriptor.cr index 3c7823e62d3e..4fdc319a8b6c 100644 --- a/src/crystal/system/win32/file_descriptor.cr +++ b/src/crystal/system/win32/file_descriptor.cr @@ -3,6 +3,7 @@ require "c/consoleapi" require "c/consoleapi2" require "c/winnls" require "crystal/system/win32/iocp" +require "crystal/system/thread" module Crystal::System::FileDescriptor # Platform-specific type to represent a file descriptor handle to the operating @@ -76,13 +77,24 @@ module Crystal::System::FileDescriptor bytes_written end + def emulated_blocking? : Bool? + # reading from STDIN is done via a separate thread (see + # `ConsoleUtils.read_console` below) + handle = windows_handle + if LibC.GetConsoleMode(handle, out _) != 0 + if handle == LibC.GetStdHandle(LibC::STD_INPUT_HANDLE) + return false + end + end + end + # :nodoc: def system_blocking? @system_blocking end private def system_blocking=(blocking) - unless blocking == @system_blocking + unless blocking == self.blocking raise IO::Error.new("Cannot reconfigure `IO::FileDescriptor#blocking` after creation") end end @@ -339,7 +351,11 @@ module Crystal::System::FileDescriptor end end + # `blocking` must be set to `true` because the underlying handles never + # support overlapped I/O; instead, `#emulated_blocking?` should return + # `false` for `STDIN` as it uses a separate thread io = IO::FileDescriptor.new(handle.address, blocking: true) + # Set sync or flush_on_newline as described in STDOUT and STDERR docs. # See https://crystal-lang.org/api/toplevel.html#STDERR if console_handle @@ -465,11 +481,57 @@ private module ConsoleUtils end private def self.read_console(handle : LibC::HANDLE, slice : Slice(UInt16)) : Int32 + @@mtx.synchronize do + @@read_requests << ReadRequest.new( + handle: handle, + slice: slice, + iocp: Crystal::EventLoop.current.iocp, + completion_key: Crystal::IOCP::CompletionKey.new(:stdin_read, ::Fiber.current), + ) + @@read_cv.signal + end + + ::Fiber.suspend + + @@mtx.synchronize do + @@bytes_read.shift + end + end + + private def self.read_console_blocking(handle : LibC::HANDLE, slice : Slice(UInt16)) : Int32 if 0 == LibC.ReadConsoleW(handle, slice, slice.size, out units_read, nil) raise IO::Error.from_winerror("ReadConsoleW") end units_read.to_i32 end + + record ReadRequest, handle : LibC::HANDLE, slice : Slice(UInt16), iocp : LibC::HANDLE, completion_key : Crystal::IOCP::CompletionKey + + @@read_cv = ::Thread::ConditionVariable.new + @@read_requests = Deque(ReadRequest).new + @@bytes_read = Deque(Int32).new + @@mtx = ::Thread::Mutex.new + @@reader_thread = ::Thread.new { reader_loop } + + private def self.reader_loop + while true + request = @@mtx.synchronize do + loop do + if entry = @@read_requests.shift? + break entry + end + @@read_cv.wait(@@mtx) + end + end + + bytes = read_console_blocking(request.handle, request.slice) + + @@mtx.synchronize do + @@bytes_read << bytes + LibC.PostQueuedCompletionStatus(request.iocp, LibC::JOB_OBJECT_MSG_EXIT_PROCESS, request.completion_key.object_id, nil) + end + end + end end # Enable UTF-8 console I/O for the duration of program execution diff --git a/src/crystal/system/win32/iocp.cr b/src/crystal/system/win32/iocp.cr index 6f5746954277..ba87ed123f22 100644 --- a/src/crystal/system/win32/iocp.cr +++ b/src/crystal/system/win32/iocp.cr @@ -6,7 +6,16 @@ require "crystal/system/thread_linked_list" module Crystal::IOCP # :nodoc: class CompletionKey + enum Tag + ProcessRun + StdinRead + end + property fiber : Fiber? + getter tag : Tag + + def initialize(@tag : Tag, @fiber : Fiber? = nil) + end end def self.wait_queued_completions(timeout, alertable = false, &) @@ -39,20 +48,19 @@ module Crystal::IOCP # at the moment only `::Process#wait` uses a non-nil completion key; all # I/O operations, including socket ones, do not set this field case completion_key = Pointer(Void).new(entry.lpCompletionKey).as(CompletionKey?) - when Nil + in Nil operation = OverlappedOperation.unbox(entry.lpOverlapped) operation.schedule { |fiber| yield fiber } - else - case entry.dwNumberOfBytesTransferred - when LibC::JOB_OBJECT_MSG_EXIT_PROCESS, LibC::JOB_OBJECT_MSG_ABNORMAL_EXIT_PROCESS + in CompletionKey + if completion_key_valid?(completion_key, entry.dwNumberOfBytesTransferred) + # if `Process` exits before a call to `#wait`, this fiber will be + # reset already if fiber = completion_key.fiber - # this ensures the `::Process` doesn't keep an indirect reference to - # `::Thread.current`, as that leads to a finalization cycle + # this ensures existing references to `completion_key` do not keep + # an indirect reference to `::Thread.current`, as that leads to a + # finalization cycle completion_key.fiber = nil - yield fiber - else - # the `Process` exits before a call to `#wait`; do nothing end end end @@ -61,6 +69,15 @@ module Crystal::IOCP false end + private def self.completion_key_valid?(completion_key, number_of_bytes_transferred) + case completion_key.tag + in .process_run? + number_of_bytes_transferred.in?(LibC::JOB_OBJECT_MSG_EXIT_PROCESS, LibC::JOB_OBJECT_MSG_ABNORMAL_EXIT_PROCESS) + in .stdin_read? + true + end + end + class OverlappedOperation enum State STARTED diff --git a/src/crystal/system/win32/process.cr b/src/crystal/system/win32/process.cr index 05b2ea36584e..2c6d81720636 100644 --- a/src/crystal/system/win32/process.cr +++ b/src/crystal/system/win32/process.cr @@ -17,7 +17,7 @@ struct Crystal::System::Process @thread_id : LibC::DWORD @process_handle : LibC::HANDLE @job_object : LibC::HANDLE - @completion_key = IOCP::CompletionKey.new + @completion_key = IOCP::CompletionKey.new(:process_run) @@interrupt_handler : Proc(::Process::ExitReason, Nil)? @@interrupt_count = Crystal::AtomicSemaphore.new diff --git a/src/io/file_descriptor.cr b/src/io/file_descriptor.cr index 8940a118041f..622229e43e00 100644 --- a/src/io/file_descriptor.cr +++ b/src/io/file_descriptor.cr @@ -66,7 +66,15 @@ class IO::FileDescriptor < IO Crystal::System::FileDescriptor.from_stdio(fd) end + # Returns whether I/O operations on this file descriptor block the current + # thread. If false, operations might opt to suspend the current fiber instead. + # + # This might be different from the internal file descriptor. For example, when + # `STDIN` is a terminal on Windows, this returns `false` since the underlying + # blocking reads are done on a completely separate thread. def blocking + emulated = emulated_blocking? + return emulated unless emulated.nil? system_blocking? end diff --git a/src/lib_c/x86_64-windows-msvc/c/ioapiset.cr b/src/lib_c/x86_64-windows-msvc/c/ioapiset.cr index 1c94b66db4c8..f6d56ef5a0e6 100644 --- a/src/lib_c/x86_64-windows-msvc/c/ioapiset.cr +++ b/src/lib_c/x86_64-windows-msvc/c/ioapiset.cr @@ -21,6 +21,14 @@ lib LibC dwMilliseconds : DWORD, fAlertable : BOOL ) : BOOL + + fun PostQueuedCompletionStatus( + completionPort : HANDLE, + dwNumberOfBytesTransferred : DWORD, + dwCompletionKey : ULONG_PTR, + lpOverlapped : OVERLAPPED* + ) : BOOL + fun CancelIoEx( hFile : HANDLE, lpOverlapped : OVERLAPPED* From f6e2ab33f272167b68a64ac3ab2ca877fa714e2a Mon Sep 17 00:00:00 2001 From: Quinton Miller Date: Thu, 5 Sep 2024 15:37:47 +0800 Subject: [PATCH 02/36] Deprecate `::sleep(Number)` (#14962) This is in line with other places in the standard library that favor `Time::Span` over number types, such as `IO` timeouts (#14368) and `Benchmark.ips` (#14805). --- samples/channel_select.cr | 2 +- samples/conway.cr | 4 ++-- samples/tcp_client.cr | 2 +- spec/std/benchmark_spec.cr | 4 ++-- spec/std/channel_spec.cr | 12 ++++++------ spec/std/http/client/client_spec.cr | 14 +++++++------- spec/std/http/server/server_spec.cr | 4 ++-- spec/std/http/spec_helper.cr | 2 +- spec/std/openssl/ssl/server_spec.cr | 2 +- spec/std/signal_spec.cr | 4 ++-- spec/support/channel.cr | 4 ++-- spec/support/retry.cr | 2 +- src/benchmark.cr | 6 +++--- src/concurrent.cr | 5 +++-- src/crystal/system/unix/file_descriptor.cr | 2 +- src/crystal/system/win32/file_descriptor.cr | 2 +- src/signal.cr | 6 +++--- 17 files changed, 39 insertions(+), 38 deletions(-) diff --git a/samples/channel_select.cr b/samples/channel_select.cr index 1ad24e1ff779..25ef96c7db16 100644 --- a/samples/channel_select.cr +++ b/samples/channel_select.cr @@ -2,7 +2,7 @@ def generator(n : T) forall T channel = Channel(T).new spawn do loop do - sleep n + sleep n.seconds channel.send n end end diff --git a/samples/conway.cr b/samples/conway.cr index b1d9d9089bb0..5178d48f9bd0 100644 --- a/samples/conway.cr +++ b/samples/conway.cr @@ -78,7 +78,7 @@ struct ConwayMap end end -PAUSE_MILLIS = 20 +PAUSE = 20.milliseconds DEFAULT_COUNT = 300 INITIAL_MAP = [ " 1 ", @@ -99,6 +99,6 @@ spawn { gets; exit } 1.upto(DEFAULT_COUNT) do |i| puts map puts "n = #{i}\tPress ENTER to exit" - sleep PAUSE_MILLIS * 0.001 + sleep PAUSE map.next end diff --git a/samples/tcp_client.cr b/samples/tcp_client.cr index 95392dc72601..f4f02d5bdf05 100644 --- a/samples/tcp_client.cr +++ b/samples/tcp_client.cr @@ -6,5 +6,5 @@ socket = TCPSocket.new "127.0.0.1", 9000 10.times do |i| socket.puts i puts "Server response: #{socket.gets}" - sleep 0.5 + sleep 0.5.seconds end diff --git a/spec/std/benchmark_spec.cr b/spec/std/benchmark_spec.cr index 2f3c1fb06fd5..8113f5f03a4c 100644 --- a/spec/std/benchmark_spec.cr +++ b/spec/std/benchmark_spec.cr @@ -13,8 +13,8 @@ describe Benchmark::IPS::Job do # test several things to avoid running a benchmark over and over again in # the specs j = Benchmark::IPS::Job.new(0.001, 0.001, interactive: false) - a = j.report("a") { sleep 0.001 } - b = j.report("b") { sleep 0.002 } + a = j.report("a") { sleep 1.milliseconds } + b = j.report("b") { sleep 2.milliseconds } j.execute diff --git a/spec/std/channel_spec.cr b/spec/std/channel_spec.cr index 9d121f9d9827..69161dd96e01 100644 --- a/spec/std/channel_spec.cr +++ b/spec/std/channel_spec.cr @@ -110,7 +110,7 @@ describe Channel do it "raises if channel is closed while waiting" do ch = Channel(String).new - spawn_and_wait(->{ sleep 0.2; ch.close }) do + spawn_and_wait(->{ sleep 0.2.seconds; ch.close }) do expect_raises Channel::ClosedError do Channel.select(ch.receive_select_action) end @@ -129,7 +129,7 @@ describe Channel do end } - spawn_and_wait(->{ sleep 0.2; ch.close }) do + spawn_and_wait(->{ sleep 0.2.seconds; ch.close }) do r = parallel p.call, p.call, p.call, p.call r.should eq({1, 1, 1, 1}) end @@ -178,7 +178,7 @@ describe Channel do it "returns nil channel is closed while waiting" do ch = Channel(String).new - spawn_and_wait(->{ sleep 0.2; ch.close }) do + spawn_and_wait(->{ sleep 0.2.seconds; ch.close }) do i, m = Channel.select(ch.receive_select_action?) m.should be_nil end @@ -191,7 +191,7 @@ describe Channel do Channel.select(ch.receive_select_action?) } - spawn_and_wait(->{ sleep 0.2; ch.close }) do + spawn_and_wait(->{ sleep 0.2.seconds; ch.close }) do r = parallel p.call, p.call, p.call, p.call r.should eq({ {0, nil}, {0, nil}, {0, nil}, {0, nil} }) end @@ -273,7 +273,7 @@ describe Channel do it "raises if channel is closed while waiting" do ch = Channel(String).new - spawn_and_wait(->{ sleep 0.2; ch.close }) do + spawn_and_wait(->{ sleep 0.2.seconds; ch.close }) do expect_raises Channel::ClosedError do Channel.select(ch.send_select_action("foo")) end @@ -292,7 +292,7 @@ describe Channel do end } - spawn_and_wait(->{ sleep 0.2; ch.close }) do + spawn_and_wait(->{ sleep 0.2.seconds; ch.close }) do r = parallel p.call, p.call, p.call, p.call r.should eq({1, 1, 1, 1}) end diff --git a/spec/std/http/client/client_spec.cr b/spec/std/http/client/client_spec.cr index 4c9da8db7ad7..451960a8c79f 100644 --- a/spec/std/http/client/client_spec.cr +++ b/spec/std/http/client/client_spec.cr @@ -6,7 +6,7 @@ require "http/server" require "http/log" require "log/spec" -private def test_server(host, port, read_time = 0, content_type = "text/plain", write_response = true, &) +private def test_server(host, port, read_time = 0.seconds, content_type = "text/plain", write_response = true, &) server = TCPServer.new(host, port) begin spawn do @@ -312,12 +312,12 @@ module HTTP end it "doesn't read the body if request was HEAD" do - resp_get = test_server("localhost", 0, 0) do |server| + resp_get = test_server("localhost", 0, 0.seconds) do |server| client = Client.new("localhost", server.local_address.port) break client.get("/") end - test_server("localhost", 0, 0) do |server| + test_server("localhost", 0, 0.seconds) do |server| client = Client.new("localhost", server.local_address.port) resp_head = client.head("/") resp_head.headers.should eq(resp_get.headers) @@ -338,7 +338,7 @@ module HTTP end it "tests read_timeout" do - test_server("localhost", 0, 0) do |server| + test_server("localhost", 0, 0.seconds) do |server| client = Client.new("localhost", server.local_address.port) client.read_timeout = 1.second client.get("/") @@ -348,7 +348,7 @@ module HTTP # it doesn't make sense to try to write because the client will already # timeout on read. Writing a response could lead on an exception in # the server if the socket is closed. - test_server("localhost", 0, 0.5, write_response: false) do |server| + test_server("localhost", 0, 0.5.seconds, write_response: false) do |server| client = Client.new("localhost", server.local_address.port) expect_raises(IO::TimeoutError, {% if flag?(:win32) %} "WSARecv timed out" {% else %} "Read timed out" {% end %}) do client.read_timeout = 0.001 @@ -362,7 +362,7 @@ module HTTP # it doesn't make sense to try to write because the client will already # timeout on read. Writing a response could lead on an exception in # the server if the socket is closed. - test_server("localhost", 0, 0, write_response: false) do |server| + test_server("localhost", 0, 0.seconds, write_response: false) do |server| client = Client.new("localhost", server.local_address.port) expect_raises(IO::TimeoutError, {% if flag?(:win32) %} "WSASend timed out" {% else %} "Write timed out" {% end %}) do client.write_timeout = 0.001 @@ -372,7 +372,7 @@ module HTTP end it "tests connect_timeout" do - test_server("localhost", 0, 0) do |server| + test_server("localhost", 0, 0.seconds) do |server| client = Client.new("localhost", server.local_address.port) client.connect_timeout = 0.5 client.get("/") diff --git a/spec/std/http/server/server_spec.cr b/spec/std/http/server/server_spec.cr index c8b39c9e7e42..5e1e5dab76f6 100644 --- a/spec/std/http/server/server_spec.cr +++ b/spec/std/http/server/server_spec.cr @@ -65,14 +65,14 @@ describe HTTP::Server do while !server.listening? Fiber.yield end - sleep 0.1 + sleep 0.1.seconds schedule_timeout ch TCPSocket.open(address.address, address.port) { } # wait before closing the server - sleep 0.1 + sleep 0.1.seconds server.close ch.receive.should eq SpecChannelStatus::End diff --git a/spec/std/http/spec_helper.cr b/spec/std/http/spec_helper.cr index 18ec9e0bab46..82b4f12d6774 100644 --- a/spec/std/http/spec_helper.cr +++ b/spec/std/http/spec_helper.cr @@ -49,7 +49,7 @@ def run_server(server, &) {% if flag?(:preview_mt) %} # avoids fiber synchronization issues in specs, like closing the server # before we properly listen, ... - sleep 0.001 + sleep 1.millisecond {% end %} yield server_done ensure diff --git a/spec/std/openssl/ssl/server_spec.cr b/spec/std/openssl/ssl/server_spec.cr index ff5e578a8ed0..2e0e413a618d 100644 --- a/spec/std/openssl/ssl/server_spec.cr +++ b/spec/std/openssl/ssl/server_spec.cr @@ -130,7 +130,7 @@ describe OpenSSL::SSL::Server do OpenSSL::SSL::Server.open tcp_server, server_context do |server| spawn do - sleep 1 + sleep 1.second OpenSSL::SSL::Socket::Client.open(TCPSocket.new(tcp_server.local_address.address, tcp_server.local_address.port), client_context, hostname: "example.com") do |socket| end end diff --git a/spec/std/signal_spec.cr b/spec/std/signal_spec.cr index cae1c5e83834..969e4dc3d742 100644 --- a/spec/std/signal_spec.cr +++ b/spec/std/signal_spec.cr @@ -27,7 +27,7 @@ pending_interpreted describe: "Signal" do Process.signal Signal::USR1, Process.pid 10.times do |i| break if ran - sleep 0.1 + sleep 0.1.seconds end ran.should be_true ensure @@ -52,7 +52,7 @@ pending_interpreted describe: "Signal" do end Process.signal Signal::USR1, Process.pid - sleep 0.1 + sleep 0.1.seconds ran_first.should be_true ran_second.should be_true ensure diff --git a/spec/support/channel.cr b/spec/support/channel.cr index 7ca8d0668797..5ec3511c89c8 100644 --- a/spec/support/channel.cr +++ b/spec/support/channel.cr @@ -10,9 +10,9 @@ def schedule_timeout(c : Channel(SpecChannelStatus)) # TODO: it's not clear why some interpreter specs # take more than 1 second in some cases. # See #12429. - sleep 5 + sleep 5.seconds {% else %} - sleep 1 + sleep 1.second {% end %} c.send(SpecChannelStatus::Timeout) end diff --git a/spec/support/retry.cr b/spec/support/retry.cr index 638804c4be81..76fca476db95 100644 --- a/spec/support/retry.cr +++ b/spec/support/retry.cr @@ -7,7 +7,7 @@ def retry(n = 5, &) if i == 0 Fiber.yield else - sleep 0.01 * (2**i) + sleep 10.milliseconds * (2**i) end else return diff --git a/src/benchmark.cr b/src/benchmark.cr index a0f4933ddf2a..14bc12ae069a 100644 --- a/src/benchmark.cr +++ b/src/benchmark.cr @@ -11,8 +11,8 @@ require "./benchmark/**" # require "benchmark" # # Benchmark.ips do |x| -# x.report("short sleep") { sleep 0.01 } -# x.report("shorter sleep") { sleep 0.001 } +# x.report("short sleep") { sleep 10.milliseconds } +# x.report("shorter sleep") { sleep 1.millisecond } # end # ``` # @@ -31,7 +31,7 @@ require "./benchmark/**" # require "benchmark" # # Benchmark.ips(warmup: 4, calculation: 10) do |x| -# x.report("sleep") { sleep 0.01 } +# x.report("sleep") { sleep 10.milliseconds } # end # ``` # diff --git a/src/concurrent.cr b/src/concurrent.cr index 6f3a58291a22..0f8805857720 100644 --- a/src/concurrent.cr +++ b/src/concurrent.cr @@ -7,6 +7,7 @@ require "crystal/tracing" # # While this fiber is waiting this time, other ready-to-execute # fibers might start their execution. +@[Deprecated("Use `::sleep(Time::Span)` instead")] def sleep(seconds : Number) : Nil if seconds < 0 raise ArgumentError.new "Sleep seconds must be positive" @@ -42,7 +43,7 @@ end # # spawn do # 6.times do -# sleep 1 +# sleep 1.second # puts 1 # end # ch.send(nil) @@ -50,7 +51,7 @@ end # # spawn do # 3.times do -# sleep 2 +# sleep 2.seconds # puts 2 # end # ch.send(nil) diff --git a/src/crystal/system/unix/file_descriptor.cr b/src/crystal/system/unix/file_descriptor.cr index fc8839ac9e83..56a9eee80dd5 100644 --- a/src/crystal/system/unix/file_descriptor.cr +++ b/src/crystal/system/unix/file_descriptor.cr @@ -158,7 +158,7 @@ module Crystal::System::FileDescriptor if retry until flock(op) - sleep 0.1 + sleep 0.1.seconds end else flock(op) || raise IO::Error.from_errno("Error applying file lock: file is already locked", target: self) diff --git a/src/crystal/system/win32/file_descriptor.cr b/src/crystal/system/win32/file_descriptor.cr index 4fdc319a8b6c..d4831d9528cb 100644 --- a/src/crystal/system/win32/file_descriptor.cr +++ b/src/crystal/system/win32/file_descriptor.cr @@ -226,7 +226,7 @@ module Crystal::System::FileDescriptor handle = windows_handle if retry && system_blocking? until lock_file(handle, flags) - sleep 0.1 + sleep 0.1.seconds end else lock_file(handle, flags) || raise IO::Error.from_winerror("Error applying file lock: file is already locked", target: self) diff --git a/src/signal.cr b/src/signal.cr index e0f59a9f57d3..37999c76b9e1 100644 --- a/src/signal.cr +++ b/src/signal.cr @@ -8,17 +8,17 @@ require "crystal/system/signal" # # ``` # puts "Ctrl+C still has the OS default action (stops the program)" -# sleep 3 +# sleep 3.seconds # # Signal::INT.trap do # puts "Gotcha!" # end # puts "Ctrl+C will be caught from now on" -# sleep 3 +# sleep 3.seconds # # Signal::INT.reset # puts "Ctrl+C is back to the OS default action" -# sleep 3 +# sleep 3.seconds # ``` # # WARNING: An uncaught exception in a signal handler is a fatal error. From 256c555b92db30b88742b0f30144c08fd07b5ce3 Mon Sep 17 00:00:00 2001 From: Quinton Miller Date: Thu, 5 Sep 2024 23:30:31 +0800 Subject: [PATCH 03/36] Implement `System::Group` on Windows (#14945) More examples of valid group IDs can be obtained using `whoami.exe /groups`. --- spec/std/system/group_spec.cr | 12 +++-- src/crystal/system/group.cr | 2 + src/crystal/system/win32/group.cr | 82 +++++++++++++++++++++++++++++++ src/crystal/system/win32/user.cr | 65 +++--------------------- src/crystal/system/windows.cr | 53 ++++++++++++++++++++ src/docs_main.cr | 4 +- 6 files changed, 153 insertions(+), 65 deletions(-) create mode 100644 src/crystal/system/win32/group.cr diff --git a/spec/std/system/group_spec.cr b/spec/std/system/group_spec.cr index 5c55611e4d28..ba511d03a05c 100644 --- a/spec/std/system/group_spec.cr +++ b/spec/std/system/group_spec.cr @@ -1,10 +1,14 @@ -{% skip_file if flag?(:win32) %} - require "spec" require "system/group" -GROUP_NAME = {{ `id -gn`.stringify.chomp }} -GROUP_ID = {{ `id -g`.stringify.chomp }} +{% if flag?(:win32) %} + GROUP_NAME = "BUILTIN\\Administrators" + GROUP_ID = "S-1-5-32-544" +{% else %} + GROUP_NAME = {{ `id -gn`.stringify.chomp }} + GROUP_ID = {{ `id -g`.stringify.chomp }} +{% end %} + INVALID_GROUP_NAME = "this_group_does_not_exist" INVALID_GROUP_ID = {% if flag?(:android) %}"8888"{% else %}"1234567"{% end %} diff --git a/src/crystal/system/group.cr b/src/crystal/system/group.cr index 8a542e2cc63c..6cb93739a900 100644 --- a/src/crystal/system/group.cr +++ b/src/crystal/system/group.cr @@ -12,6 +12,8 @@ end require "./wasi/group" {% elsif flag?(:unix) %} require "./unix/group" +{% elsif flag?(:win32) %} + require "./win32/group" {% else %} {% raise "No Crystal::System::Group implementation available" %} {% end %} diff --git a/src/crystal/system/win32/group.cr b/src/crystal/system/win32/group.cr new file mode 100644 index 000000000000..3b40774ac2d8 --- /dev/null +++ b/src/crystal/system/win32/group.cr @@ -0,0 +1,82 @@ +require "crystal/system/windows" + +# This file contains source code derived from the following: +# +# * https://cs.opensource.google/go/go/+/refs/tags/go1.23.0:src/os/user/lookup_windows.go +# * https://cs.opensource.google/go/go/+/refs/tags/go1.23.0:src/syscall/security_windows.go +# +# The following is their license: +# +# Copyright 2009 The Go Authors. +# +# Redistribution and use in source and binary forms, with or without +# modification, are permitted provided that the following conditions are +# met: +# +# * Redistributions of source code must retain the above copyright +# notice, this list of conditions and the following disclaimer. +# * Redistributions in binary form must reproduce the above +# copyright notice, this list of conditions and the following disclaimer +# in the documentation and/or other materials provided with the +# distribution. +# * Neither the name of Google LLC nor the names of its +# contributors may be used to endorse or promote products derived from +# this software without specific prior written permission. +# +# THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS +# "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT +# LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR +# A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT +# OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, +# SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT +# LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, +# DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY +# THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT +# (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE +# OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. + +module Crystal::System::Group + def initialize(@name : String, @id : String) + end + + def system_name : String + @name + end + + def system_id : String + @id + end + + def self.from_name?(groupname : String) : ::System::Group? + if found = Crystal::System.name_to_sid(groupname) + from_sid(found.sid) + end + end + + def self.from_id?(groupid : String) : ::System::Group? + if sid = Crystal::System.sid_from_s(groupid) + begin + from_sid(sid) + ensure + LibC.LocalFree(sid) + end + end + end + + private def self.from_sid(sid : LibC::SID*) : ::System::Group? + canonical = Crystal::System.sid_to_name(sid) || return + + # https://learn.microsoft.com/en-us/openspecs/windows_protocols/ms-samr/7b2aeb27-92fc-41f6-8437-deb65d950921#gt_0387e636-5654-4910-9519-1f8326cf5ec0 + # SidTypeAlias should also be treated as a group type next to SidTypeGroup + # and SidTypeWellKnownGroup: + # "alias object -> resource group: A group object..." + # + # Tests show that "Administrators" can be considered of type SidTypeAlias. + case canonical.type + when .sid_type_group?, .sid_type_well_known_group?, .sid_type_alias? + domain_and_group = canonical.domain.empty? ? canonical.name : "#{canonical.domain}\\#{canonical.name}" + gid = Crystal::System.sid_to_s(sid) + ::System::Group.new(domain_and_group, gid) + end + end +end diff --git a/src/crystal/system/win32/user.cr b/src/crystal/system/win32/user.cr index e5fcdbba10aa..4a06570c72b8 100644 --- a/src/crystal/system/win32/user.cr +++ b/src/crystal/system/win32/user.cr @@ -1,4 +1,4 @@ -require "c/sddl" +require "crystal/system/windows" require "c/lm" require "c/userenv" require "c/security" @@ -71,7 +71,7 @@ module Crystal::System::User end def self.from_username?(username : String) : ::System::User? - if found = name_to_sid(username) + if found = Crystal::System.name_to_sid(username) if found.type.sid_type_user? from_sid(found.sid) end @@ -79,7 +79,7 @@ module Crystal::System::User end def self.from_id?(id : String) : ::System::User? - if sid = sid_from_s(id) + if sid = Crystal::System.sid_from_s(id) begin from_sid(sid) ensure @@ -89,13 +89,13 @@ module Crystal::System::User end private def self.from_sid(sid : LibC::SID*) : ::System::User? - canonical = sid_to_name(sid) || return + canonical = Crystal::System.sid_to_name(sid) || return return unless canonical.type.sid_type_user? domain_and_user = "#{canonical.domain}\\#{canonical.name}" full_name = lookup_full_name(canonical.name, canonical.domain, domain_and_user) || return pgid = lookup_primary_group_id(canonical.name, canonical.domain) || return - uid = sid_to_s(sid) + uid = Crystal::System.sid_to_s(sid) home_dir = lookup_home_directory(uid, canonical.name) || return ::System::User.new(domain_and_user, uid, pgid, full_name, home_dir) @@ -136,10 +136,10 @@ module Crystal::System::User # https://support.microsoft.com/en-us/help/297951/how-to-use-the-primarygroupid-attribute-to-find-the-primary-group-for # The method follows this formula: domainRID + "-" + primaryGroupRID private def self.lookup_primary_group_id(name : String, domain : String) : String? - domain_sid = name_to_sid(domain) || return + domain_sid = Crystal::System.name_to_sid(domain) || return return unless domain_sid.type.sid_type_domain? - domain_sid_str = sid_to_s(domain_sid.sid) + domain_sid_str = Crystal::System.sid_to_s(domain_sid.sid) # If the user has joined a domain use the RID of the default primary group # called "Domain Users": @@ -210,43 +210,6 @@ module Crystal::System::User return "#{profile_dir}\\#{username}" if profile_dir end - private record SIDLookupResult, sid : LibC::SID*, domain : String, type : LibC::SID_NAME_USE - - private def self.name_to_sid(name : String) : SIDLookupResult? - utf16_name = Crystal::System.to_wstr(name) - - sid_size = LibC::DWORD.zero - domain_buf_size = LibC::DWORD.zero - LibC.LookupAccountNameW(nil, utf16_name, nil, pointerof(sid_size), nil, pointerof(domain_buf_size), out _) - - unless WinError.value.error_none_mapped? - sid = Pointer(UInt8).malloc(sid_size).as(LibC::SID*) - domain_buf = Slice(LibC::WCHAR).new(domain_buf_size) - if LibC.LookupAccountNameW(nil, utf16_name, sid, pointerof(sid_size), domain_buf, pointerof(domain_buf_size), out sid_type) != 0 - domain = String.from_utf16(domain_buf[..-2]) - SIDLookupResult.new(sid, domain, sid_type) - end - end - end - - private record NameLookupResult, name : String, domain : String, type : LibC::SID_NAME_USE - - private def self.sid_to_name(sid : LibC::SID*) : NameLookupResult? - name_buf_size = LibC::DWORD.zero - domain_buf_size = LibC::DWORD.zero - LibC.LookupAccountSidW(nil, sid, nil, pointerof(name_buf_size), nil, pointerof(domain_buf_size), out _) - - unless WinError.value.error_none_mapped? - name_buf = Slice(LibC::WCHAR).new(name_buf_size) - domain_buf = Slice(LibC::WCHAR).new(domain_buf_size) - if LibC.LookupAccountSidW(nil, sid, name_buf, pointerof(name_buf_size), domain_buf, pointerof(domain_buf_size), out sid_type) != 0 - name = String.from_utf16(name_buf[..-2]) - domain = String.from_utf16(domain_buf[..-2]) - NameLookupResult.new(name, domain, sid_type) - end - end - end - private def self.domain_joined? : Bool status = LibC.NetGetJoinInformation(nil, out domain, out type) if status != LibC::NERR_Success @@ -256,18 +219,4 @@ module Crystal::System::User LibC.NetApiBufferFree(domain) is_domain end - - private def self.sid_to_s(sid : LibC::SID*) : String - if LibC.ConvertSidToStringSidW(sid, out ptr) == 0 - raise RuntimeError.from_winerror("ConvertSidToStringSidW") - end - str, _ = String.from_utf16(ptr) - LibC.LocalFree(ptr) - str - end - - private def self.sid_from_s(str : String) : LibC::SID* - status = LibC.ConvertStringSidToSidW(Crystal::System.to_wstr(str), out sid) - status != 0 ? sid : Pointer(LibC::SID).null - end end diff --git a/src/crystal/system/windows.cr b/src/crystal/system/windows.cr index b303d4d61f6d..90b38396cf8f 100644 --- a/src/crystal/system/windows.cr +++ b/src/crystal/system/windows.cr @@ -1,3 +1,5 @@ +require "c/sddl" + # :nodoc: module Crystal::System def self.retry_wstr_buffer(&) @@ -13,4 +15,55 @@ module Crystal::System def self.to_wstr(str : String, name : String? = nil) : LibC::LPWSTR str.check_no_null_byte(name).to_utf16.to_unsafe end + + def self.sid_to_s(sid : LibC::SID*) : String + if LibC.ConvertSidToStringSidW(sid, out ptr) == 0 + raise RuntimeError.from_winerror("ConvertSidToStringSidW") + end + str, _ = String.from_utf16(ptr) + LibC.LocalFree(ptr) + str + end + + def self.sid_from_s(str : String) : LibC::SID* + status = LibC.ConvertStringSidToSidW(to_wstr(str), out sid) + status != 0 ? sid : Pointer(LibC::SID).null + end + + record SIDLookupResult, sid : LibC::SID*, domain : String, type : LibC::SID_NAME_USE + + def self.name_to_sid(name : String) : SIDLookupResult? + utf16_name = to_wstr(name) + + sid_size = LibC::DWORD.zero + domain_buf_size = LibC::DWORD.zero + LibC.LookupAccountNameW(nil, utf16_name, nil, pointerof(sid_size), nil, pointerof(domain_buf_size), out _) + + unless WinError.value.error_none_mapped? + sid = Pointer(UInt8).malloc(sid_size).as(LibC::SID*) + domain_buf = Slice(LibC::WCHAR).new(domain_buf_size) + if LibC.LookupAccountNameW(nil, utf16_name, sid, pointerof(sid_size), domain_buf, pointerof(domain_buf_size), out sid_type) != 0 + domain = String.from_utf16(domain_buf[..-2]) + SIDLookupResult.new(sid, domain, sid_type) + end + end + end + + record NameLookupResult, name : String, domain : String, type : LibC::SID_NAME_USE + + def self.sid_to_name(sid : LibC::SID*) : NameLookupResult? + name_buf_size = LibC::DWORD.zero + domain_buf_size = LibC::DWORD.zero + LibC.LookupAccountSidW(nil, sid, nil, pointerof(name_buf_size), nil, pointerof(domain_buf_size), out _) + + unless WinError.value.error_none_mapped? + name_buf = Slice(LibC::WCHAR).new(name_buf_size) + domain_buf = Slice(LibC::WCHAR).new(domain_buf_size) + if LibC.LookupAccountSidW(nil, sid, name_buf, pointerof(name_buf_size), domain_buf, pointerof(domain_buf_size), out sid_type) != 0 + name = String.from_utf16(name_buf[..-2]) + domain = String.from_utf16(domain_buf[..-2]) + NameLookupResult.new(name, domain, sid_type) + end + end + end end diff --git a/src/docs_main.cr b/src/docs_main.cr index e670d6d3fa83..ab3ee2affdbc 100644 --- a/src/docs_main.cr +++ b/src/docs_main.cr @@ -56,8 +56,6 @@ require "./uri/params/serializable" require "./uuid" require "./uuid/json" require "./syscall" -{% unless flag?(:win32) %} - require "./system/*" -{% end %} +require "./system/*" require "./wait_group" require "./docs_pseudo_methods" From 05c5eaa17ce17c60917b30cebaca022d59721e2e Mon Sep 17 00:00:00 2001 From: Quinton Miller Date: Thu, 5 Sep 2024 23:31:33 +0800 Subject: [PATCH 04/36] Implement `Reference.pre_initialize` in the interpreter (#14968) --- spec/primitives/reference_spec.cr | 12 ++++++---- .../crystal/interpreter/instructions.cr | 10 ++++++++ .../crystal/interpreter/primitives.cr | 24 +++++++++++++++++++ 3 files changed, 42 insertions(+), 4 deletions(-) diff --git a/spec/primitives/reference_spec.cr b/spec/primitives/reference_spec.cr index 13bb024f1ba9..497b49155b5a 100644 --- a/spec/primitives/reference_spec.cr +++ b/spec/primitives/reference_spec.cr @@ -37,8 +37,7 @@ describe "Primitives: reference" do end end - # TODO: implement in the interpreter - pending_interpreted describe: ".pre_initialize" do + describe ".pre_initialize" do it "doesn't fail on complex ivar initializer if value is discarded (#14325)" do bar_buffer = GC.malloc(instance_sizeof(Outer)) Outer.pre_initialize(bar_buffer) @@ -55,7 +54,12 @@ describe "Primitives: reference" do it "sets type ID" do foo_buffer = GC.malloc(instance_sizeof(Foo)) base = Foo.pre_initialize(foo_buffer).as(Base) - base.crystal_type_id.should eq(Foo.crystal_instance_type_id) + base.should be_a(Foo) + base.as(typeof(Foo.crystal_instance_type_id)*).value.should eq(Foo.crystal_instance_type_id) + {% unless flag?(:interpreted) %} + # FIXME: `Object#crystal_type_id` is incorrect for virtual types in the interpreter (#14967) + base.crystal_type_id.should eq(Foo.crystal_instance_type_id) + {% end %} end it "runs inline instance initializers" do @@ -89,7 +93,7 @@ describe "Primitives: reference" do end end - pending_interpreted describe: ".unsafe_construct" do + describe ".unsafe_construct" do it "constructs an object in-place" do foo_buffer = GC.malloc(instance_sizeof(Foo)) foo = Foo.unsafe_construct(foo_buffer, 123_i64) diff --git a/src/compiler/crystal/interpreter/instructions.cr b/src/compiler/crystal/interpreter/instructions.cr index 8fae94f5ee62..6a38afd888d3 100644 --- a/src/compiler/crystal/interpreter/instructions.cr +++ b/src/compiler/crystal/interpreter/instructions.cr @@ -1276,6 +1276,16 @@ require "./repl" ptr end, }, + reset_class: { + operands: [size : Int32, type_id : Int32], + pop_values: [pointer : Pointer(UInt8)], + push: true, + code: begin + pointer.clear(size) + pointer.as(Int32*).value = type_id + pointer + end, + }, put_metaclass: { operands: [size : Int32, union_type : Bool], push: true, diff --git a/src/compiler/crystal/interpreter/primitives.cr b/src/compiler/crystal/interpreter/primitives.cr index 7ad508f8d0fc..ca436947370e 100644 --- a/src/compiler/crystal/interpreter/primitives.cr +++ b/src/compiler/crystal/interpreter/primitives.cr @@ -178,6 +178,30 @@ class Crystal::Repl::Compiler pop(sizeof(Pointer(Void)), node: nil) end end + when "pre_initialize" + type = + if obj + discard_value(obj) + obj.type.instance_type + else + scope.instance_type + end + + accept_call_members(node) + + dup sizeof(Pointer(Void)), node: nil + reset_class(aligned_instance_sizeof_type(type), type_id(type), node: node) + + initializer_compiled_defs = @context.type_instance_var_initializers(type) + unless initializer_compiled_defs.empty? + initializer_compiled_defs.size.times do + dup sizeof(Pointer(Void)), node: nil + end + + initializer_compiled_defs.each do |compiled_def| + call compiled_def, node: nil + end + end when "tuple_indexer_known_index" unless @wants_value accept_call_members(node) From 777643886c3e7e549ce295e920b95d8d426e544c Mon Sep 17 00:00:00 2001 From: Quinton Miller Date: Fri, 6 Sep 2024 06:36:14 +0800 Subject: [PATCH 05/36] Add `Crystal::System::Addrinfo` (#14957) Moves the platform-specific code into a separate module, so that implementations other than `LibC.getaddrinfo` can be added without cluttering the same file (e.g. Win32's `GetAddrInfoExW` from #13619). --- src/crystal/system/addrinfo.cr | 36 ++++++++++ src/crystal/system/unix/addrinfo.cr | 71 +++++++++++++++++++ src/crystal/system/wasi/addrinfo.cr | 27 +++++++ src/crystal/system/win32/addrinfo.cr | 61 ++++++++++++++++ src/socket/addrinfo.cr | 102 ++++----------------------- 5 files changed, 209 insertions(+), 88 deletions(-) create mode 100644 src/crystal/system/addrinfo.cr create mode 100644 src/crystal/system/unix/addrinfo.cr create mode 100644 src/crystal/system/wasi/addrinfo.cr create mode 100644 src/crystal/system/win32/addrinfo.cr diff --git a/src/crystal/system/addrinfo.cr b/src/crystal/system/addrinfo.cr new file mode 100644 index 000000000000..23513e6f763e --- /dev/null +++ b/src/crystal/system/addrinfo.cr @@ -0,0 +1,36 @@ +module Crystal::System::Addrinfo + # alias Handle + + # protected def initialize(addrinfo : Handle) + + # def system_ip_address : ::Socket::IPAddress + + # def self.getaddrinfo(domain, service, family, type, protocol, timeout) : Handle + + # def self.next_addrinfo(addrinfo : Handle) : Handle + + # def self.free_addrinfo(addrinfo : Handle) + + def self.getaddrinfo(domain, service, family, type, protocol, timeout, & : ::Socket::Addrinfo ->) + addrinfo = root = getaddrinfo(domain, service, family, type, protocol, timeout) + + begin + while addrinfo + yield ::Socket::Addrinfo.new(addrinfo) + addrinfo = next_addrinfo(addrinfo) + end + ensure + free_addrinfo(root) + end + end +end + +{% if flag?(:wasi) %} + require "./wasi/addrinfo" +{% elsif flag?(:unix) %} + require "./unix/addrinfo" +{% elsif flag?(:win32) %} + require "./win32/addrinfo" +{% else %} + {% raise "No Crystal::System::Addrinfo implementation available" %} +{% end %} diff --git a/src/crystal/system/unix/addrinfo.cr b/src/crystal/system/unix/addrinfo.cr new file mode 100644 index 000000000000..7f1e51558397 --- /dev/null +++ b/src/crystal/system/unix/addrinfo.cr @@ -0,0 +1,71 @@ +module Crystal::System::Addrinfo + alias Handle = LibC::Addrinfo* + + @addr : LibC::SockaddrIn6 + + protected def initialize(addrinfo : Handle) + @family = ::Socket::Family.from_value(addrinfo.value.ai_family) + @type = ::Socket::Type.from_value(addrinfo.value.ai_socktype) + @protocol = ::Socket::Protocol.from_value(addrinfo.value.ai_protocol) + @size = addrinfo.value.ai_addrlen.to_i + + @addr = uninitialized LibC::SockaddrIn6 + + case @family + when ::Socket::Family::INET6 + addrinfo.value.ai_addr.as(LibC::SockaddrIn6*).copy_to(pointerof(@addr).as(LibC::SockaddrIn6*), 1) + when ::Socket::Family::INET + addrinfo.value.ai_addr.as(LibC::SockaddrIn*).copy_to(pointerof(@addr).as(LibC::SockaddrIn*), 1) + else + # TODO: (asterite) UNSPEC and UNIX unsupported? + end + end + + def system_ip_address : ::Socket::IPAddress + ::Socket::IPAddress.from(to_unsafe, size) + end + + def to_unsafe + pointerof(@addr).as(LibC::Sockaddr*) + end + + def self.getaddrinfo(domain, service, family, type, protocol, timeout) : Handle + hints = LibC::Addrinfo.new + hints.ai_family = (family || ::Socket::Family::UNSPEC).to_i32 + hints.ai_socktype = type + hints.ai_protocol = protocol + hints.ai_flags = 0 + + if service.is_a?(Int) + hints.ai_flags |= LibC::AI_NUMERICSERV + end + + # On OS X < 10.12, the libsystem implementation of getaddrinfo segfaults + # if AI_NUMERICSERV is set, and servname is NULL or 0. + {% if flag?(:darwin) %} + if service.in?(0, nil) && (hints.ai_flags & LibC::AI_NUMERICSERV) + hints.ai_flags |= LibC::AI_NUMERICSERV + service = "00" + end + {% end %} + + ret = LibC.getaddrinfo(domain, service.to_s, pointerof(hints), out ptr) + unless ret.zero? + if ret == LibC::EAI_SYSTEM + raise ::Socket::Addrinfo::Error.from_os_error nil, Errno.value, domain: domain + end + + error = Errno.new(ret) + raise ::Socket::Addrinfo::Error.from_os_error(nil, error, domain: domain, type: type, protocol: protocol, service: service) + end + ptr + end + + def self.next_addrinfo(addrinfo : Handle) : Handle + addrinfo.value.ai_next + end + + def self.free_addrinfo(addrinfo : Handle) + LibC.freeaddrinfo(addrinfo) + end +end diff --git a/src/crystal/system/wasi/addrinfo.cr b/src/crystal/system/wasi/addrinfo.cr new file mode 100644 index 000000000000..29ba8e0b3cfc --- /dev/null +++ b/src/crystal/system/wasi/addrinfo.cr @@ -0,0 +1,27 @@ +module Crystal::System::Addrinfo + alias Handle = NoReturn + + protected def initialize(addrinfo : Handle) + raise NotImplementedError.new("Crystal::System::Addrinfo#initialize") + end + + def system_ip_address : ::Socket::IPAddress + raise NotImplementedError.new("Crystal::System::Addrinfo#system_ip_address") + end + + def to_unsafe + raise NotImplementedError.new("Crystal::System::Addrinfo#to_unsafe") + end + + def self.getaddrinfo(domain, service, family, type, protocol, timeout) : Handle + raise NotImplementedError.new("Crystal::System::Addrinfo.getaddrinfo") + end + + def self.next_addrinfo(addrinfo : Handle) : Handle + raise NotImplementedError.new("Crystal::System::Addrinfo.next_addrinfo") + end + + def self.free_addrinfo(addrinfo : Handle) + raise NotImplementedError.new("Crystal::System::Addrinfo.free_addrinfo") + end +end diff --git a/src/crystal/system/win32/addrinfo.cr b/src/crystal/system/win32/addrinfo.cr new file mode 100644 index 000000000000..b033d61f16e7 --- /dev/null +++ b/src/crystal/system/win32/addrinfo.cr @@ -0,0 +1,61 @@ +module Crystal::System::Addrinfo + alias Handle = LibC::Addrinfo* + + @addr : LibC::SockaddrIn6 + + protected def initialize(addrinfo : Handle) + @family = ::Socket::Family.from_value(addrinfo.value.ai_family) + @type = ::Socket::Type.from_value(addrinfo.value.ai_socktype) + @protocol = ::Socket::Protocol.from_value(addrinfo.value.ai_protocol) + @size = addrinfo.value.ai_addrlen.to_i + + @addr = uninitialized LibC::SockaddrIn6 + + case @family + when ::Socket::Family::INET6 + addrinfo.value.ai_addr.as(LibC::SockaddrIn6*).copy_to(pointerof(@addr).as(LibC::SockaddrIn6*), 1) + when ::Socket::Family::INET + addrinfo.value.ai_addr.as(LibC::SockaddrIn*).copy_to(pointerof(@addr).as(LibC::SockaddrIn*), 1) + else + # TODO: (asterite) UNSPEC and UNIX unsupported? + end + end + + def system_ip_address : ::Socket::IPAddress + ::Socket::IPAddress.from(to_unsafe, size) + end + + def to_unsafe + pointerof(@addr).as(LibC::Sockaddr*) + end + + def self.getaddrinfo(domain, service, family, type, protocol, timeout) : Handle + hints = LibC::Addrinfo.new + hints.ai_family = (family || ::Socket::Family::UNSPEC).to_i32 + hints.ai_socktype = type + hints.ai_protocol = protocol + hints.ai_flags = 0 + + if service.is_a?(Int) + hints.ai_flags |= LibC::AI_NUMERICSERV + if service < 0 + raise ::Socket::Addrinfo::Error.from_os_error(nil, WinError::WSATYPE_NOT_FOUND, domain: domain, type: type, protocol: protocol, service: service) + end + end + + ret = LibC.getaddrinfo(domain, service.to_s, pointerof(hints), out ptr) + unless ret.zero? + error = WinError.new(ret.to_u32!) + raise ::Socket::Addrinfo::Error.from_os_error(nil, error, domain: domain, type: type, protocol: protocol, service: service) + end + ptr + end + + def self.next_addrinfo(addrinfo : Handle) : Handle + addrinfo.value.ai_next + end + + def self.free_addrinfo(addrinfo : Handle) + LibC.freeaddrinfo(addrinfo) + end +end diff --git a/src/socket/addrinfo.cr b/src/socket/addrinfo.cr index c7a8ada00d86..cdf55c912601 100644 --- a/src/socket/addrinfo.cr +++ b/src/socket/addrinfo.cr @@ -1,16 +1,17 @@ require "uri/punycode" require "./address" +require "crystal/system/addrinfo" class Socket # Domain name resolver. struct Addrinfo + include Crystal::System::Addrinfo + getter family : Family getter type : Type getter protocol : Protocol getter size : Int32 - @addr : LibC::SockaddrIn6 - # Resolves a domain that best matches the given options. # # - *domain* may be an IP address or a domain name. @@ -126,66 +127,15 @@ class Socket end private def self.getaddrinfo(domain, service, family, type, protocol, timeout, &) - {% if flag?(:wasm32) %} - raise NotImplementedError.new "Socket::Addrinfo.getaddrinfo" - {% else %} - # RFC 3986 says: - # > When a non-ASCII registered name represents an internationalized domain name - # > intended for resolution via the DNS, the name must be transformed to the IDNA - # > encoding [RFC3490] prior to name lookup. - domain = URI::Punycode.to_ascii domain - - hints = LibC::Addrinfo.new - hints.ai_family = (family || Family::UNSPEC).to_i32 - hints.ai_socktype = type - hints.ai_protocol = protocol - hints.ai_flags = 0 - - if service.is_a?(Int) - hints.ai_flags |= LibC::AI_NUMERICSERV - end - - # On OS X < 10.12, the libsystem implementation of getaddrinfo segfaults - # if AI_NUMERICSERV is set, and servname is NULL or 0. - {% if flag?(:darwin) %} - if service.in?(0, nil) && (hints.ai_flags & LibC::AI_NUMERICSERV) - hints.ai_flags |= LibC::AI_NUMERICSERV - service = "00" - end - {% end %} - {% if flag?(:win32) %} - if service.is_a?(Int) && service < 0 - raise Error.from_os_error(nil, WinError::WSATYPE_NOT_FOUND, domain: domain, type: type, protocol: protocol, service: service) - end - {% end %} - - ret = LibC.getaddrinfo(domain, service.to_s, pointerof(hints), out ptr) - unless ret.zero? - {% if flag?(:unix) %} - # EAI_SYSTEM is not defined on win32 - if ret == LibC::EAI_SYSTEM - raise Error.from_os_error nil, Errno.value, domain: domain - end - {% end %} - - error = {% if flag?(:win32) %} - WinError.new(ret.to_u32!) - {% else %} - Errno.new(ret) - {% end %} - raise Error.from_os_error(nil, error, domain: domain, type: type, protocol: protocol, service: service) - end - - addrinfo = ptr - begin - while addrinfo - yield new(addrinfo) - addrinfo = addrinfo.value.ai_next - end - ensure - LibC.freeaddrinfo(ptr) - end - {% end %} + # RFC 3986 says: + # > When a non-ASCII registered name represents an internationalized domain name + # > intended for resolution via the DNS, the name must be transformed to the IDNA + # > encoding [RFC3490] prior to name lookup. + domain = URI::Punycode.to_ascii domain + + Crystal::System::Addrinfo.getaddrinfo(domain, service, family, type, protocol, timeout) do |addrinfo| + yield addrinfo + end end # Resolves *domain* for the TCP protocol and returns an `Array` of possible @@ -226,29 +176,9 @@ class Socket resolve(domain, service, family, Type::DGRAM, Protocol::UDP) { |addrinfo| yield addrinfo } end - protected def initialize(addrinfo : LibC::Addrinfo*) - @family = Family.from_value(addrinfo.value.ai_family) - @type = Type.from_value(addrinfo.value.ai_socktype) - @protocol = Protocol.from_value(addrinfo.value.ai_protocol) - @size = addrinfo.value.ai_addrlen.to_i - - @addr = uninitialized LibC::SockaddrIn6 - - case @family - when Family::INET6 - addrinfo.value.ai_addr.as(LibC::SockaddrIn6*).copy_to(pointerof(@addr).as(LibC::SockaddrIn6*), 1) - when Family::INET - addrinfo.value.ai_addr.as(LibC::SockaddrIn*).copy_to(pointerof(@addr).as(LibC::SockaddrIn*), 1) - else - # TODO: (asterite) UNSPEC and UNIX unsupported? - end - end - - @ip_address : IPAddress? - # Returns an `IPAddress` matching this addrinfo. - def ip_address : Socket::IPAddress - @ip_address ||= IPAddress.from(to_unsafe, size) + getter(ip_address : Socket::IPAddress) do + system_ip_address end def inspect(io : IO) @@ -259,9 +189,5 @@ class Socket io << protocol io << ")" end - - def to_unsafe - pointerof(@addr).as(LibC::Sockaddr*) - end end end From db2ecd781422d5b4cd615d5b10874072c1310b4d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Johannes=20M=C3=BCller?= Date: Fri, 6 Sep 2024 00:36:42 +0200 Subject: [PATCH 06/36] Fix `Range#size` return type to `Int32` (#14588) The `super` implementation `Enumerable#size` has the same type restriction already. --- src/range.cr | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/src/range.cr b/src/range.cr index 39d8119dff6e..e8ee24b190cb 100644 --- a/src/range.cr +++ b/src/range.cr @@ -480,7 +480,10 @@ struct Range(B, E) # (3..8).size # => 6 # (3...8).size # => 5 # ``` - def size + # + # Raises `OverflowError` if the difference is bigger than `Int32`. + # Raises `ArgumentError` if either `begin` or `end` are `nil`. + def size : Int32 b = self.begin e = self.end @@ -488,7 +491,7 @@ struct Range(B, E) if b.is_a?(Int) && e.is_a?(Int) e -= 1 if @exclusive n = e - b + 1 - n < 0 ? 0 : n + n < 0 ? 0 : n.to_i32 else if b.nil? || e.nil? raise ArgumentError.new("Can't calculate size of an open range") From 214d39ad112374910640c87e21695c9e8eb9d213 Mon Sep 17 00:00:00 2001 From: Quinton Miller Date: Fri, 6 Sep 2024 14:15:19 +0800 Subject: [PATCH 07/36] Fix CRT static-dynamic linking conflict in specs with C sources (#14970) This fixes the `LINK : warning LNK4098: defaultlib 'LIBCMT' conflicts with use of other libs; use /NODEFAULTLIB:library` message that shows up on Windows CI while running compiler specs. --- spec/support/tempfile.cr | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/spec/support/tempfile.cr b/spec/support/tempfile.cr index a77070d90e40..ef4468040955 100644 --- a/spec/support/tempfile.cr +++ b/spec/support/tempfile.cr @@ -67,7 +67,7 @@ def with_temp_c_object_file(c_code, *, filename = "temp_c", file = __FILE__, &) end end - `#{cl} /nologo /c #{Process.quote(c_filename)} #{Process.quote("/Fo#{o_filename}")}`.should be_truthy + `#{cl} /nologo /c /MD #{Process.quote(c_filename)} #{Process.quote("/Fo#{o_filename}")}`.should be_truthy {% else %} `#{ENV["CC"]? || "cc"} #{Process.quote(c_filename)} -c -o #{Process.quote(o_filename)}`.should be_truthy {% end %} From a310dee1bbf30839964e798d7cd5653c5149ba3d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Johannes=20M=C3=BCller?= Date: Fri, 6 Sep 2024 08:19:38 +0200 Subject: [PATCH 08/36] Fix use global paths in macro bodies (#14965) Macros inject code into other scopes. Paths are resolved in the expanded scope and there can be namespace conflicts. This fixes non-global paths in macro bodies that expand into uncontrolled scopes where namespaces could clash. This is a fixup for #14282 (released in 1.12.0). --- src/crystal/pointer_linked_list.cr | 4 ++-- src/ecr/macros.cr | 2 +- src/intrinsics.cr | 34 +++++++++++++++--------------- src/json/serialization.cr | 6 +++--- src/number.cr | 8 +++---- src/object.cr | 12 +++++------ src/slice.cr | 6 +++--- src/spec/dsl.cr | 4 ++-- src/spec/helpers/iterate.cr | 8 +++---- src/static_array.cr | 2 +- src/syscall/aarch64-linux.cr | 2 +- src/syscall/arm-linux.cr | 2 +- src/syscall/i386-linux.cr | 2 +- src/syscall/x86_64-linux.cr | 2 +- src/uri/params/serializable.cr | 14 ++++++------ src/yaml/serialization.cr | 14 ++++++------ 16 files changed, 61 insertions(+), 61 deletions(-) diff --git a/src/crystal/pointer_linked_list.cr b/src/crystal/pointer_linked_list.cr index 03109979d662..cde9b0b79ddc 100644 --- a/src/crystal/pointer_linked_list.cr +++ b/src/crystal/pointer_linked_list.cr @@ -7,8 +7,8 @@ struct Crystal::PointerLinkedList(T) module Node macro included - property previous : Pointer(self) = Pointer(self).null - property next : Pointer(self) = Pointer(self).null + property previous : ::Pointer(self) = ::Pointer(self).null + property next : ::Pointer(self) = ::Pointer(self).null end end diff --git a/src/ecr/macros.cr b/src/ecr/macros.cr index 92c02cc4284a..5e051232271b 100644 --- a/src/ecr/macros.cr +++ b/src/ecr/macros.cr @@ -34,7 +34,7 @@ module ECR # ``` macro def_to_s(filename) def to_s(__io__ : IO) : Nil - ECR.embed {{filename}}, "__io__" + ::ECR.embed {{filename}}, "__io__" end end diff --git a/src/intrinsics.cr b/src/intrinsics.cr index c5ae837d8931..7cdc462ce543 100644 --- a/src/intrinsics.cr +++ b/src/intrinsics.cr @@ -179,7 +179,7 @@ end module Intrinsics macro debugtrap - LibIntrinsics.debugtrap + ::LibIntrinsics.debugtrap end def self.pause @@ -191,15 +191,15 @@ module Intrinsics end macro memcpy(dest, src, len, is_volatile) - LibIntrinsics.memcpy({{dest}}, {{src}}, {{len}}, {{is_volatile}}) + ::LibIntrinsics.memcpy({{dest}}, {{src}}, {{len}}, {{is_volatile}}) end macro memmove(dest, src, len, is_volatile) - LibIntrinsics.memmove({{dest}}, {{src}}, {{len}}, {{is_volatile}}) + ::LibIntrinsics.memmove({{dest}}, {{src}}, {{len}}, {{is_volatile}}) end macro memset(dest, val, len, is_volatile) - LibIntrinsics.memset({{dest}}, {{val}}, {{len}}, {{is_volatile}}) + ::LibIntrinsics.memset({{dest}}, {{val}}, {{len}}, {{is_volatile}}) end def self.read_cycle_counter @@ -263,43 +263,43 @@ module Intrinsics end macro countleading8(src, zero_is_undef) - LibIntrinsics.countleading8({{src}}, {{zero_is_undef}}) + ::LibIntrinsics.countleading8({{src}}, {{zero_is_undef}}) end macro countleading16(src, zero_is_undef) - LibIntrinsics.countleading16({{src}}, {{zero_is_undef}}) + ::LibIntrinsics.countleading16({{src}}, {{zero_is_undef}}) end macro countleading32(src, zero_is_undef) - LibIntrinsics.countleading32({{src}}, {{zero_is_undef}}) + ::LibIntrinsics.countleading32({{src}}, {{zero_is_undef}}) end macro countleading64(src, zero_is_undef) - LibIntrinsics.countleading64({{src}}, {{zero_is_undef}}) + ::LibIntrinsics.countleading64({{src}}, {{zero_is_undef}}) end macro countleading128(src, zero_is_undef) - LibIntrinsics.countleading128({{src}}, {{zero_is_undef}}) + ::LibIntrinsics.countleading128({{src}}, {{zero_is_undef}}) end macro counttrailing8(src, zero_is_undef) - LibIntrinsics.counttrailing8({{src}}, {{zero_is_undef}}) + ::LibIntrinsics.counttrailing8({{src}}, {{zero_is_undef}}) end macro counttrailing16(src, zero_is_undef) - LibIntrinsics.counttrailing16({{src}}, {{zero_is_undef}}) + ::LibIntrinsics.counttrailing16({{src}}, {{zero_is_undef}}) end macro counttrailing32(src, zero_is_undef) - LibIntrinsics.counttrailing32({{src}}, {{zero_is_undef}}) + ::LibIntrinsics.counttrailing32({{src}}, {{zero_is_undef}}) end macro counttrailing64(src, zero_is_undef) - LibIntrinsics.counttrailing64({{src}}, {{zero_is_undef}}) + ::LibIntrinsics.counttrailing64({{src}}, {{zero_is_undef}}) end macro counttrailing128(src, zero_is_undef) - LibIntrinsics.counttrailing128({{src}}, {{zero_is_undef}}) + ::LibIntrinsics.counttrailing128({{src}}, {{zero_is_undef}}) end def self.fshl8(a, b, count) : UInt8 @@ -343,14 +343,14 @@ module Intrinsics end macro va_start(ap) - LibIntrinsics.va_start({{ap}}) + ::LibIntrinsics.va_start({{ap}}) end macro va_end(ap) - LibIntrinsics.va_end({{ap}}) + ::LibIntrinsics.va_end({{ap}}) end end macro debugger - Intrinsics.debugtrap + ::Intrinsics.debugtrap end diff --git a/src/json/serialization.cr b/src/json/serialization.cr index b1eb86d15082..15d948f02f40 100644 --- a/src/json/serialization.cr +++ b/src/json/serialization.cr @@ -164,7 +164,7 @@ module JSON private def self.new_from_json_pull_parser(pull : ::JSON::PullParser) instance = allocate instance.initialize(__pull_for_json_serializable: pull) - GC.add_finalizer(instance) if instance.responds_to?(:finalize) + ::GC.add_finalizer(instance) if instance.responds_to?(:finalize) instance end @@ -422,8 +422,8 @@ module JSON # Try to find the discriminator while also getting the raw # string value of the parsed JSON, so then we can pass it # to the final type. - json = String.build do |io| - JSON.build(io) do |builder| + json = ::String.build do |io| + ::JSON.build(io) do |builder| builder.start_object pull.read_object do |key| if key == {{field.id.stringify}} diff --git a/src/number.cr b/src/number.cr index f7c82aa4cded..9d955c065df3 100644 --- a/src/number.cr +++ b/src/number.cr @@ -59,7 +59,7 @@ struct Number # :nodoc: macro expand_div(rhs_types, result_type) {% for rhs in rhs_types %} - @[AlwaysInline] + @[::AlwaysInline] def /(other : {{rhs}}) : {{result_type}} {{result_type}}.new(self) / {{result_type}}.new(other) end @@ -84,7 +84,7 @@ struct Number # [1, 2, 3, 4] of Int64 # : Array(Int64) # ``` macro [](*nums) - Array({{@type}}).build({{nums.size}}) do |%buffer| + ::Array({{@type}}).build({{nums.size}}) do |%buffer| {% for num, i in nums %} %buffer[{{i}}] = {{@type}}.new({{num}}) {% end %} @@ -113,7 +113,7 @@ struct Number # Slice[1_i64, 2_i64, 3_i64, 4_i64] # : Slice(Int64) # ``` macro slice(*nums, read_only = false) - %slice = Slice({{@type}}).new({{nums.size}}, read_only: {{read_only}}) + %slice = ::Slice({{@type}}).new({{nums.size}}, read_only: {{read_only}}) {% for num, i in nums %} %slice.to_unsafe[{{i}}] = {{@type}}.new!({{num}}) {% end %} @@ -139,7 +139,7 @@ struct Number # StaticArray[1_i64, 2_i64, 3_i64, 4_i64] # : StaticArray(Int64) # ``` macro static_array(*nums) - %array = uninitialized StaticArray({{@type}}, {{nums.size}}) + %array = uninitialized ::StaticArray({{@type}}, {{nums.size}}) {% for num, i in nums %} %array.to_unsafe[{{i}}] = {{@type}}.new!({{num}}) {% end %} diff --git a/src/object.cr b/src/object.cr index ba818ac2979e..800736687788 100644 --- a/src/object.cr +++ b/src/object.cr @@ -562,7 +562,7 @@ class Object def {{method_prefix}}\{{name.var.id}} : \{{name.type}} if (value = {{var_prefix}}\{{name.var.id}}).nil? - ::raise NilAssertionError.new("\{{@type}}\{{"{{doc_prefix}}".id}}\{{name.var.id}} cannot be nil") + ::raise ::NilAssertionError.new("\{{@type}}\{{"{{doc_prefix}}".id}}\{{name.var.id}} cannot be nil") else value end @@ -574,7 +574,7 @@ class Object def {{method_prefix}}\{{name.id}} if (value = {{var_prefix}}\{{name.id}}).nil? - ::raise NilAssertionError.new("\{{@type}}\{{"{{doc_prefix}}".id}}\{{name.id}} cannot be nil") + ::raise ::NilAssertionError.new("\{{@type}}\{{"{{doc_prefix}}".id}}\{{name.id}} cannot be nil") else value end @@ -1293,7 +1293,7 @@ class Object # wrapper.capitalize # => "Hello" # ``` macro delegate(*methods, to object) - {% if compare_versions(Crystal::VERSION, "1.12.0-dev") >= 0 %} + {% if compare_versions(::Crystal::VERSION, "1.12.0-dev") >= 0 %} {% eq_operators = %w(<= >= == != []= ===) %} {% for method in methods %} {% if method.id.ends_with?('=') && !eq_operators.includes?(method.id.stringify) %} @@ -1427,18 +1427,18 @@ class Object macro def_clone # Returns a copy of `self` with all instance variables cloned. def clone - \{% if @type < Reference && !@type.instance_vars.map(&.type).all? { |t| t == ::Bool || t == ::Char || t == ::Symbol || t == ::String || t < ::Number::Primitive } %} + \{% if @type < ::Reference && !@type.instance_vars.map(&.type).all? { |t| t == ::Bool || t == ::Char || t == ::Symbol || t == ::String || t < ::Number::Primitive } %} exec_recursive_clone do |hash| clone = \{{@type}}.allocate hash[object_id] = clone.object_id clone.initialize_copy(self) - GC.add_finalizer(clone) if clone.responds_to?(:finalize) + ::GC.add_finalizer(clone) if clone.responds_to?(:finalize) clone end \{% else %} clone = \{{@type}}.allocate clone.initialize_copy(self) - GC.add_finalizer(clone) if clone.responds_to?(:finalize) + ::GC.add_finalizer(clone) if clone.responds_to?(:finalize) clone \{% end %} end diff --git a/src/slice.cr b/src/slice.cr index c87816f315d9..ace008e53e05 100644 --- a/src/slice.cr +++ b/src/slice.cr @@ -34,14 +34,14 @@ struct Slice(T) macro [](*args, read_only = false) # TODO: there should be a better way to check this, probably # asking if @type was instantiated or if T is defined - {% if @type.name != "Slice(T)" && T < Number %} + {% if @type.name != "Slice(T)" && T < ::Number %} {{T}}.slice({{args.splat(", ")}}read_only: {{read_only}}) {% else %} - %ptr = Pointer(typeof({{args.splat}})).malloc({{args.size}}) + %ptr = ::Pointer(typeof({{args.splat}})).malloc({{args.size}}) {% for arg, i in args %} %ptr[{{i}}] = {{arg}} {% end %} - Slice.new(%ptr, {{args.size}}, read_only: {{read_only}}) + ::Slice.new(%ptr, {{args.size}}, read_only: {{read_only}}) {% end %} end diff --git a/src/spec/dsl.cr b/src/spec/dsl.cr index 578076b86d69..d712aa59da4f 100644 --- a/src/spec/dsl.cr +++ b/src/spec/dsl.cr @@ -298,8 +298,8 @@ module Spec # If the "log" module is required it is configured to emit no entries by default. def log_setup defined?(::Log) do - if Log.responds_to?(:setup) - Log.setup_from_env(default_level: :none) + if ::Log.responds_to?(:setup) + ::Log.setup_from_env(default_level: :none) end end end diff --git a/src/spec/helpers/iterate.cr b/src/spec/helpers/iterate.cr index be302ebb49c2..7a70f83408ca 100644 --- a/src/spec/helpers/iterate.cr +++ b/src/spec/helpers/iterate.cr @@ -47,7 +47,7 @@ module Spec::Methods # See `.it_iterates` for details. macro assert_iterates_yielding(expected, method, *, infinite = false, tuple = false) %remaining = ({{expected}}).size - %ary = [] of typeof(Enumerable.element_type({{ expected }})) + %ary = [] of typeof(::Enumerable.element_type({{ expected }})) {{ method.id }} do |{% if tuple %}*{% end %}x| if %remaining == 0 if {{ infinite }} @@ -73,11 +73,11 @@ module Spec::Methods # # See `.it_iterates` for details. macro assert_iterates_iterator(expected, method, *, infinite = false) - %ary = [] of typeof(Enumerable.element_type({{ expected }})) + %ary = [] of typeof(::Enumerable.element_type({{ expected }})) %iter = {{ method.id }} ({{ expected }}).size.times do %v = %iter.next - if %v.is_a?(Iterator::Stop) + if %v.is_a?(::Iterator::Stop) # Compare the actual value directly. Since there are less # then expected values, the expectation will fail and raise. %ary.should eq({{ expected }}) @@ -86,7 +86,7 @@ module Spec::Methods %ary << %v end unless {{ infinite }} - %iter.next.should be_a(Iterator::Stop) + %iter.next.should be_a(::Iterator::Stop) end %ary.should eq({{ expected }}) diff --git a/src/static_array.cr b/src/static_array.cr index 2c09e21df166..3d00705bc21a 100644 --- a/src/static_array.cr +++ b/src/static_array.cr @@ -50,7 +50,7 @@ struct StaticArray(T, N) # * `Number.static_array` is a convenient alternative for designating a # specific numerical item type. macro [](*args) - %array = uninitialized StaticArray(typeof({{args.splat}}), {{args.size}}) + %array = uninitialized ::StaticArray(typeof({{args.splat}}), {{args.size}}) {% for arg, i in args %} %array.to_unsafe[{{i}}] = {{arg}} {% end %} diff --git a/src/syscall/aarch64-linux.cr b/src/syscall/aarch64-linux.cr index 5a61e8e7eed8..77b891fe2a7c 100644 --- a/src/syscall/aarch64-linux.cr +++ b/src/syscall/aarch64-linux.cr @@ -334,7 +334,7 @@ module Syscall end macro def_syscall(name, return_type, *args) - @[AlwaysInline] + @[::AlwaysInline] def self.{{name.id}}({{args.splat}}) : {{return_type}} ret = uninitialized {{return_type}} diff --git a/src/syscall/arm-linux.cr b/src/syscall/arm-linux.cr index 97119fc4b3f3..da349dd45301 100644 --- a/src/syscall/arm-linux.cr +++ b/src/syscall/arm-linux.cr @@ -409,7 +409,7 @@ module Syscall end macro def_syscall(name, return_type, *args) - @[AlwaysInline] + @[::AlwaysInline] def self.{{name.id}}({{args.splat}}) : {{return_type}} ret = uninitialized {{return_type}} diff --git a/src/syscall/i386-linux.cr b/src/syscall/i386-linux.cr index 843b2d1fd856..a0f94a51160a 100644 --- a/src/syscall/i386-linux.cr +++ b/src/syscall/i386-linux.cr @@ -445,7 +445,7 @@ module Syscall end macro def_syscall(name, return_type, *args) - @[AlwaysInline] + @[::AlwaysInline] def self.{{name.id}}({{args.splat}}) : {{return_type}} ret = uninitialized {{return_type}} diff --git a/src/syscall/x86_64-linux.cr b/src/syscall/x86_64-linux.cr index 1f01c9226658..5a63b6ee2e1a 100644 --- a/src/syscall/x86_64-linux.cr +++ b/src/syscall/x86_64-linux.cr @@ -368,7 +368,7 @@ module Syscall end macro def_syscall(name, return_type, *args) - @[AlwaysInline] + @[::AlwaysInline] def self.{{name.id}}({{args.splat}}) : {{return_type}} ret = uninitialized {{return_type}} diff --git a/src/uri/params/serializable.cr b/src/uri/params/serializable.cr index c0d766e85242..54d3b970e53c 100644 --- a/src/uri/params/serializable.cr +++ b/src/uri/params/serializable.cr @@ -59,19 +59,19 @@ struct URI::Params # ``` module Serializable macro included - def self.from_www_form(params : String) - new_from_www_form URI::Params.parse params + def self.from_www_form(params : ::String) + new_from_www_form ::URI::Params.parse params end # :nodoc: # # This is needed so that nested types can pass the name thru internally. # Has to be public so the generated code can call it, but should be considered an implementation detail. - def self.from_www_form(params : ::URI::Params, name : String) + def self.from_www_form(params : ::URI::Params, name : ::String) new_from_www_form(params, name) end - protected def self.new_from_www_form(params : ::URI::Params, name : String? = nil) + protected def self.new_from_www_form(params : ::URI::Params, name : ::String? = nil) instance = allocate instance.initialize(__uri_params: params, name: name) GC.add_finalizer(instance) if instance.responds_to?(:finalize) @@ -79,12 +79,12 @@ struct URI::Params end macro inherited - def self.from_www_form(params : String) - new_from_www_form URI::Params.parse params + def self.from_www_form(params : ::String) + new_from_www_form ::URI::Params.parse params end # :nodoc: - def self.from_www_form(params : ::URI::Params, name : String) + def self.from_www_form(params : ::URI::Params, name : ::String) new_from_www_form(params, name) end end diff --git a/src/yaml/serialization.cr b/src/yaml/serialization.cr index d5fae8dfe9c0..4a1521469dea 100644 --- a/src/yaml/serialization.cr +++ b/src/yaml/serialization.cr @@ -156,11 +156,11 @@ module YAML # Define a `new` directly in the included type, # so it overloads well with other possible initializes - def self.new(ctx : YAML::ParseContext, node : YAML::Nodes::Node) + def self.new(ctx : ::YAML::ParseContext, node : ::YAML::Nodes::Node) new_from_yaml_node(ctx, node) end - private def self.new_from_yaml_node(ctx : YAML::ParseContext, node : YAML::Nodes::Node) + private def self.new_from_yaml_node(ctx : ::YAML::ParseContext, node : ::YAML::Nodes::Node) ctx.read_alias(node, self) do |obj| return obj end @@ -170,7 +170,7 @@ module YAML ctx.record_anchor(node, instance) instance.initialize(__context_for_yaml_serializable: ctx, __node_for_yaml_serializable: node) - GC.add_finalizer(instance) if instance.responds_to?(:finalize) + ::GC.add_finalizer(instance) if instance.responds_to?(:finalize) instance end @@ -178,7 +178,7 @@ module YAML # so it can compete with other possible initializes macro inherited - def self.new(ctx : YAML::ParseContext, node : YAML::Nodes::Node) + def self.new(ctx : ::YAML::ParseContext, node : ::YAML::Nodes::Node) new_from_yaml_node(ctx, node) end end @@ -409,17 +409,17 @@ module YAML {% mapping.raise "Mapping argument must be a HashLiteral or a NamedTupleLiteral, not #{mapping.class_name.id}" %} {% end %} - def self.new(ctx : YAML::ParseContext, node : YAML::Nodes::Node) + def self.new(ctx : ::YAML::ParseContext, node : ::YAML::Nodes::Node) ctx.read_alias(node, \{{@type}}) do |obj| return obj end - unless node.is_a?(YAML::Nodes::Mapping) + unless node.is_a?(::YAML::Nodes::Mapping) node.raise "Expected YAML mapping, not #{node.class}" end node.each do |key, value| - next unless key.is_a?(YAML::Nodes::Scalar) && value.is_a?(YAML::Nodes::Scalar) + next unless key.is_a?(::YAML::Nodes::Scalar) && value.is_a?(::YAML::Nodes::Scalar) next unless key.value == {{field.id.stringify}} discriminator_value = value.value From 9240f50795ebfc3f52d85a07d546acf173dc6379 Mon Sep 17 00:00:00 2001 From: Quinton Miller Date: Fri, 6 Sep 2024 18:37:06 +0800 Subject: [PATCH 09/36] Fix exponent wrapping in `Math.frexp(BigFloat)` for very large values (#14971) `BigFloat`s represent their base-`256 ** sizeof(LibGMP::MpLimb)` exponent with a `LibGMP::MpExp` field, but `LibGMP.mpf_get_d_2exp` only returns the base-2 exponent as a `LibC::Long`, so values outside `(2.0.to_big_f ** -0x80000001)...(2.0.to_big_f ** 0x7FFFFFFF)` lead to an exponent overflow on Windows or 32-bit platforms: ```crystal require "big" Math.frexp(2.0.to_big_f ** 0xFFFFFFF5) # => {1.55164027193164307015e+1292913986, -10} Math.frexp(2.0.to_big_f ** -0xFFFFFFF4) # => {1.61119819150333097422e-1292913987, 13} Math.frexp(2.0.to_big_f ** 0x7FFFFFFF) # raises OverflowError ``` This patch fixes it by computing the exponent ourselves. --- spec/std/big/big_float_spec.cr | 16 +++++++++++++ src/big/big_float.cr | 41 +++++++++++++--------------------- 2 files changed, 32 insertions(+), 25 deletions(-) diff --git a/spec/std/big/big_float_spec.cr b/spec/std/big/big_float_spec.cr index 08d7e93bfb0b..73c6bcf06de8 100644 --- a/spec/std/big/big_float_spec.cr +++ b/spec/std/big/big_float_spec.cr @@ -548,7 +548,23 @@ end describe "BigFloat Math" do it ".frexp" do + Math.frexp(0.to_big_f).should eq({0.0, 0}) + Math.frexp(1.to_big_f).should eq({0.5, 1}) Math.frexp(0.2.to_big_f).should eq({0.8, -2}) + Math.frexp(2.to_big_f ** 63).should eq({0.5, 64}) + Math.frexp(2.to_big_f ** 64).should eq({0.5, 65}) + Math.frexp(2.to_big_f ** 200).should eq({0.5, 201}) + Math.frexp(2.to_big_f ** -200).should eq({0.5, -199}) + Math.frexp(2.to_big_f ** 0x7FFFFFFF).should eq({0.5, 0x80000000}) + Math.frexp(2.to_big_f ** 0x80000000).should eq({0.5, 0x80000001}) + Math.frexp(2.to_big_f ** 0xFFFFFFFF).should eq({0.5, 0x100000000}) + Math.frexp(1.75 * 2.to_big_f ** 0x123456789).should eq({0.875, 0x12345678A}) + Math.frexp(2.to_big_f ** -0x80000000).should eq({0.5, -0x7FFFFFFF}) + Math.frexp(2.to_big_f ** -0x80000001).should eq({0.5, -0x80000000}) + Math.frexp(2.to_big_f ** -0x100000000).should eq({0.5, -0xFFFFFFFF}) + Math.frexp(1.75 * 2.to_big_f ** -0x123456789).should eq({0.875, -0x123456788}) + Math.frexp(-(2.to_big_f ** 0x7FFFFFFF)).should eq({-0.5, 0x80000000}) + Math.frexp(-(2.to_big_f ** -0x100000000)).should eq({-0.5, -0xFFFFFFFF}) end it ".sqrt" do diff --git a/src/big/big_float.cr b/src/big/big_float.cr index 2c567f21eec9..ff78b7de8290 100644 --- a/src/big/big_float.cr +++ b/src/big/big_float.cr @@ -537,15 +537,24 @@ end module Math # Decomposes the given floating-point *value* into a normalized fraction and an integral power of two. def frexp(value : BigFloat) : {BigFloat, Int64} - LibGMP.mpf_get_d_2exp(out exp, value) # we need BigFloat frac, so will skip Float64 one. + return {BigFloat.zero, 0_i64} if value.zero? + + # We compute this ourselves since `LibGMP.mpf_get_d_2exp` only returns a + # `LibC::Long` exponent, which is not sufficient for 32-bit `LibC::Long` and + # 32-bit `LibGMP::MpExp`, e.g. on 64-bit Windows. + # Since `0.5 <= frac.abs < 1.0`, the radix point should be just above the + # most significant limb, and there should be no leading zeros in that limb. + leading_zeros = value.@mpf._mp_d[value.@mpf._mp_size.abs - 1].leading_zeros_count + exp = 8_i64 * sizeof(LibGMP::MpLimb) * value.@mpf._mp_exp - leading_zeros + frac = BigFloat.new do |mpf| - if exp >= 0 - LibGMP.mpf_div_2exp(mpf, value, exp) - else - LibGMP.mpf_mul_2exp(mpf, value, -exp) - end + # remove leading zeros in the most significant limb + LibGMP.mpf_mul_2exp(mpf, value, leading_zeros) + # reset the exponent manually + mpf.value._mp_exp = 0 end - {frac, exp.to_i64} + + {frac, exp} end # Calculates the square root of *value*. @@ -559,21 +568,3 @@ module Math BigFloat.new { |mpf| LibGMP.mpf_sqrt(mpf, value) } end end - -# :nodoc: -struct Crystal::Hasher - def self.reduce_num(value : BigFloat) - float_normalize_wrap(value) do |value| - # more exact version of `Math.frexp` - LibGMP.mpf_get_d_2exp(out exp, value) - frac = BigFloat.new do |mpf| - if exp >= 0 - LibGMP.mpf_div_2exp(mpf, value, exp) - else - LibGMP.mpf_mul_2exp(mpf, value, -exp) - end - end - float_normalize_reference(value, frac, exp) - end - end -end From 85d1ae78670b756e47c544ce34d6022ea0b6b4d1 Mon Sep 17 00:00:00 2001 From: Julien Portalier Date: Fri, 6 Sep 2024 12:37:53 +0200 Subject: [PATCH 10/36] Fix: `Crystal::SpinLock` doesn't need to be allocated on the HEAP (#14972) The abstraction is a mere abstraction over an atomic integer and the object itself are only ever used internally of other objects, with the exception of Channel where the code explicitely accesses the ivar directly (thus not making copies). We can avoid a HEAP allocation everywhere we use them (i.e. in lots of places). --- src/crystal/spin_lock.cr | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/crystal/spin_lock.cr b/src/crystal/spin_lock.cr index 4255fcae7bbd..105c235e0c66 100644 --- a/src/crystal/spin_lock.cr +++ b/src/crystal/spin_lock.cr @@ -1,5 +1,5 @@ # :nodoc: -class Crystal::SpinLock +struct Crystal::SpinLock private UNLOCKED = 0 private LOCKED = 1 From 025f3e041693882790d8941a8ecbd989715645cb Mon Sep 17 00:00:00 2001 From: Julien Portalier Date: Fri, 6 Sep 2024 12:39:25 +0200 Subject: [PATCH 11/36] Fix: `#file_descriptor_close` should set `@closed` (UNIX) (#14973) Prevents the GC from trying to cleanup resources that had already been closed in signal/process pipes. --- src/crystal/system/unix/file_descriptor.cr | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/src/crystal/system/unix/file_descriptor.cr b/src/crystal/system/unix/file_descriptor.cr index 56a9eee80dd5..759802f4323e 100644 --- a/src/crystal/system/unix/file_descriptor.cr +++ b/src/crystal/system/unix/file_descriptor.cr @@ -121,6 +121,13 @@ module Crystal::System::FileDescriptor end def file_descriptor_close(&) : Nil + # It would usually be set by IO::Buffered#unbuffered_close but we sometimes + # close file descriptors directly (i.e. signal/process pipes) and the IO + # object wouldn't be marked as closed, leading IO::FileDescriptor#finalize + # to try to close the fd again (pointless) and lead to other issues if we + # try to do more cleanup in the finalizer (error) + @closed = true + # Clear the @volatile_fd before actually closing it in order to # reduce the chance of reading an outdated fd value _fd = @volatile_fd.swap(-1) From cf15fb2dfbd9cf92aff2f191a136c1a1c12013ca Mon Sep 17 00:00:00 2001 From: "Brian J. Cardiff" Date: Fri, 6 Sep 2024 08:59:02 -0300 Subject: [PATCH 12/36] Adds initial support for external commands (#14953) --- spec/primitives/external_command_spec.cr | 34 ++++++++++++++++++++++++ src/compiler/crystal/command.cr | 3 +++ 2 files changed, 37 insertions(+) create mode 100644 spec/primitives/external_command_spec.cr diff --git a/spec/primitives/external_command_spec.cr b/spec/primitives/external_command_spec.cr new file mode 100644 index 000000000000..91687f7c2d21 --- /dev/null +++ b/spec/primitives/external_command_spec.cr @@ -0,0 +1,34 @@ +{% skip_file if flag?(:interpreted) %} + +require "../spec_helper" + +describe Crystal::Command do + it "exec external commands", tags: %w[slow] do + with_temp_executable "crystal-external" do |path| + with_tempfile "crystal-external.cr" do |source_file| + File.write source_file, <<-CRYSTAL + puts ENV["CRYSTAL"]? + puts PROGRAM_NAME + puts ARGV + CRYSTAL + + Process.run(ENV["CRYSTAL_SPEC_COMPILER_BIN"]? || "bin/crystal", ["build", source_file, "-o", path]) + end + + File.exists?(path).should be_true + + process = Process.new(ENV["CRYSTAL_SPEC_COMPILER_BIN"]? || "bin/crystal", + ["external", "foo", "bar"], + output: :pipe, + env: {"PATH" => {ENV["PATH"], File.dirname(path)}.join(Process::PATH_DELIMITER)} + ) + output = process.output.gets_to_end + status = process.wait + status.success?.should be_true + lines = output.lines + lines[0].should match /crystal/ + lines[1].should match /crystal-external/ + lines[2].should eq %(["foo", "bar"]) + end + end +end diff --git a/src/compiler/crystal/command.cr b/src/compiler/crystal/command.cr index f8ece87e3d4b..1354594706fb 100644 --- a/src/compiler/crystal/command.cr +++ b/src/compiler/crystal/command.cr @@ -130,6 +130,9 @@ class Crystal::Command else if command.ends_with?(".cr") error "file '#{command}' does not exist" + elsif external_command = Process.find_executable("crystal-#{command}") + options.shift + Process.exec(external_command, options, env: {"CRYSTAL" => Process.executable_path}) else error "unknown command: #{command}" end From ef306fb5aba4a03c497317ae7bf9f5da0c7a3549 Mon Sep 17 00:00:00 2001 From: Julien Portalier Date: Fri, 6 Sep 2024 18:13:08 +0200 Subject: [PATCH 13/36] Fix: reinit event loop first after fork (UNIX) (#14975) Signal handling manipulate pipes (file descriptors) on UNIX, and messing with the evloop after fork can affect the parent process evloop in some cases. --- src/kernel.cr | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/src/kernel.cr b/src/kernel.cr index 8c84a197b78f..14e66bd4fade 100644 --- a/src/kernel.cr +++ b/src/kernel.cr @@ -584,14 +584,14 @@ end # Hooks are defined here due to load order problems. def self.after_fork_child_callbacks @@after_fork_child_callbacks ||= [ - # clean ups (don't depend on event loop): + # reinit event loop first: + ->{ Crystal::EventLoop.current.after_fork }, + + # reinit signal handling: ->Crystal::System::Signal.after_fork, ->Crystal::System::SignalChildHandler.after_fork, - # reinit event loop: - ->{ Crystal::EventLoop.current.after_fork }, - - # more clean ups (may depend on event loop): + # additional reinitialization ->Random::DEFAULT.new_seed, ] of -> Nil end From 136f85ede8c7a3985eff2e4a0871f3645876e36f Mon Sep 17 00:00:00 2001 From: Julien Portalier Date: Fri, 6 Sep 2024 18:13:49 +0200 Subject: [PATCH 14/36] Don't involve evloop after fork in System::Process.spawn (UNIX) (#14974) Refactors spawning child processes on UNIX that relies on fork/exec to not involve the event loop after fork and before exec. We still continue to rely on the eventloop in the parent process, of course. * Extract Crystal::System::FileDescriptor.system_pipe (UNIX) * Fix: avoid evloop after fork to report failures in Process.spawn (UNIX) * Fix: don't involve evloop in System::Process.reopen_io (UNIX) This is called after fork before exec to reopen the stdio. We can leverage some abstractions (set blocking, unset cloexec) but musn't call to methods that involve the evloop and would mess with the parent evloop. --- src/crystal/system/unix/file_descriptor.cr | 28 ++++++++++-- src/crystal/system/unix/process.cr | 53 ++++++++++++---------- 2 files changed, 52 insertions(+), 29 deletions(-) diff --git a/src/crystal/system/unix/file_descriptor.cr b/src/crystal/system/unix/file_descriptor.cr index 759802f4323e..60515b701136 100644 --- a/src/crystal/system/unix/file_descriptor.cr +++ b/src/crystal/system/unix/file_descriptor.cr @@ -203,6 +203,14 @@ module Crystal::System::FileDescriptor end def self.pipe(read_blocking, write_blocking) + pipe_fds = system_pipe + r = IO::FileDescriptor.new(pipe_fds[0], read_blocking) + w = IO::FileDescriptor.new(pipe_fds[1], write_blocking) + w.sync = true + {r, w} + end + + def self.system_pipe : StaticArray(LibC::Int, 2) pipe_fds = uninitialized StaticArray(LibC::Int, 2) {% if LibC.has_method?(:pipe2) %} @@ -219,11 +227,7 @@ module Crystal::System::FileDescriptor end {% end %} - r = IO::FileDescriptor.new(pipe_fds[0], read_blocking) - w = IO::FileDescriptor.new(pipe_fds[1], write_blocking) - w.sync = true - - {r, w} + pipe_fds end def self.pread(file, buffer, offset) @@ -255,6 +259,20 @@ module Crystal::System::FileDescriptor io end + # Helper to write *size* values at *pointer* to a given *fd*. + def self.write_fully(fd : LibC::Int, pointer : Pointer, size : Int32 = 1) : Nil + write_fully(fd, Slice.new(pointer, size).unsafe_slice_of(UInt8)) + end + + # Helper to fully write a slice to a given *fd*. + def self.write_fully(fd : LibC::Int, slice : Slice(UInt8)) : Nil + until slice.size == 0 + size = LibC.write(fd, slice, slice.size) + break if size == -1 + slice += size + end + end + private def system_echo(enable : Bool, mode = nil) new_mode = mode || FileDescriptor.tcgetattr(fd) flags = LibC::ECHO | LibC::ECHOE | LibC::ECHOK | LibC::ECHONL diff --git a/src/crystal/system/unix/process.cr b/src/crystal/system/unix/process.cr index 4a540fa53a3d..06b18aea7b1d 100644 --- a/src/crystal/system/unix/process.cr +++ b/src/crystal/system/unix/process.cr @@ -231,44 +231,47 @@ struct Crystal::System::Process end def self.spawn(command_args, env, clear_env, input, output, error, chdir) - reader_pipe, writer_pipe = IO.pipe + r, w = FileDescriptor.system_pipe pid = self.fork(will_exec: true) if !pid + LibC.close(r) begin - reader_pipe.close - writer_pipe.close_on_exec = true self.try_replace(command_args, env, clear_env, input, output, error, chdir) - writer_pipe.write_byte(1) - writer_pipe.write_bytes(Errno.value.to_i) + byte = 1_u8 + errno = Errno.value.to_i32 + FileDescriptor.write_fully(w, pointerof(byte)) + FileDescriptor.write_fully(w, pointerof(errno)) rescue ex - writer_pipe.write_byte(0) + byte = 0_u8 message = ex.inspect_with_backtrace - writer_pipe.write_bytes(message.bytesize) - writer_pipe << message - writer_pipe.close + FileDescriptor.write_fully(w, pointerof(byte)) + FileDescriptor.write_fully(w, message.to_slice) ensure + LibC.close(w) LibC._exit 127 end end - writer_pipe.close + LibC.close(w) + reader_pipe = IO::FileDescriptor.new(r, blocking: false) + begin case reader_pipe.read_byte when nil # Pipe was closed, no error when 0 # Error message coming - message_size = reader_pipe.read_bytes(Int32) - if message_size > 0 - message = String.build(message_size) { |io| IO.copy(reader_pipe, io, message_size) } - end - reader_pipe.close + message = reader_pipe.gets_to_end raise RuntimeError.new("Error executing process: '#{command_args[0]}': #{message}") when 1 # Errno coming - errno = Errno.new(reader_pipe.read_bytes(Int32)) - self.raise_exception_from_errno(command_args[0], errno) + # can't use IO#read_bytes(Int32) because we skipped system/network + # endianness check when writing the integer while read_bytes would; + # we thus read it in the same as order as written + buf = uninitialized StaticArray(UInt8, 4) + reader_pipe.read_fully(buf.to_slice) + raise_exception_from_errno(command_args[0], Errno.new(buf.unsafe_as(Int32))) else raise RuntimeError.new("BUG: Invalid error response received from subprocess") end @@ -339,15 +342,17 @@ struct Crystal::System::Process private def self.reopen_io(src_io : IO::FileDescriptor, dst_io : IO::FileDescriptor) if src_io.closed? - dst_io.close - return - end + dst_io.file_descriptor_close + else + src_io = to_real_fd(src_io) - src_io = to_real_fd(src_io) + # dst_io.reopen(src_io) + ret = LibC.dup2(src_io.fd, dst_io.fd) + raise IO::Error.from_errno("dup2") if ret == -1 - dst_io.reopen(src_io) - dst_io.blocking = true - dst_io.close_on_exec = false + dst_io.blocking = true + dst_io.close_on_exec = false + end end private def self.to_real_fd(fd : IO::FileDescriptor) From cdd9ccf460641ee17a603c67ff9d23aa1199f14f Mon Sep 17 00:00:00 2001 From: Quinton Miller Date: Sat, 7 Sep 2024 17:43:48 +0800 Subject: [PATCH 15/36] Enable the interpreter on Windows (#14964) --- .github/workflows/win.yml | 37 +++++++++++++++++++++++- .github/workflows/win_build_portable.yml | 2 +- spec/std/http/client/client_spec.cr | 6 ++++ spec/std/http/server/server_spec.cr | 6 ++++ spec/std/http/web_socket_spec.cr | 6 ++++ spec/std/io/io_spec.cr | 33 +++++++++++---------- spec/std/oauth2/client_spec.cr | 6 ++++ spec/std/openssl/ssl/server_spec.cr | 6 ++++ spec/std/openssl/ssl/socket_spec.cr | 6 ++++ spec/std/process_spec.cr | 7 ++++- spec/std/socket/socket_spec.cr | 6 ++++ spec/std/socket/tcp_socket_spec.cr | 6 ++++ spec/std/socket/unix_server_spec.cr | 6 ++++ spec/std/socket/unix_socket_spec.cr | 6 ++++ spec/std/uuid_spec.cr | 1 + src/compiler/crystal/loader/msvc.cr | 37 ++++++++++++++++++------ src/kernel.cr | 13 +++++++++ 17 files changed, 163 insertions(+), 27 deletions(-) diff --git a/.github/workflows/win.yml b/.github/workflows/win.yml index 05f74b6378c6..568828b17bee 100644 --- a/.github/workflows/win.yml +++ b/.github/workflows/win.yml @@ -7,6 +7,7 @@ concurrency: cancel-in-progress: ${{ github.ref != 'refs/heads/master' }} env: + SPEC_SPLIT_DOTS: 160 CI_LLVM_VERSION: "18.1.1" jobs: @@ -266,13 +267,47 @@ jobs: run: make -f Makefile.win samples x86_64-windows-release: - if: github.repository_owner == 'crystal-lang' && (startsWith(github.ref, 'refs/tags/') || startsWith(github.ref, 'refs/heads/ci/')) needs: [x86_64-windows-libs, x86_64-windows-dlls, x86_64-windows-llvm-libs, x86_64-windows-llvm-dlls] uses: ./.github/workflows/win_build_portable.yml with: release: true llvm_version: "18.1.1" + x86_64-windows-test-interpreter: + runs-on: windows-2022 + needs: [x86_64-windows-release] + steps: + - name: Disable CRLF line ending substitution + run: | + git config --global core.autocrlf false + + - name: Download Crystal source + uses: actions/checkout@v4 + + - name: Download Crystal executable + uses: actions/download-artifact@v4 + with: + name: crystal-release + path: build + + - name: Restore LLVM + uses: actions/cache/restore@v4 + with: + path: llvm + key: llvm-libs-${{ env.CI_LLVM_VERSION }}-msvc + fail-on-cache-miss: true + + - name: Set up environment + run: | + Add-Content $env:GITHUB_PATH "$(pwd)\build" + Add-Content $env:GITHUB_ENV "CRYSTAL_SPEC_COMPILER_BIN=$(pwd)\build\crystal.exe" + + - name: Run stdlib specs with interpreter + run: bin\crystal i spec\std_spec.cr + + - name: Run primitives specs with interpreter + run: bin\crystal i spec\primitives_spec.cr + x86_64-windows-installer: if: github.repository_owner == 'crystal-lang' && (startsWith(github.ref, 'refs/tags/') || startsWith(github.ref, 'refs/heads/ci/')) runs-on: windows-2022 diff --git a/.github/workflows/win_build_portable.yml b/.github/workflows/win_build_portable.yml index d2ed6469d264..12ee17da9e68 100644 --- a/.github/workflows/win_build_portable.yml +++ b/.github/workflows/win_build_portable.yml @@ -114,7 +114,7 @@ jobs: - name: Build Crystal run: | bin/crystal.bat env - make -f Makefile.win -B ${{ inputs.release && 'release=1' || '' }} + make -f Makefile.win -B ${{ inputs.release && 'release=1' || '' }} interpreter=1 - name: Download shards release uses: actions/checkout@v4 diff --git a/spec/std/http/client/client_spec.cr b/spec/std/http/client/client_spec.cr index 451960a8c79f..6bd04ab3e2f2 100644 --- a/spec/std/http/client/client_spec.cr +++ b/spec/std/http/client/client_spec.cr @@ -6,6 +6,12 @@ require "http/server" require "http/log" require "log/spec" +# TODO: Windows networking in the interpreter requires #12495 +{% if flag?(:interpreted) && flag?(:win32) %} + pending HTTP::Client + {% skip_file %} +{% end %} + private def test_server(host, port, read_time = 0.seconds, content_type = "text/plain", write_response = true, &) server = TCPServer.new(host, port) begin diff --git a/spec/std/http/server/server_spec.cr b/spec/std/http/server/server_spec.cr index 5e1e5dab76f6..3980084ea414 100644 --- a/spec/std/http/server/server_spec.cr +++ b/spec/std/http/server/server_spec.cr @@ -4,6 +4,12 @@ require "http/client" require "../../../support/ssl" require "../../../support/channel" +# TODO: Windows networking in the interpreter requires #12495 +{% if flag?(:interpreted) && flag?(:win32) %} + pending HTTP::Server + {% skip_file %} +{% end %} + # TODO: replace with `HTTP::Client.get` once it supports connecting to Unix socket (#2735) private def unix_request(path) UNIXSocket.open(path) do |io| diff --git a/spec/std/http/web_socket_spec.cr b/spec/std/http/web_socket_spec.cr index 75a54e91fb2e..164a1d067df5 100644 --- a/spec/std/http/web_socket_spec.cr +++ b/spec/std/http/web_socket_spec.cr @@ -7,6 +7,12 @@ require "../../support/fibers" require "../../support/ssl" require "../socket/spec_helper.cr" +# TODO: Windows networking in the interpreter requires #12495 +{% if flag?(:interpreted) && flag?(:win32) %} + pending HTTP::WebSocket + {% skip_file %} +{% end %} + private def assert_text_packet(packet, size, final = false) assert_packet packet, HTTP::WebSocket::Protocol::Opcode::TEXT, size, final: final end diff --git a/spec/std/io/io_spec.cr b/spec/std/io/io_spec.cr index 6974a9fe3466..c584ec81a1e8 100644 --- a/spec/std/io/io_spec.cr +++ b/spec/std/io/io_spec.cr @@ -816,23 +816,26 @@ describe IO do io.gets_to_end.should eq("\r\nFoo\nBar") end - it "gets ascii from socket (#9056)" do - server = TCPServer.new "localhost", 0 - sock = TCPSocket.new "localhost", server.local_address.port - begin - sock.set_encoding("ascii") - spawn do - client = server.accept - message = client.gets - client << "#{message}\n" + # TODO: Windows networking in the interpreter requires #12495 + {% unless flag?(:interpreted) || flag?(:win32) %} + it "gets ascii from socket (#9056)" do + server = TCPServer.new "localhost", 0 + sock = TCPSocket.new "localhost", server.local_address.port + begin + sock.set_encoding("ascii") + spawn do + client = server.accept + message = client.gets + client << "#{message}\n" + end + sock << "K\n" + sock.gets.should eq("K") + ensure + server.close + sock.close end - sock << "K\n" - sock.gets.should eq("K") - ensure - server.close - sock.close end - end + {% end %} end describe "encode" do diff --git a/spec/std/oauth2/client_spec.cr b/spec/std/oauth2/client_spec.cr index 3ee66e29ab49..ee445f3426e7 100644 --- a/spec/std/oauth2/client_spec.cr +++ b/spec/std/oauth2/client_spec.cr @@ -3,6 +3,12 @@ require "oauth2" require "http/server" require "../http/spec_helper" +# TODO: Windows networking in the interpreter requires #12495 +{% if flag?(:interpreted) && flag?(:win32) %} + pending OAuth2::Client + {% skip_file %} +{% end %} + describe OAuth2::Client do describe "authorization uri" do it "gets with default endpoint" do diff --git a/spec/std/openssl/ssl/server_spec.cr b/spec/std/openssl/ssl/server_spec.cr index 2e0e413a618d..8618ed780a50 100644 --- a/spec/std/openssl/ssl/server_spec.cr +++ b/spec/std/openssl/ssl/server_spec.cr @@ -3,6 +3,12 @@ require "socket" require "../../spec_helper" require "../../../support/ssl" +# TODO: Windows networking in the interpreter requires #12495 +{% if flag?(:interpreted) && flag?(:win32) %} + pending OpenSSL::SSL::Server + {% skip_file %} +{% end %} + describe OpenSSL::SSL::Server do it "sync_close" do TCPServer.open(0) do |tcp_server| diff --git a/spec/std/openssl/ssl/socket_spec.cr b/spec/std/openssl/ssl/socket_spec.cr index bbc5b11e4b9b..47374ce28cca 100644 --- a/spec/std/openssl/ssl/socket_spec.cr +++ b/spec/std/openssl/ssl/socket_spec.cr @@ -4,6 +4,12 @@ require "../../spec_helper" require "../../socket/spec_helper" require "../../../support/ssl" +# TODO: Windows networking in the interpreter requires #12495 +{% if flag?(:interpreted) && flag?(:win32) %} + pending OpenSSL::SSL::Socket + {% skip_file %} +{% end %} + describe OpenSSL::SSL::Socket do describe OpenSSL::SSL::Socket::Server do it "auto accept client by default" do diff --git a/spec/std/process_spec.cr b/spec/std/process_spec.cr index 57f90121c26b..d41ee0bed242 100644 --- a/spec/std/process_spec.cr +++ b/spec/std/process_spec.cr @@ -55,7 +55,12 @@ private def newline end # interpreted code doesn't receive SIGCHLD for `#wait` to work (#12241) -pending_interpreted describe: Process do +{% if flag?(:interpreted) && !flag?(:win32) %} + pending Process + {% skip_file %} +{% end %} + +describe Process do describe ".new" do it "raises if command doesn't exist" do expect_raises(File::NotFoundError, "Error executing process: 'foobarbaz'") do diff --git a/spec/std/socket/socket_spec.cr b/spec/std/socket/socket_spec.cr index 2127e196b746..98555937dea3 100644 --- a/spec/std/socket/socket_spec.cr +++ b/spec/std/socket/socket_spec.cr @@ -2,6 +2,12 @@ require "./spec_helper" require "../../support/tempfile" require "../../support/win32" +# TODO: Windows networking in the interpreter requires #12495 +{% if flag?(:interpreted) && flag?(:win32) %} + pending Socket + {% skip_file %} +{% end %} + describe Socket, tags: "network" do describe ".unix" do it "creates a unix socket" do diff --git a/spec/std/socket/tcp_socket_spec.cr b/spec/std/socket/tcp_socket_spec.cr index 68c00ccd2e79..f3d460f92401 100644 --- a/spec/std/socket/tcp_socket_spec.cr +++ b/spec/std/socket/tcp_socket_spec.cr @@ -3,6 +3,12 @@ require "./spec_helper" require "../../support/win32" +# TODO: Windows networking in the interpreter requires #12495 +{% if flag?(:interpreted) && flag?(:win32) %} + pending TCPSocket + {% skip_file %} +{% end %} + describe TCPSocket, tags: "network" do describe "#connect" do each_ip_family do |family, address| diff --git a/spec/std/socket/unix_server_spec.cr b/spec/std/socket/unix_server_spec.cr index ca364f08667c..60f0279b4091 100644 --- a/spec/std/socket/unix_server_spec.cr +++ b/spec/std/socket/unix_server_spec.cr @@ -4,6 +4,12 @@ require "../../support/fibers" require "../../support/channel" require "../../support/tempfile" +# TODO: Windows networking in the interpreter requires #12495 +{% if flag?(:interpreted) && flag?(:win32) %} + pending UNIXServer + {% skip_file %} +{% end %} + describe UNIXServer do describe ".new" do it "raises when path is too long" do diff --git a/spec/std/socket/unix_socket_spec.cr b/spec/std/socket/unix_socket_spec.cr index 24777bada67f..c51f37193c0e 100644 --- a/spec/std/socket/unix_socket_spec.cr +++ b/spec/std/socket/unix_socket_spec.cr @@ -2,6 +2,12 @@ require "spec" require "socket" require "../../support/tempfile" +# TODO: Windows networking in the interpreter requires #12495 +{% if flag?(:interpreted) && flag?(:win32) %} + pending UNIXSocket + {% skip_file %} +{% end %} + describe UNIXSocket do it "raises when path is too long" do with_tempfile("unix_socket-too_long-#{("a" * 2048)}.sock") do |path| diff --git a/spec/std/uuid_spec.cr b/spec/std/uuid_spec.cr index 48cc3351a3c6..5d7e627031f0 100644 --- a/spec/std/uuid_spec.cr +++ b/spec/std/uuid_spec.cr @@ -1,6 +1,7 @@ require "spec" require "uuid" require "spec/helpers/string" +require "../support/wasm32" describe "UUID" do describe "#==" do diff --git a/src/compiler/crystal/loader/msvc.cr b/src/compiler/crystal/loader/msvc.cr index 05bf988c9218..772e4c5c232f 100644 --- a/src/compiler/crystal/loader/msvc.cr +++ b/src/compiler/crystal/loader/msvc.cr @@ -133,15 +133,25 @@ class Crystal::Loader end def load_file?(path : String | ::Path) : Bool + # API sets shouldn't be linked directly from linker flags, but just in case + if api_set?(path) + return load_dll?(path.to_s) + end + return false unless File.file?(path) # On Windows, each `.lib` import library may reference any number of `.dll` # files, whose base names may not match the library's. Thus it is necessary # to extract this information from the library archive itself. - System::LibraryArchive.imported_dlls(path).each do |dll| - dll_full_path = @dll_search_paths.try &.each do |search_path| - full_path = File.join(search_path, dll) - break full_path if File.file?(full_path) + System::LibraryArchive.imported_dlls(path).all? do |dll| + # API set names do not refer to physical filenames despite ending with + # `.dll`, and therefore should not use a path search: + # https://learn.microsoft.com/en-us/cpp/windows/universal-crt-deployment?view=msvc-170#local-deployment + unless api_set?(dll) + dll_full_path = @dll_search_paths.try &.each do |search_path| + full_path = File.join(search_path, dll) + break full_path if File.file?(full_path) + end end dll = dll_full_path || dll @@ -152,13 +162,16 @@ class Crystal::Loader # # Note that the compiler's directory and PATH are effectively searched # twice when coming from the interpreter - handle = open_library(dll) - return false unless handle - - @handles << handle - @loaded_libraries << (module_filename(handle) || dll) + load_dll?(dll) end + end + + private def load_dll?(dll) + handle = open_library(dll) + return false unless handle + @handles << handle + @loaded_libraries << (module_filename(handle) || dll) true end @@ -190,6 +203,12 @@ class Crystal::Loader @handles.clear end + # Returns whether *dll* names an API set according to: + # https://learn.microsoft.com/en-us/windows/win32/apiindex/windows-apisets#api-set-contract-names + private def api_set?(dll) + dll.to_s.matches?(/^(?:api-|ext-)[a-zA-Z0-9-]*l\d+-\d+-\d+\.dll$/) + end + private def module_filename(handle) Crystal::System.retry_wstr_buffer do |buffer, small_buf| len = LibC.GetModuleFileNameW(handle, buffer, buffer.size) diff --git a/src/kernel.cr b/src/kernel.cr index 14e66bd4fade..16c4a770309a 100644 --- a/src/kernel.cr +++ b/src/kernel.cr @@ -616,3 +616,16 @@ end Crystal::System::Signal.setup_default_handlers {% end %} {% end %} + +# This is a temporary workaround to ensure there is always something in the IOCP +# event loop being awaited, since both the interrupt loop and the fiber stack +# pool collector are disabled in interpreted code. Without this, asynchronous +# code that bypasses `Crystal::IOCP::OverlappedOperation` does not currently +# work, see https://github.com/crystal-lang/crystal/pull/14949#issuecomment-2328314463 +{% if flag?(:interpreted) && flag?(:win32) %} + spawn(name: "Interpreter idle loop") do + while true + sleep 1.day + end + end +{% end %} From bdddae759a2e1f77306e1a54523a136a0b64c078 Mon Sep 17 00:00:00 2001 From: Julien Portalier Date: Mon, 9 Sep 2024 22:12:05 +0200 Subject: [PATCH 16/36] Add methods to Crystal::EventLoop (#14977) Add `#after_fork_before_exec` to allow an evloop to do some cleanup before exec (UNIX only). Add `#remove(io)` to allow an evloop to free resources when the IO is closed in a GC finalizer. --- src/crystal/scheduler.cr | 6 ++++++ src/crystal/system/event_loop.cr | 5 +++++ src/crystal/system/event_loop/file_descriptor.cr | 8 ++++++++ src/crystal/system/event_loop/socket.cr | 8 ++++++++ src/crystal/system/file_descriptor.cr | 4 ++++ src/crystal/system/socket.cr | 4 ++++ src/crystal/system/unix/event_loop_libevent.cr | 9 +++++++++ src/crystal/system/unix/process.cr | 3 +++ src/crystal/system/wasi/event_loop.cr | 6 ++++++ src/crystal/system/win32/event_loop_iocp.cr | 6 ++++++ src/io/file_descriptor.cr | 1 + src/socket.cr | 1 + 12 files changed, 61 insertions(+) diff --git a/src/crystal/scheduler.cr b/src/crystal/scheduler.cr index d3634e9aea6a..bed98ef4d05b 100644 --- a/src/crystal/scheduler.cr +++ b/src/crystal/scheduler.cr @@ -24,6 +24,12 @@ class Crystal::Scheduler Thread.current.scheduler.@event_loop end + def self.event_loop? + if scheduler = Thread.current?.try(&.scheduler?) + scheduler.@event_loop + end + end + def self.enqueue(fiber : Fiber) : Nil Crystal.trace :sched, "enqueue", fiber: fiber do thread = Thread.current diff --git a/src/crystal/system/event_loop.cr b/src/crystal/system/event_loop.cr index 46954e6034ff..fb1042b21f96 100644 --- a/src/crystal/system/event_loop.cr +++ b/src/crystal/system/event_loop.cr @@ -17,6 +17,11 @@ abstract class Crystal::EventLoop Crystal::Scheduler.event_loop end + @[AlwaysInline] + def self.current? : self? + Crystal::Scheduler.event_loop? + end + # Runs the loop. # # Returns immediately if events are activable. Set `blocking` to false to diff --git a/src/crystal/system/event_loop/file_descriptor.cr b/src/crystal/system/event_loop/file_descriptor.cr index a041263609d9..5fb6cbb95cb0 100644 --- a/src/crystal/system/event_loop/file_descriptor.cr +++ b/src/crystal/system/event_loop/file_descriptor.cr @@ -19,5 +19,13 @@ abstract class Crystal::EventLoop # Closes the file descriptor resource. abstract def close(file_descriptor : Crystal::System::FileDescriptor) : Nil + + # Removes the file descriptor from the event loop. Can be used to free up + # memory resources associated with the file descriptor, as well as removing + # the file descriptor from kernel data structures. + # + # Called by `::IO::FileDescriptor#finalize` before closing the file + # descriptor. Errors shall be silently ignored. + abstract def remove(file_descriptor : Crystal::System::FileDescriptor) : Nil end end diff --git a/src/crystal/system/event_loop/socket.cr b/src/crystal/system/event_loop/socket.cr index e6f35478b487..6309aed391e0 100644 --- a/src/crystal/system/event_loop/socket.cr +++ b/src/crystal/system/event_loop/socket.cr @@ -62,5 +62,13 @@ abstract class Crystal::EventLoop # Closes the socket. abstract def close(socket : ::Socket) : Nil + + # Removes the socket from the event loop. Can be used to free up memory + # resources associated with the socket, as well as removing the socket from + # kernel data structures. + # + # Called by `::Socket#finalize` before closing the socket. Errors shall be + # silently ignored. + abstract def remove(socket : ::Socket) : Nil end end diff --git a/src/crystal/system/file_descriptor.cr b/src/crystal/system/file_descriptor.cr index 481e00982e25..03868bc07034 100644 --- a/src/crystal/system/file_descriptor.cr +++ b/src/crystal/system/file_descriptor.cr @@ -39,6 +39,10 @@ module Crystal::System::FileDescriptor event_loop.write(self, slice) end + private def event_loop? : Crystal::EventLoop::FileDescriptor? + Crystal::EventLoop.current? + end + private def event_loop : Crystal::EventLoop::FileDescriptor Crystal::EventLoop.current end diff --git a/src/crystal/system/socket.cr b/src/crystal/system/socket.cr index 10f902e9f0c1..8d5e8c9afaf0 100644 --- a/src/crystal/system/socket.cr +++ b/src/crystal/system/socket.cr @@ -99,6 +99,10 @@ module Crystal::System::Socket # Also used in `Socket#finalize` # def socket_close + private def event_loop? : Crystal::EventLoop::Socket? + Crystal::EventLoop.current? + end + private def event_loop : Crystal::EventLoop::Socket Crystal::EventLoop.current end diff --git a/src/crystal/system/unix/event_loop_libevent.cr b/src/crystal/system/unix/event_loop_libevent.cr index b67bad63ff2f..4594f07ffe66 100644 --- a/src/crystal/system/unix/event_loop_libevent.cr +++ b/src/crystal/system/unix/event_loop_libevent.cr @@ -4,6 +4,9 @@ require "./event_libevent" class Crystal::LibEvent::EventLoop < Crystal::EventLoop private getter(event_base) { Crystal::LibEvent::Event::Base.new } + def after_fork_before_exec : Nil + end + {% unless flag?(:preview_mt) %} # Reinitializes the event loop after a fork. def after_fork : Nil @@ -93,6 +96,9 @@ class Crystal::LibEvent::EventLoop < Crystal::EventLoop file_descriptor.evented_close end + def remove(file_descriptor : Crystal::System::FileDescriptor) : Nil + end + def read(socket : ::Socket, slice : Bytes) : Int32 evented_read(socket, "Error reading socket") do LibC.recv(socket.fd, slice, slice.size, 0).to_i32 @@ -186,6 +192,9 @@ class Crystal::LibEvent::EventLoop < Crystal::EventLoop socket.evented_close end + def remove(socket : ::Socket) : Nil + end + def evented_read(target, errno_msg : String, &) : Int32 loop do bytes_read = yield diff --git a/src/crystal/system/unix/process.cr b/src/crystal/system/unix/process.cr index 06b18aea7b1d..420030f8ba53 100644 --- a/src/crystal/system/unix/process.cr +++ b/src/crystal/system/unix/process.cr @@ -185,6 +185,9 @@ struct Crystal::System::Process # child: pid = nil if will_exec + # notify event loop + Crystal::EventLoop.current.after_fork_before_exec + # reset signal handlers, then sigmask (inherited on exec): Crystal::System::Signal.after_fork_before_exec LibC.sigemptyset(pointerof(newmask)) diff --git a/src/crystal/system/wasi/event_loop.cr b/src/crystal/system/wasi/event_loop.cr index ba657b917154..c804c4be27aa 100644 --- a/src/crystal/system/wasi/event_loop.cr +++ b/src/crystal/system/wasi/event_loop.cr @@ -53,6 +53,9 @@ class Crystal::Wasi::EventLoop < Crystal::EventLoop file_descriptor.evented_close end + def remove(file_descriptor : Crystal::System::FileDescriptor) : Nil + end + def read(socket : ::Socket, slice : Bytes) : Int32 evented_read(socket, "Error reading socket") do LibC.recv(socket.fd, slice, slice.size, 0).to_i32 @@ -85,6 +88,9 @@ class Crystal::Wasi::EventLoop < Crystal::EventLoop socket.evented_close end + def remove(socket : ::Socket) : Nil + end + def evented_read(target, errno_msg : String, &) : Int32 loop do bytes_read = yield diff --git a/src/crystal/system/win32/event_loop_iocp.cr b/src/crystal/system/win32/event_loop_iocp.cr index d1aae09b680a..d3cfaf98d853 100644 --- a/src/crystal/system/win32/event_loop_iocp.cr +++ b/src/crystal/system/win32/event_loop_iocp.cr @@ -161,6 +161,9 @@ class Crystal::IOCP::EventLoop < Crystal::EventLoop LibC.CancelIoEx(file_descriptor.windows_handle, nil) unless file_descriptor.system_blocking? end + def remove(file_descriptor : Crystal::System::FileDescriptor) : Nil + end + private def wsa_buffer(bytes) wsabuf = LibC::WSABUF.new wsabuf.len = bytes.size @@ -271,6 +274,9 @@ class Crystal::IOCP::EventLoop < Crystal::EventLoop def close(socket : ::Socket) : Nil end + + def remove(socket : ::Socket) : Nil + end end class Crystal::IOCP::Event diff --git a/src/io/file_descriptor.cr b/src/io/file_descriptor.cr index 622229e43e00..a9b303b4b58c 100644 --- a/src/io/file_descriptor.cr +++ b/src/io/file_descriptor.cr @@ -255,6 +255,7 @@ class IO::FileDescriptor < IO def finalize return if closed? || !close_on_finalize? + event_loop?.try(&.remove(self)) file_descriptor_close { } # ignore error end diff --git a/src/socket.cr b/src/socket.cr index 1d367f805343..e97deea9eb04 100644 --- a/src/socket.cr +++ b/src/socket.cr @@ -430,6 +430,7 @@ class Socket < IO def finalize return if closed? + event_loop?.try(&.remove(self)) socket_close { } # ignore error end From 849e0d7ad61448eb5b9c9cbd7e1296f41d0f88f9 Mon Sep 17 00:00:00 2001 From: Quinton Miller Date: Tue, 10 Sep 2024 04:12:58 +0800 Subject: [PATCH 17/36] Make `Crystal::IOCP::OverlappedOperation` abstract (#14987) This allows different overlapped operations to provide their own closure data, instead of putting everything in one big class, such as in https://github.com/crystal-lang/crystal/pull/14979#discussion_r1746549086. --- src/crystal/system/win32/file_descriptor.cr | 4 +- src/crystal/system/win32/iocp.cr | 121 ++++++++++++-------- src/crystal/system/win32/socket.cr | 8 +- 3 files changed, 79 insertions(+), 54 deletions(-) diff --git a/src/crystal/system/win32/file_descriptor.cr b/src/crystal/system/win32/file_descriptor.cr index d4831d9528cb..cdd23e3ed54d 100644 --- a/src/crystal/system/win32/file_descriptor.cr +++ b/src/crystal/system/win32/file_descriptor.cr @@ -234,7 +234,7 @@ module Crystal::System::FileDescriptor end private def lock_file(handle, flags) - IOCP::OverlappedOperation.run(handle) do |operation| + IOCP::IOOverlappedOperation.run(handle) do |operation| result = LibC.LockFileEx(handle, flags, 0, 0xFFFF_FFFF, 0xFFFF_FFFF, operation) if result == 0 @@ -260,7 +260,7 @@ module Crystal::System::FileDescriptor end private def unlock_file(handle) - IOCP::OverlappedOperation.run(handle) do |operation| + IOCP::IOOverlappedOperation.run(handle) do |operation| result = LibC.UnlockFileEx(handle, 0, 0xFFFF_FFFF, 0xFFFF_FFFF, operation) if result == 0 diff --git a/src/crystal/system/win32/iocp.cr b/src/crystal/system/win32/iocp.cr index ba87ed123f22..384784a193db 100644 --- a/src/crystal/system/win32/iocp.cr +++ b/src/crystal/system/win32/iocp.cr @@ -78,39 +78,66 @@ module Crystal::IOCP end end - class OverlappedOperation + abstract class OverlappedOperation enum State STARTED DONE end + abstract def wait_for_result(timeout, & : WinError ->) + private abstract def try_cancel : Bool + @overlapped = LibC::OVERLAPPED.new @fiber = Fiber.current @state : State = :started - def initialize(@handle : LibC::HANDLE) + def self.run(*args, **opts, &) + operation_storage = uninitialized ReferenceStorage(self) + operation = unsafe_construct(pointerof(operation_storage), *args, **opts) + yield operation end - def initialize(handle : LibC::SOCKET) - @handle = LibC::HANDLE.new(handle) + def self.unbox(overlapped : LibC::OVERLAPPED*) : self + start = overlapped.as(Pointer(UInt8)) - offsetof(self, @overlapped) + Box(self).unbox(start.as(Pointer(Void))) end - def self.run(handle, &) - operation_storage = uninitialized ReferenceStorage(OverlappedOperation) - operation = OverlappedOperation.unsafe_construct(pointerof(operation_storage), handle) - yield operation + def to_unsafe + pointerof(@overlapped) end - def self.unbox(overlapped : LibC::OVERLAPPED*) - start = overlapped.as(Pointer(UInt8)) - offsetof(OverlappedOperation, @overlapped) - Box(OverlappedOperation).unbox(start.as(Pointer(Void))) + protected def schedule(&) + done! + yield @fiber end - def to_unsafe - pointerof(@overlapped) + private def done! + @fiber.cancel_timeout + @state = :done end - def wait_for_result(timeout, &) + private def wait_for_completion(timeout) + if timeout + sleep timeout + else + Fiber.suspend + end + + unless @state.done? + if try_cancel + # Wait for cancellation to complete. We must not free the operation + # until it's completed. + Fiber.suspend + end + end + end + end + + class IOOverlappedOperation < OverlappedOperation + def initialize(@handle : LibC::HANDLE) + end + + def wait_for_result(timeout, & : WinError ->) wait_for_completion(timeout) result = LibC.GetOverlappedResult(@handle, self, out bytes, 0) @@ -124,11 +151,35 @@ module Crystal::IOCP bytes end - def wait_for_wsa_result(timeout, &) + private def try_cancel : Bool + # Microsoft documentation: + # The application must not free or reuse the OVERLAPPED structure + # associated with the canceled I/O operations until they have completed + # (this does not apply to asynchronous operations that finished + # synchronously, as nothing would be queued to the IOCP) + ret = LibC.CancelIoEx(@handle, self) + if ret.zero? + case error = WinError.value + when .error_not_found? + # Operation has already completed, do nothing + return false + else + raise RuntimeError.from_os_error("CancelIoEx", os_error: error) + end + end + true + end + end + + class WSAOverlappedOperation < OverlappedOperation + def initialize(@handle : LibC::SOCKET) + end + + def wait_for_result(timeout, & : WinError ->) wait_for_completion(timeout) flags = 0_u32 - result = LibC.WSAGetOverlappedResult(LibC::SOCKET.new(@handle.address), self, out bytes, false, pointerof(flags)) + result = LibC.WSAGetOverlappedResult(@handle, self, out bytes, false, pointerof(flags)) if result.zero? error = WinError.wsa_value yield error @@ -139,57 +190,31 @@ module Crystal::IOCP bytes end - protected def schedule(&) - done! - yield @fiber - end - - def done! - @fiber.cancel_timeout - @state = :done - end - - def try_cancel : Bool + private def try_cancel : Bool # Microsoft documentation: # The application must not free or reuse the OVERLAPPED structure # associated with the canceled I/O operations until they have completed # (this does not apply to asynchronous operations that finished # synchronously, as nothing would be queued to the IOCP) - ret = LibC.CancelIoEx(@handle, self) + ret = LibC.CancelIoEx(Pointer(Void).new(@handle), self) if ret.zero? case error = WinError.value when .error_not_found? # Operation has already completed, do nothing return false else - raise RuntimeError.from_os_error("CancelIOEx", os_error: error) + raise RuntimeError.from_os_error("CancelIoEx", os_error: error) end end true end - - def wait_for_completion(timeout) - if timeout - sleep timeout - else - Fiber.suspend - end - - unless @state.done? - if try_cancel - # Wait for cancellation to complete. We must not free the operation - # until it's completed. - Fiber.suspend - end - end - end end def self.overlapped_operation(file_descriptor, method, timeout, *, offset = nil, writing = false, &) handle = file_descriptor.windows_handle seekable = LibC.SetFilePointerEx(handle, 0, out original_offset, IO::Seek::Current) != 0 - OverlappedOperation.run(handle) do |operation| + IOOverlappedOperation.run(handle) do |operation| overlapped = operation.to_unsafe if seekable start_offset = offset || original_offset @@ -243,7 +268,7 @@ module Crystal::IOCP end def self.wsa_overlapped_operation(target, socket, method, timeout, connreset_is_error = true, &) - OverlappedOperation.run(socket) do |operation| + WSAOverlappedOperation.run(socket) do |operation| result, value = yield operation if result == LibC::SOCKET_ERROR @@ -257,7 +282,7 @@ module Crystal::IOCP return value end - operation.wait_for_wsa_result(timeout) do |error| + operation.wait_for_result(timeout) do |error| case error when .wsa_io_incomplete?, .error_operation_aborted? raise IO::TimeoutError.new("#{method} timed out") diff --git a/src/crystal/system/win32/socket.cr b/src/crystal/system/win32/socket.cr index 3172be467836..5ed235e24574 100644 --- a/src/crystal/system/win32/socket.cr +++ b/src/crystal/system/win32/socket.cr @@ -129,7 +129,7 @@ module Crystal::System::Socket # :nodoc: def overlapped_connect(socket, method, timeout, &) - IOCP::OverlappedOperation.run(socket) do |operation| + IOCP::WSAOverlappedOperation.run(socket) do |operation| result = yield operation if result == 0 @@ -145,7 +145,7 @@ module Crystal::System::Socket return nil end - operation.wait_for_wsa_result(timeout) do |error| + operation.wait_for_result(timeout) do |error| case error when .wsa_io_incomplete?, .wsaeconnrefused? return ::Socket::ConnectError.from_os_error(method, error) @@ -192,7 +192,7 @@ module Crystal::System::Socket end def overlapped_accept(socket, method, &) - IOCP::OverlappedOperation.run(socket) do |operation| + IOCP::WSAOverlappedOperation.run(socket) do |operation| result = yield operation if result == 0 @@ -206,7 +206,7 @@ module Crystal::System::Socket return true end - operation.wait_for_wsa_result(read_timeout) do |error| + operation.wait_for_result(read_timeout) do |error| case error when .wsa_io_incomplete?, .wsaenotsock? return false From c8ecd9339f64a72c83f76f7c65975856ba96e3b5 Mon Sep 17 00:00:00 2001 From: Julien Portalier Date: Mon, 9 Sep 2024 22:14:43 +0200 Subject: [PATCH 18/36] Refactor `EventLoop` interface for sleeps & select timeouts (#14980) A couple refactors related to select timeout and the signature of `Crystal::EventLoop::Event#add` that only needs to handle a nilable for `libevent` (because it caches events); other loop don't need that. Sleep and registering a select action are always setting a `Time::Span` and don't need to deal with a nilable. The exception is `IO::Evented` that keeps a cache of `libevent` events and must be able to remove the timeout in case `@read_timeout` would have been set to nil before a second wait on read. --- src/channel/select/timeout_action.cr | 8 +++++--- src/crystal/system/event_loop.cr | 2 +- src/crystal/system/unix/event_libevent.cr | 20 ++++++++++---------- src/crystal/system/wasi/event_loop.cr | 5 ++++- src/crystal/system/win32/event_loop_iocp.cr | 4 ++-- src/fiber.cr | 5 +++-- 6 files changed, 25 insertions(+), 19 deletions(-) diff --git a/src/channel/select/timeout_action.cr b/src/channel/select/timeout_action.cr index 9240b480db1a..39986197bbdc 100644 --- a/src/channel/select/timeout_action.cr +++ b/src/channel/select/timeout_action.cr @@ -58,9 +58,11 @@ class Channel(T) end def time_expired(fiber : Fiber) : Nil - if @select_context.try &.try_trigger - fiber.enqueue - end + fiber.enqueue if time_expired? + end + + def time_expired? : Bool + @select_context.try &.try_trigger || false end end end diff --git a/src/crystal/system/event_loop.cr b/src/crystal/system/event_loop.cr index fb1042b21f96..fe973ec8c99e 100644 --- a/src/crystal/system/event_loop.cr +++ b/src/crystal/system/event_loop.cr @@ -56,7 +56,7 @@ abstract class Crystal::EventLoop abstract def free : Nil # Adds a new timeout to this event. - abstract def add(timeout : Time::Span?) : Nil + abstract def add(timeout : Time::Span) : Nil end end diff --git a/src/crystal/system/unix/event_libevent.cr b/src/crystal/system/unix/event_libevent.cr index 21d6765646d1..32578e5aba9a 100644 --- a/src/crystal/system/unix/event_libevent.cr +++ b/src/crystal/system/unix/event_libevent.cr @@ -19,16 +19,16 @@ module Crystal::LibEvent @freed = false end - def add(timeout : Time::Span?) : Nil - if timeout - timeval = LibC::Timeval.new( - tv_sec: LibC::TimeT.new(timeout.total_seconds), - tv_usec: timeout.nanoseconds // 1_000 - ) - LibEvent2.event_add(@event, pointerof(timeval)) - else - LibEvent2.event_add(@event, nil) - end + def add(timeout : Time::Span) : Nil + timeval = LibC::Timeval.new( + tv_sec: LibC::TimeT.new(timeout.total_seconds), + tv_usec: timeout.nanoseconds // 1_000 + ) + LibEvent2.event_add(@event, pointerof(timeval)) + end + + def add(timeout : Nil) : Nil + LibEvent2.event_add(@event, nil) end def free : Nil diff --git a/src/crystal/system/wasi/event_loop.cr b/src/crystal/system/wasi/event_loop.cr index c804c4be27aa..3cce9ba8361c 100644 --- a/src/crystal/system/wasi/event_loop.cr +++ b/src/crystal/system/wasi/event_loop.cr @@ -132,7 +132,10 @@ end struct Crystal::Wasi::Event include Crystal::EventLoop::Event - def add(timeout : Time::Span?) : Nil + def add(timeout : Time::Span) : Nil + end + + def add(timeout : Nil) : Nil end def free : Nil diff --git a/src/crystal/system/win32/event_loop_iocp.cr b/src/crystal/system/win32/event_loop_iocp.cr index d3cfaf98d853..d3655fdb5861 100644 --- a/src/crystal/system/win32/event_loop_iocp.cr +++ b/src/crystal/system/win32/event_loop_iocp.cr @@ -298,8 +298,8 @@ class Crystal::IOCP::Event free end - def add(timeout : Time::Span?) : Nil - @wake_at = timeout ? Time.monotonic + timeout : Time.monotonic + def add(timeout : Time::Span) : Nil + @wake_at = Time.monotonic + timeout Crystal::EventLoop.current.enqueue(self) end end diff --git a/src/fiber.cr b/src/fiber.cr index 0d471e5a96e4..1086ebdd3669 100644 --- a/src/fiber.cr +++ b/src/fiber.cr @@ -234,20 +234,21 @@ class Fiber end # :nodoc: - def timeout(timeout : Time::Span?, select_action : Channel::TimeoutAction? = nil) : Nil + def timeout(timeout : Time::Span, select_action : Channel::TimeoutAction) : Nil @timeout_select_action = select_action timeout_event.add(timeout) end # :nodoc: def cancel_timeout : Nil + return unless @timeout_select_action @timeout_select_action = nil @timeout_event.try &.delete end # The current fiber will resume after a period of time. # The timeout can be cancelled with `cancel_timeout` - def self.timeout(timeout : Time::Span?, select_action : Channel::TimeoutAction? = nil) : Nil + def self.timeout(timeout : Time::Span, select_action : Channel::TimeoutAction) : Nil Fiber.current.timeout(timeout, select_action) end From 4023f522935743ba6966b627c0976fa653739975 Mon Sep 17 00:00:00 2001 From: Quinton Miller Date: Tue, 10 Sep 2024 14:25:48 +0800 Subject: [PATCH 19/36] Remove some uses of deprecated overloads in standard library specs (#14963) * `File.real_path` * `Benchmark::IPS::Job.new` * `File.executable?`, `.readable?`, `.writable?` * `#read_timeout=`, `#write_timeout=`, `#connect_timeout=` --- spec/compiler/semantic/warnings_spec.cr | 4 +- spec/std/benchmark_spec.cr | 2 +- spec/std/dir_spec.cr | 2 +- spec/std/file_spec.cr | 269 ++++++++++++------------ spec/std/http/client/client_spec.cr | 6 +- spec/std/http/server/server_spec.cr | 2 +- spec/std/io/io_spec.cr | 4 +- spec/std/socket/socket_spec.cr | 2 +- spec/std/socket/unix_socket_spec.cr | 4 +- 9 files changed, 149 insertions(+), 146 deletions(-) diff --git a/spec/compiler/semantic/warnings_spec.cr b/spec/compiler/semantic/warnings_spec.cr index 6c6914c60fe5..e8bbad7b7c29 100644 --- a/spec/compiler/semantic/warnings_spec.cr +++ b/spec/compiler/semantic/warnings_spec.cr @@ -234,7 +234,7 @@ describe "Semantic: warnings" do # NOTE tempfile might be created in symlinked folder # which affects how to match current dir /var/folders/... # with the real path /private/var/folders/... - path = File.real_path(path) + path = File.realpath(path) main_filename = File.join(path, "main.cr") output_filename = File.join(path, "main") @@ -416,7 +416,7 @@ describe "Semantic: warnings" do # NOTE tempfile might be created in symlinked folder # which affects how to match current dir /var/folders/... # with the real path /private/var/folders/... - path = File.real_path(path) + path = File.realpath(path) main_filename = File.join(path, "main.cr") output_filename = File.join(path, "main") diff --git a/spec/std/benchmark_spec.cr b/spec/std/benchmark_spec.cr index 8113f5f03a4c..4a46798b2436 100644 --- a/spec/std/benchmark_spec.cr +++ b/spec/std/benchmark_spec.cr @@ -12,7 +12,7 @@ describe Benchmark::IPS::Job do it "works in general / integration test" do # test several things to avoid running a benchmark over and over again in # the specs - j = Benchmark::IPS::Job.new(0.001, 0.001, interactive: false) + j = Benchmark::IPS::Job.new(1.millisecond, 1.millisecond, interactive: false) a = j.report("a") { sleep 1.milliseconds } b = j.report("b") { sleep 2.milliseconds } diff --git a/spec/std/dir_spec.cr b/spec/std/dir_spec.cr index 439da15becd9..d37483eba947 100644 --- a/spec/std/dir_spec.cr +++ b/spec/std/dir_spec.cr @@ -643,7 +643,7 @@ describe "Dir" do Dir.mkdir_p path # Resolve any symbolic links in path caused by tmpdir being a link. # For example on macOS, /tmp is a symlink to /private/tmp. - path = File.real_path(path) + path = File.realpath(path) target_path = File.join(path, "target") link_path = File.join(path, "link") diff --git a/spec/std/file_spec.cr b/spec/std/file_spec.cr index 07b919bd4a6e..0f88b2028c2f 100644 --- a/spec/std/file_spec.cr +++ b/spec/std/file_spec.cr @@ -236,136 +236,6 @@ describe "File" do end end - describe "executable?" do - it "gives true" do - crystal = Process.executable_path || pending! "Unable to locate compiler executable" - File.executable?(crystal).should be_true - end - - it "gives false" do - File.executable?(datapath("test_file.txt")).should be_false - end - - it "gives false when the file doesn't exist" do - File.executable?(datapath("non_existing_file.txt")).should be_false - end - - it "gives false when a component of the path is a file" do - File.executable?(datapath("dir", "test_file.txt", "")).should be_false - end - - it "follows symlinks" do - with_tempfile("good_symlink_x.txt", "bad_symlink_x.txt") do |good_path, bad_path| - crystal = Process.executable_path || pending! "Unable to locate compiler executable" - File.symlink(File.expand_path(crystal), good_path) - File.symlink(File.expand_path(datapath("non_existing_file.txt")), bad_path) - - File.executable?(good_path).should be_true - File.executable?(bad_path).should be_false - end - end - end - - describe "readable?" do - it "gives true" do - File.readable?(datapath("test_file.txt")).should be_true - end - - it "gives false when the file doesn't exist" do - File.readable?(datapath("non_existing_file.txt")).should be_false - end - - it "gives false when a component of the path is a file" do - File.readable?(datapath("dir", "test_file.txt", "")).should be_false - end - - # win32 doesn't have a way to make files unreadable via chmod - {% unless flag?(:win32) %} - it "gives false when the file has no read permissions" do - with_tempfile("unreadable.txt") do |path| - File.write(path, "") - File.chmod(path, 0o222) - pending_if_superuser! - File.readable?(path).should be_false - end - end - - it "gives false when the file has no permissions" do - with_tempfile("unaccessible.txt") do |path| - File.write(path, "") - File.chmod(path, 0o000) - pending_if_superuser! - File.readable?(path).should be_false - end - end - - it "follows symlinks" do - with_tempfile("good_symlink_r.txt", "bad_symlink_r.txt", "unreadable.txt") do |good_path, bad_path, unreadable| - File.write(unreadable, "") - File.chmod(unreadable, 0o222) - pending_if_superuser! - - File.symlink(File.expand_path(datapath("test_file.txt")), good_path) - File.symlink(File.expand_path(unreadable), bad_path) - - File.readable?(good_path).should be_true - File.readable?(bad_path).should be_false - end - end - {% end %} - - it "gives false when the symbolic link destination doesn't exist" do - with_tempfile("missing_symlink_r.txt") do |missing_path| - File.symlink(File.expand_path(datapath("non_existing_file.txt")), missing_path) - File.readable?(missing_path).should be_false - end - end - end - - describe "writable?" do - it "gives true" do - File.writable?(datapath("test_file.txt")).should be_true - end - - it "gives false when the file doesn't exist" do - File.writable?(datapath("non_existing_file.txt")).should be_false - end - - it "gives false when a component of the path is a file" do - File.writable?(datapath("dir", "test_file.txt", "")).should be_false - end - - it "gives false when the file has no write permissions" do - with_tempfile("readonly.txt") do |path| - File.write(path, "") - File.chmod(path, 0o444) - pending_if_superuser! - File.writable?(path).should be_false - end - end - - it "follows symlinks" do - with_tempfile("good_symlink_w.txt", "bad_symlink_w.txt", "readonly.txt") do |good_path, bad_path, readonly| - File.write(readonly, "") - File.chmod(readonly, 0o444) - pending_if_superuser! - - File.symlink(File.expand_path(datapath("test_file.txt")), good_path) - File.symlink(File.expand_path(readonly), bad_path) - - File.writable?(good_path).should be_true - File.writable?(bad_path).should be_false - end - end - - it "gives false when the symbolic link destination doesn't exist" do - with_tempfile("missing_symlink_w.txt") do |missing_path| - File.symlink(File.expand_path(datapath("non_existing_file.txt")), missing_path) - File.writable?(missing_path).should be_false - end - end - end - describe "file?" do it "gives true" do File.file?(datapath("test_file.txt")).should be_true @@ -701,6 +571,139 @@ describe "File" do it "tests unequal for file and directory" do File.info(datapath("dir")).should_not eq(File.info(datapath("test_file.txt"))) end + + describe ".executable?" do + it "gives true" do + crystal = Process.executable_path || pending! "Unable to locate compiler executable" + File::Info.executable?(crystal).should be_true + File.executable?(crystal).should be_true # deprecated + end + + it "gives false" do + File::Info.executable?(datapath("test_file.txt")).should be_false + end + + it "gives false when the file doesn't exist" do + File::Info.executable?(datapath("non_existing_file.txt")).should be_false + end + + it "gives false when a component of the path is a file" do + File::Info.executable?(datapath("dir", "test_file.txt", "")).should be_false + end + + it "follows symlinks" do + with_tempfile("good_symlink_x.txt", "bad_symlink_x.txt") do |good_path, bad_path| + crystal = Process.executable_path || pending! "Unable to locate compiler executable" + File.symlink(File.expand_path(crystal), good_path) + File.symlink(File.expand_path(datapath("non_existing_file.txt")), bad_path) + + File::Info.executable?(good_path).should be_true + File::Info.executable?(bad_path).should be_false + end + end + end + + describe ".readable?" do + it "gives true" do + File::Info.readable?(datapath("test_file.txt")).should be_true + File.readable?(datapath("test_file.txt")).should be_true # deprecated + end + + it "gives false when the file doesn't exist" do + File::Info.readable?(datapath("non_existing_file.txt")).should be_false + end + + it "gives false when a component of the path is a file" do + File::Info.readable?(datapath("dir", "test_file.txt", "")).should be_false + end + + # win32 doesn't have a way to make files unreadable via chmod + {% unless flag?(:win32) %} + it "gives false when the file has no read permissions" do + with_tempfile("unreadable.txt") do |path| + File.write(path, "") + File.chmod(path, 0o222) + pending_if_superuser! + File::Info.readable?(path).should be_false + end + end + + it "gives false when the file has no permissions" do + with_tempfile("unaccessible.txt") do |path| + File.write(path, "") + File.chmod(path, 0o000) + pending_if_superuser! + File::Info.readable?(path).should be_false + end + end + + it "follows symlinks" do + with_tempfile("good_symlink_r.txt", "bad_symlink_r.txt", "unreadable.txt") do |good_path, bad_path, unreadable| + File.write(unreadable, "") + File.chmod(unreadable, 0o222) + pending_if_superuser! + + File.symlink(File.expand_path(datapath("test_file.txt")), good_path) + File.symlink(File.expand_path(unreadable), bad_path) + + File::Info.readable?(good_path).should be_true + File::Info.readable?(bad_path).should be_false + end + end + {% end %} + + it "gives false when the symbolic link destination doesn't exist" do + with_tempfile("missing_symlink_r.txt") do |missing_path| + File.symlink(File.expand_path(datapath("non_existing_file.txt")), missing_path) + File::Info.readable?(missing_path).should be_false + end + end + end + + describe ".writable?" do + it "gives true" do + File::Info.writable?(datapath("test_file.txt")).should be_true + File.writable?(datapath("test_file.txt")).should be_true # deprecated + end + + it "gives false when the file doesn't exist" do + File::Info.writable?(datapath("non_existing_file.txt")).should be_false + end + + it "gives false when a component of the path is a file" do + File::Info.writable?(datapath("dir", "test_file.txt", "")).should be_false + end + + it "gives false when the file has no write permissions" do + with_tempfile("readonly.txt") do |path| + File.write(path, "") + File.chmod(path, 0o444) + pending_if_superuser! + File::Info.writable?(path).should be_false + end + end + + it "follows symlinks" do + with_tempfile("good_symlink_w.txt", "bad_symlink_w.txt", "readonly.txt") do |good_path, bad_path, readonly| + File.write(readonly, "") + File.chmod(readonly, 0o444) + pending_if_superuser! + + File.symlink(File.expand_path(datapath("test_file.txt")), good_path) + File.symlink(File.expand_path(readonly), bad_path) + + File::Info.writable?(good_path).should be_true + File::Info.writable?(bad_path).should be_false + end + end + + it "gives false when the symbolic link destination doesn't exist" do + with_tempfile("missing_symlink_w.txt") do |missing_path| + File.symlink(File.expand_path(datapath("non_existing_file.txt")), missing_path) + File::Info.writable?(missing_path).should be_false + end + end + end end describe "size" do @@ -1372,15 +1375,15 @@ describe "File" do end it_raises_on_null_byte "readable?" do - File.readable?("foo\0bar") + File::Info.readable?("foo\0bar") end it_raises_on_null_byte "writable?" do - File.writable?("foo\0bar") + File::Info.writable?("foo\0bar") end it_raises_on_null_byte "executable?" do - File.executable?("foo\0bar") + File::Info.executable?("foo\0bar") end it_raises_on_null_byte "file?" do diff --git a/spec/std/http/client/client_spec.cr b/spec/std/http/client/client_spec.cr index 6bd04ab3e2f2..4cd51bf83075 100644 --- a/spec/std/http/client/client_spec.cr +++ b/spec/std/http/client/client_spec.cr @@ -357,7 +357,7 @@ module HTTP test_server("localhost", 0, 0.5.seconds, write_response: false) do |server| client = Client.new("localhost", server.local_address.port) expect_raises(IO::TimeoutError, {% if flag?(:win32) %} "WSARecv timed out" {% else %} "Read timed out" {% end %}) do - client.read_timeout = 0.001 + client.read_timeout = 1.millisecond client.get("/?sleep=1") end end @@ -371,7 +371,7 @@ module HTTP test_server("localhost", 0, 0.seconds, write_response: false) do |server| client = Client.new("localhost", server.local_address.port) expect_raises(IO::TimeoutError, {% if flag?(:win32) %} "WSASend timed out" {% else %} "Write timed out" {% end %}) do - client.write_timeout = 0.001 + client.write_timeout = 1.millisecond client.post("/", body: "a" * 5_000_000) end end @@ -380,7 +380,7 @@ module HTTP it "tests connect_timeout" do test_server("localhost", 0, 0.seconds) do |server| client = Client.new("localhost", server.local_address.port) - client.connect_timeout = 0.5 + client.connect_timeout = 0.5.seconds client.get("/") end end diff --git a/spec/std/http/server/server_spec.cr b/spec/std/http/server/server_spec.cr index 3980084ea414..3c634d755abf 100644 --- a/spec/std/http/server/server_spec.cr +++ b/spec/std/http/server/server_spec.cr @@ -433,7 +433,7 @@ describe HTTP::Server do begin ch.receive client = HTTP::Client.new(address.address, address.port, client_context) - client.read_timeout = client.connect_timeout = 3 + client.read_timeout = client.connect_timeout = 3.seconds client.get("/").body.should eq "ok" ensure ch.send nil diff --git a/spec/std/io/io_spec.cr b/spec/std/io/io_spec.cr index c584ec81a1e8..3be5c07e1479 100644 --- a/spec/std/io/io_spec.cr +++ b/spec/std/io/io_spec.cr @@ -105,11 +105,11 @@ describe IO do write.puts "hello" slice = Bytes.new 1024 - read.read_timeout = 1 + read.read_timeout = 1.second read.read(slice).should eq(6) expect_raises(IO::TimeoutError) do - read.read_timeout = 0.0000001 + read.read_timeout = 0.1.microseconds read.read(slice) end end diff --git a/spec/std/socket/socket_spec.cr b/spec/std/socket/socket_spec.cr index 98555937dea3..f4ff7c90972b 100644 --- a/spec/std/socket/socket_spec.cr +++ b/spec/std/socket/socket_spec.cr @@ -79,7 +79,7 @@ describe Socket, tags: "network" do server = Socket.new(Socket::Family::INET, Socket::Type::STREAM, Socket::Protocol::TCP) port = unused_local_port server.bind("0.0.0.0", port) - server.read_timeout = 0.1 + server.read_timeout = 0.1.seconds server.listen expect_raises(IO::TimeoutError) { server.accept } diff --git a/spec/std/socket/unix_socket_spec.cr b/spec/std/socket/unix_socket_spec.cr index c51f37193c0e..b3bc4931ec78 100644 --- a/spec/std/socket/unix_socket_spec.cr +++ b/spec/std/socket/unix_socket_spec.cr @@ -82,8 +82,8 @@ describe UNIXSocket do it "tests read and write timeouts" do UNIXSocket.pair do |left, right| # BUG: shrink the socket buffers first - left.write_timeout = 0.0001 - right.read_timeout = 0.0001 + left.write_timeout = 0.1.milliseconds + right.read_timeout = 0.1.milliseconds buf = ("a" * IO::DEFAULT_BUFFER_SIZE).to_slice expect_raises(IO::TimeoutError, "Write timed out") do From b5f24681038628de57bf42114cb6eb5a6acfa5d2 Mon Sep 17 00:00:00 2001 From: Quinton Miller Date: Tue, 10 Sep 2024 14:26:10 +0800 Subject: [PATCH 20/36] Fix exponent overflow in `BigFloat#to_s` for very large values (#14982) This is similar to #14971 where the base-10 exponent returned by `LibGMP.mpf_get_str` is not large enough to handle all values internally representable by GMP / MPIR. --- spec/std/big/big_float_spec.cr | 7 +++++ src/big/big_float.cr | 53 +++++++++++++++++++++++++++++++++- 2 files changed, 59 insertions(+), 1 deletion(-) diff --git a/spec/std/big/big_float_spec.cr b/spec/std/big/big_float_spec.cr index 73c6bcf06de8..1b5e4e3893fc 100644 --- a/spec/std/big/big_float_spec.cr +++ b/spec/std/big/big_float_spec.cr @@ -345,6 +345,13 @@ describe "BigFloat" do it { assert_prints (0.1).to_big_f.to_s, "0.100000000000000005551" } it { assert_prints Float64::MAX.to_big_f.to_s, "1.79769313486231570815e+308" } it { assert_prints Float64::MIN_POSITIVE.to_big_f.to_s, "2.22507385850720138309e-308" } + + it { (2.to_big_f ** 7133786264).to_s.should end_with("e+2147483648") } # least power of two with a base-10 exponent greater than Int32::MAX + it { (2.to_big_f ** -7133786264).to_s.should end_with("e-2147483649") } # least power of two with a base-10 exponent less than Int32::MIN + it { (10.to_big_f ** 3000000000 * 1.5).to_s.should end_with("e+3000000000") } + it { (10.to_big_f ** -3000000000 * 1.5).to_s.should end_with("e-3000000000") } + it { (10.to_big_f ** 10000000000 * 1.5).to_s.should end_with("e+10000000000") } + it { (10.to_big_f ** -10000000000 * 1.5).to_s.should end_with("e-10000000000") } end describe "#inspect" do diff --git a/src/big/big_float.cr b/src/big/big_float.cr index ff78b7de8290..f6ab46def36d 100644 --- a/src/big/big_float.cr +++ b/src/big/big_float.cr @@ -362,10 +362,12 @@ struct BigFloat < Float end def to_s(io : IO) : Nil - cstr = LibGMP.mpf_get_str(nil, out decimal_exponent, 10, 0, self) + cstr = LibGMP.mpf_get_str(nil, out orig_decimal_exponent, 10, 0, self) length = LibC.strlen(cstr) buffer = Slice.new(cstr, length) + decimal_exponent = fix_exponent_overflow(orig_decimal_exponent) + # add negative sign if buffer[0]? == 45 # '-' io << '-' @@ -415,6 +417,55 @@ struct BigFloat < Float end end + # The same `LibGMP::MpExp` is used in `LibGMP::MPF` to represent a + # `BigFloat`'s exponent in base `256 ** sizeof(LibGMP::MpLimb)`, and to return + # a base-10 exponent in `LibGMP.mpf_get_str`. The latter is around 9.6x the + # former when `MpLimb` is 32-bit, or around 19.3x when `MpLimb` is 64-bit. + # This means the base-10 exponent will overflow for the majority of `MpExp`'s + # domain, even though `BigFloat`s will work correctly in this exponent range + # otherwise. This method exists to recover the original exponent for `#to_s`. + # + # Note that if `MpExp` is 64-bit, which is the case for non-Windows 64-bit + # targets, then `mpf_get_str` will simply crash for values above + # `2 ** 0x1_0000_0000_0000_0080`; here `exponent10` is around 5.553e+18, and + # never overflows. Thus there is no need to check for overflow in that case. + private def fix_exponent_overflow(exponent10) + {% if LibGMP::MpExp == Int64 %} + exponent10 + {% else %} + # When `self` is non-zero, + # + # @mpf.@_mp_exp == Math.log(abs, 256.0 ** sizeof(LibGMP::MpLimb)).floor + 1 + # @mpf.@_mp_exp - 1 <= Math.log(abs, 256.0 ** sizeof(LibGMP::MpLimb)) < @mpf.@_mp_exp + # @mpf.@_mp_exp - 1 <= Math.log10(abs) / Math.log10(256.0 ** sizeof(LibGMP::MpLimb)) < @mpf.@_mp_exp + # Math.log10(abs) >= (@mpf.@_mp_exp - 1) * Math.log10(256.0 ** sizeof(LibGMP::MpLimb)) + # Math.log10(abs) < @mpf.@_mp_exp * Math.log10(256.0 ** sizeof(LibGMP::MpLimb)) + # + # And also, + # + # exponent10 == Math.log10(abs).floor + 1 + # exponent10 - 1 <= Math.log10(abs) < exponent10 + # + # When `exponent10` overflows, it differs from its real value by an + # integer multiple of `256.0 ** sizeof(LibGMP::MpExp)`. We have to recover + # the integer `overflow_n` such that: + # + # LibGMP::MpExp::MIN <= exponent10 <= LibGMP::MpExp::MAX + # Math.log10(abs) ~= exponent10 + overflow_n * 256.0 ** sizeof(LibGMP::MpExp) + # ~= @mpf.@_mp_exp * Math.log10(256.0 ** sizeof(LibGMP::MpLimb)) + # + # Because the possible intervals for the real `exponent10` are so far apart, + # it suffices to approximate `overflow_n` as follows: + # + # overflow_n ~= (@mpf.@_mp_exp * Math.log10(256.0 ** sizeof(LibGMP::MpLimb)) - exponent10) / 256.0 ** sizeof(LibGMP::MpExp) + # + # This value will be very close to an integer, which we then obtain with + # `#round`. + overflow_n = ((@mpf.@_mp_exp * Math.log10(256.0 ** sizeof(LibGMP::MpLimb)) - exponent10) / 256.0 ** sizeof(LibGMP::MpExp)) + exponent10.to_i64 + overflow_n.round.to_i64 * (256_i64 ** sizeof(LibGMP::MpExp)) + {% end %} + end + def clone self end From ce76bf573f3dadba4c936bba54cc3a4b10d410bf Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Johannes=20M=C3=BCller?= Date: Tue, 10 Sep 2024 08:26:24 +0200 Subject: [PATCH 21/36] Add type restriction to `String#byte_index` `offset` parameter (#14981) The return type is `Int32?`, so `offset` must be `Int32` as well. --- src/string.cr | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/string.cr b/src/string.cr index 35c33b903939..f0dbd1a1eae3 100644 --- a/src/string.cr +++ b/src/string.cr @@ -3715,7 +3715,7 @@ class String # "Dizzy Miss Lizzy".byte_index('z'.ord, -4) # => 13 # "Dizzy Miss Lizzy".byte_index('z'.ord, -17) # => nil # ``` - def byte_index(byte : Int, offset = 0) : Int32? + def byte_index(byte : Int, offset : Int32 = 0) : Int32? offset += bytesize if offset < 0 return if offset < 0 From fdbaeb4c3dec8e1174eb22310602119cab0f5d4f Mon Sep 17 00:00:00 2001 From: Quinton Miller Date: Wed, 11 Sep 2024 05:54:01 +0800 Subject: [PATCH 22/36] Implement floating-point manipulation functions for `BigFloat` (#11007) `BigFloat` already has an implementation of `frexp` because `hash` needs it (I think); this PR adds the remaining floating-point manipulation operations. Methods that take an additional integer accept an `Int` directly, so this takes care of the allowed conversions in #10907. `BigFloat` from GMP doesn't have the notion of signed zeros, so `copysign` is only an approximation. (As usual, MPFR floats feature signed zeros.) --- spec/std/big/big_float_spec.cr | 54 ++++++++++++++++++++++++++++++++++ src/big/big_float.cr | 52 ++++++++++++++++++++++++++++++++ 2 files changed, 106 insertions(+) diff --git a/spec/std/big/big_float_spec.cr b/spec/std/big/big_float_spec.cr index 1b5e4e3893fc..23c782aa3de8 100644 --- a/spec/std/big/big_float_spec.cr +++ b/spec/std/big/big_float_spec.cr @@ -554,6 +554,48 @@ describe "BigFloat" do end describe "BigFloat Math" do + it ".ilogb" do + Math.ilogb(0.2.to_big_f).should eq(-3) + Math.ilogb(123.45.to_big_f).should eq(6) + Math.ilogb(2.to_big_f ** 1_000_000_000).should eq(1_000_000_000) + Math.ilogb(2.to_big_f ** 100_000_000_000).should eq(100_000_000_000) + Math.ilogb(2.to_big_f ** -100_000_000_000).should eq(-100_000_000_000) + expect_raises(ArgumentError) { Math.ilogb(0.to_big_f) } + end + + it ".logb" do + Math.logb(0.2.to_big_f).should eq(-3.to_big_f) + Math.logb(123.45.to_big_f).should eq(6.to_big_f) + Math.logb(2.to_big_f ** 1_000_000_000).should eq(1_000_000_000.to_big_f) + Math.logb(2.to_big_f ** 100_000_000_000).should eq(100_000_000_000.to_big_f) + Math.logb(2.to_big_f ** -100_000_000_000).should eq(-100_000_000_000.to_big_f) + expect_raises(ArgumentError) { Math.logb(0.to_big_f) } + end + + it ".ldexp" do + Math.ldexp(0.2.to_big_f, 2).should eq(0.8.to_big_f) + Math.ldexp(0.2.to_big_f, -2).should eq(0.05.to_big_f) + Math.ldexp(1.to_big_f, 1_000_000_000).should eq(2.to_big_f ** 1_000_000_000) + Math.ldexp(1.to_big_f, 100_000_000_000).should eq(2.to_big_f ** 100_000_000_000) + Math.ldexp(1.to_big_f, -100_000_000_000).should eq(0.5.to_big_f ** 100_000_000_000) + end + + it ".scalbn" do + Math.scalbn(0.2.to_big_f, 2).should eq(0.8.to_big_f) + Math.scalbn(0.2.to_big_f, -2).should eq(0.05.to_big_f) + Math.scalbn(1.to_big_f, 1_000_000_000).should eq(2.to_big_f ** 1_000_000_000) + Math.scalbn(1.to_big_f, 100_000_000_000).should eq(2.to_big_f ** 100_000_000_000) + Math.scalbn(1.to_big_f, -100_000_000_000).should eq(0.5.to_big_f ** 100_000_000_000) + end + + it ".scalbln" do + Math.scalbln(0.2.to_big_f, 2).should eq(0.8.to_big_f) + Math.scalbln(0.2.to_big_f, -2).should eq(0.05.to_big_f) + Math.scalbln(1.to_big_f, 1_000_000_000).should eq(2.to_big_f ** 1_000_000_000) + Math.scalbln(1.to_big_f, 100_000_000_000).should eq(2.to_big_f ** 100_000_000_000) + Math.scalbln(1.to_big_f, -100_000_000_000).should eq(0.5.to_big_f ** 100_000_000_000) + end + it ".frexp" do Math.frexp(0.to_big_f).should eq({0.0, 0}) Math.frexp(1.to_big_f).should eq({0.5, 1}) @@ -574,6 +616,18 @@ describe "BigFloat Math" do Math.frexp(-(2.to_big_f ** -0x100000000)).should eq({-0.5, -0xFFFFFFFF}) end + it ".copysign" do + Math.copysign(3.to_big_f, 2.to_big_f).should eq(3.to_big_f) + Math.copysign(3.to_big_f, 0.to_big_f).should eq(3.to_big_f) + Math.copysign(3.to_big_f, -2.to_big_f).should eq(-3.to_big_f) + Math.copysign(0.to_big_f, 2.to_big_f).should eq(0.to_big_f) + Math.copysign(0.to_big_f, 0.to_big_f).should eq(0.to_big_f) + Math.copysign(0.to_big_f, -2.to_big_f).should eq(0.to_big_f) + Math.copysign(-3.to_big_f, 2.to_big_f).should eq(3.to_big_f) + Math.copysign(-3.to_big_f, 0.to_big_f).should eq(3.to_big_f) + Math.copysign(-3.to_big_f, -2.to_big_f).should eq(-3.to_big_f) + end + it ".sqrt" do Math.sqrt(BigFloat.new("1" + "0"*48)).should eq(BigFloat.new("1" + "0"*24)) end diff --git a/src/big/big_float.cr b/src/big/big_float.cr index f6ab46def36d..5a57500fbdd7 100644 --- a/src/big/big_float.cr +++ b/src/big/big_float.cr @@ -586,6 +586,47 @@ class String end module Math + # Returns the unbiased base 2 exponent of the given floating-point *value*. + # + # Raises `ArgumentError` if *value* is zero. + def ilogb(value : BigFloat) : Int64 + raise ArgumentError.new "Cannot get exponent of zero" if value.zero? + leading_zeros = value.@mpf._mp_d[value.@mpf._mp_size.abs - 1].leading_zeros_count + 8_i64 * sizeof(LibGMP::MpLimb) * value.@mpf._mp_exp - leading_zeros - 1 + end + + # Returns the unbiased radix-independent exponent of the given floating-point *value*. + # + # For `BigFloat` this is equivalent to `ilogb`. + # + # Raises `ArgumentError` is *value* is zero. + def logb(value : BigFloat) : BigFloat + ilogb(value).to_big_f + end + + # Multiplies the given floating-point *value* by 2 raised to the power *exp*. + def ldexp(value : BigFloat, exp : Int) : BigFloat + BigFloat.new do |mpf| + if exp >= 0 + LibGMP.mpf_mul_2exp(mpf, value, exp.to_u64) + else + LibGMP.mpf_div_2exp(mpf, value, exp.abs.to_u64) + end + end + end + + # Returns the floating-point *value* with its exponent raised by *exp*. + # + # For `BigFloat` this is equivalent to `ldexp`. + def scalbn(value : BigFloat, exp : Int) : BigFloat + ldexp(value, exp) + end + + # :ditto: + def scalbln(value : BigFloat, exp : Int) : BigFloat + ldexp(value, exp) + end + # Decomposes the given floating-point *value* into a normalized fraction and an integral power of two. def frexp(value : BigFloat) : {BigFloat, Int64} return {BigFloat.zero, 0_i64} if value.zero? @@ -608,6 +649,17 @@ module Math {frac, exp} end + # Returns the floating-point value with the magnitude of *value1* and the sign of *value2*. + # + # `BigFloat` does not support signed zeros; if `value2 == 0`, the returned value is non-negative. + def copysign(value1 : BigFloat, value2 : BigFloat) : BigFloat + if value1.negative? != value2.negative? # opposite signs + -value1 + else + value1 + end + end + # Calculates the square root of *value*. # # ``` From 8b1a3916fb702c8f55895139d2984d256288f13a Mon Sep 17 00:00:00 2001 From: Quinton Miller Date: Wed, 11 Sep 2024 05:54:21 +0800 Subject: [PATCH 23/36] Remove TODO in `Crystal::Loader` on Windows (#14988) This has been addressed by #14146 from outside the loader, and there is essentially only one TODO left for #11575. --- src/compiler/crystal/loader/msvc.cr | 1 - 1 file changed, 1 deletion(-) diff --git a/src/compiler/crystal/loader/msvc.cr b/src/compiler/crystal/loader/msvc.cr index 772e4c5c232f..966f6ec5d246 100644 --- a/src/compiler/crystal/loader/msvc.cr +++ b/src/compiler/crystal/loader/msvc.cr @@ -185,7 +185,6 @@ class Crystal::Loader end private def open_library(path : String) - # TODO: respect `@[Link(dll:)]`'s search order LibC.LoadLibraryExW(System.to_wstr(path), nil, 0) end From dea39ad408d06f8c3d9017e1769c8726e67c012e Mon Sep 17 00:00:00 2001 From: Quinton Miller Date: Fri, 13 Sep 2024 18:38:41 +0800 Subject: [PATCH 24/36] Async DNS resolution on Windows (#14979) Uses `GetAddrInfoExW` with IOCP --- spec/std/socket/addrinfo_spec.cr | 32 +++++++++-- src/crystal/system/addrinfo.cr | 6 +- src/crystal/system/win32/addrinfo.cr | 43 ++++++++++++--- src/crystal/system/win32/addrinfo_win7.cr | 61 +++++++++++++++++++++ src/crystal/system/win32/iocp.cr | 36 ++++++++++++ src/http/client.cr | 8 +-- src/lib_c/x86_64-windows-msvc/c/winsock2.cr | 7 +++ src/lib_c/x86_64-windows-msvc/c/ws2def.cr | 14 +++++ src/lib_c/x86_64-windows-msvc/c/ws2tcpip.cr | 20 +++++++ src/socket/addrinfo.cr | 30 ++++++---- src/socket/tcp_socket.cr | 2 +- src/winerror.cr | 5 +- 12 files changed, 233 insertions(+), 31 deletions(-) create mode 100644 src/crystal/system/win32/addrinfo_win7.cr diff --git a/spec/std/socket/addrinfo_spec.cr b/spec/std/socket/addrinfo_spec.cr index 615058472525..109eb383562b 100644 --- a/spec/std/socket/addrinfo_spec.cr +++ b/spec/std/socket/addrinfo_spec.cr @@ -22,6 +22,20 @@ describe Socket::Addrinfo, tags: "network" do end end end + + it "raises helpful message on getaddrinfo failure" do + expect_raises(Socket::Addrinfo::Error, "Hostname lookup for badhostname failed: ") do + Socket::Addrinfo.resolve("badhostname", 80, type: Socket::Type::DGRAM) + end + end + + {% if flag?(:win32) %} + it "raises timeout error" do + expect_raises(IO::TimeoutError) do + Socket::Addrinfo.resolve("badhostname", 80, type: Socket::Type::STREAM, timeout: 0.milliseconds) + end + end + {% end %} end describe ".tcp" do @@ -37,11 +51,13 @@ describe Socket::Addrinfo, tags: "network" do end end - it "raises helpful message on getaddrinfo failure" do - expect_raises(Socket::Addrinfo::Error, "Hostname lookup for badhostname failed: ") do - Socket::Addrinfo.resolve("badhostname", 80, type: Socket::Type::DGRAM) + {% if flag?(:win32) %} + it "raises timeout error" do + expect_raises(IO::TimeoutError) do + Socket::Addrinfo.tcp("badhostname", 80, timeout: 0.milliseconds) + end end - end + {% end %} end describe ".udp" do @@ -56,6 +72,14 @@ describe Socket::Addrinfo, tags: "network" do typeof(addrinfo).should eq(Socket::Addrinfo) end end + + {% if flag?(:win32) %} + it "raises timeout error" do + expect_raises(IO::TimeoutError) do + Socket::Addrinfo.udp("badhostname", 80, timeout: 0.milliseconds) + end + end + {% end %} end describe "#ip_address" do diff --git a/src/crystal/system/addrinfo.cr b/src/crystal/system/addrinfo.cr index 23513e6f763e..ff9166f3aca1 100644 --- a/src/crystal/system/addrinfo.cr +++ b/src/crystal/system/addrinfo.cr @@ -30,7 +30,11 @@ end {% elsif flag?(:unix) %} require "./unix/addrinfo" {% elsif flag?(:win32) %} - require "./win32/addrinfo" + {% if flag?(:win7) %} + require "./win32/addrinfo_win7" + {% else %} + require "./win32/addrinfo" + {% end %} {% else %} {% raise "No Crystal::System::Addrinfo implementation available" %} {% end %} diff --git a/src/crystal/system/win32/addrinfo.cr b/src/crystal/system/win32/addrinfo.cr index b033d61f16e7..91ebb1620a43 100644 --- a/src/crystal/system/win32/addrinfo.cr +++ b/src/crystal/system/win32/addrinfo.cr @@ -1,5 +1,5 @@ module Crystal::System::Addrinfo - alias Handle = LibC::Addrinfo* + alias Handle = LibC::ADDRINFOEXW* @addr : LibC::SockaddrIn6 @@ -30,7 +30,7 @@ module Crystal::System::Addrinfo end def self.getaddrinfo(domain, service, family, type, protocol, timeout) : Handle - hints = LibC::Addrinfo.new + hints = LibC::ADDRINFOEXW.new hints.ai_family = (family || ::Socket::Family::UNSPEC).to_i32 hints.ai_socktype = type hints.ai_protocol = protocol @@ -43,12 +43,39 @@ module Crystal::System::Addrinfo end end - ret = LibC.getaddrinfo(domain, service.to_s, pointerof(hints), out ptr) - unless ret.zero? - error = WinError.new(ret.to_u32!) - raise ::Socket::Addrinfo::Error.from_os_error(nil, error, domain: domain, type: type, protocol: protocol, service: service) + Crystal::IOCP::GetAddrInfoOverlappedOperation.run(Crystal::EventLoop.current.iocp) do |operation| + completion_routine = LibC::LPLOOKUPSERVICE_COMPLETION_ROUTINE.new do |dwError, dwBytes, lpOverlapped| + orig_operation = Crystal::IOCP::GetAddrInfoOverlappedOperation.unbox(lpOverlapped) + LibC.PostQueuedCompletionStatus(orig_operation.iocp, 0, 0, lpOverlapped) + end + + # NOTE: we handle the timeout ourselves so we don't pass a `LibC::Timeval` + # to Win32 here + result = LibC.GetAddrInfoExW( + Crystal::System.to_wstr(domain), Crystal::System.to_wstr(service.to_s), LibC::NS_DNS, nil, pointerof(hints), + out addrinfos, nil, operation, completion_routine, out cancel_handle) + + if result == 0 + return addrinfos + else + case error = WinError.new(result.to_u32!) + when .wsa_io_pending? + # used in `Crystal::IOCP::OverlappedOperation#try_cancel_getaddrinfo` + operation.cancel_handle = cancel_handle + else + raise ::Socket::Addrinfo::Error.from_os_error("GetAddrInfoExW", error, domain: domain, type: type, protocol: protocol, service: service) + end + end + + operation.wait_for_result(timeout) do |error| + case error + when .wsa_e_cancelled? + raise IO::TimeoutError.new("GetAddrInfoExW timed out") + else + raise ::Socket::Addrinfo::Error.from_os_error("GetAddrInfoExW", error, domain: domain, type: type, protocol: protocol, service: service) + end + end end - ptr end def self.next_addrinfo(addrinfo : Handle) : Handle @@ -56,6 +83,6 @@ module Crystal::System::Addrinfo end def self.free_addrinfo(addrinfo : Handle) - LibC.freeaddrinfo(addrinfo) + LibC.FreeAddrInfoExW(addrinfo) end end diff --git a/src/crystal/system/win32/addrinfo_win7.cr b/src/crystal/system/win32/addrinfo_win7.cr new file mode 100644 index 000000000000..b033d61f16e7 --- /dev/null +++ b/src/crystal/system/win32/addrinfo_win7.cr @@ -0,0 +1,61 @@ +module Crystal::System::Addrinfo + alias Handle = LibC::Addrinfo* + + @addr : LibC::SockaddrIn6 + + protected def initialize(addrinfo : Handle) + @family = ::Socket::Family.from_value(addrinfo.value.ai_family) + @type = ::Socket::Type.from_value(addrinfo.value.ai_socktype) + @protocol = ::Socket::Protocol.from_value(addrinfo.value.ai_protocol) + @size = addrinfo.value.ai_addrlen.to_i + + @addr = uninitialized LibC::SockaddrIn6 + + case @family + when ::Socket::Family::INET6 + addrinfo.value.ai_addr.as(LibC::SockaddrIn6*).copy_to(pointerof(@addr).as(LibC::SockaddrIn6*), 1) + when ::Socket::Family::INET + addrinfo.value.ai_addr.as(LibC::SockaddrIn*).copy_to(pointerof(@addr).as(LibC::SockaddrIn*), 1) + else + # TODO: (asterite) UNSPEC and UNIX unsupported? + end + end + + def system_ip_address : ::Socket::IPAddress + ::Socket::IPAddress.from(to_unsafe, size) + end + + def to_unsafe + pointerof(@addr).as(LibC::Sockaddr*) + end + + def self.getaddrinfo(domain, service, family, type, protocol, timeout) : Handle + hints = LibC::Addrinfo.new + hints.ai_family = (family || ::Socket::Family::UNSPEC).to_i32 + hints.ai_socktype = type + hints.ai_protocol = protocol + hints.ai_flags = 0 + + if service.is_a?(Int) + hints.ai_flags |= LibC::AI_NUMERICSERV + if service < 0 + raise ::Socket::Addrinfo::Error.from_os_error(nil, WinError::WSATYPE_NOT_FOUND, domain: domain, type: type, protocol: protocol, service: service) + end + end + + ret = LibC.getaddrinfo(domain, service.to_s, pointerof(hints), out ptr) + unless ret.zero? + error = WinError.new(ret.to_u32!) + raise ::Socket::Addrinfo::Error.from_os_error(nil, error, domain: domain, type: type, protocol: protocol, service: service) + end + ptr + end + + def self.next_addrinfo(addrinfo : Handle) : Handle + addrinfo.value.ai_next + end + + def self.free_addrinfo(addrinfo : Handle) + LibC.freeaddrinfo(addrinfo) + end +end diff --git a/src/crystal/system/win32/iocp.cr b/src/crystal/system/win32/iocp.cr index 384784a193db..19c92c8f8725 100644 --- a/src/crystal/system/win32/iocp.cr +++ b/src/crystal/system/win32/iocp.cr @@ -210,6 +210,42 @@ module Crystal::IOCP end end + class GetAddrInfoOverlappedOperation < OverlappedOperation + getter iocp + setter cancel_handle : LibC::HANDLE = LibC::INVALID_HANDLE_VALUE + + def initialize(@iocp : LibC::HANDLE) + end + + def wait_for_result(timeout, & : WinError ->) + wait_for_completion(timeout) + + result = LibC.GetAddrInfoExOverlappedResult(self) + unless result.zero? + error = WinError.new(result.to_u32!) + yield error + + raise Socket::Addrinfo::Error.from_os_error("GetAddrInfoExOverlappedResult", error) + end + + @overlapped.union.pointer.as(LibC::ADDRINFOEXW**).value + end + + private def try_cancel : Bool + ret = LibC.GetAddrInfoExCancel(pointerof(@cancel_handle)) + unless ret.zero? + case error = WinError.new(ret.to_u32!) + when .wsa_invalid_handle? + # Operation has already completed, do nothing + return false + else + raise Socket::Addrinfo::Error.from_os_error("GetAddrInfoExCancel", error) + end + end + true + end + end + def self.overlapped_operation(file_descriptor, method, timeout, *, offset = nil, writing = false, &) handle = file_descriptor.windows_handle seekable = LibC.SetFilePointerEx(handle, 0, out original_offset, IO::Seek::Current) != 0 diff --git a/src/http/client.cr b/src/http/client.cr index b641065ac930..7324bdf7d639 100644 --- a/src/http/client.cr +++ b/src/http/client.cr @@ -343,10 +343,10 @@ class HTTP::Client # ``` setter connect_timeout : Time::Span? - # **This method has no effect right now** - # # Sets the number of seconds to wait when resolving a name, before raising an `IO::TimeoutError`. # + # NOTE: *dns_timeout* is currently only supported on Windows. + # # ``` # require "http/client" # @@ -363,10 +363,10 @@ class HTTP::Client self.dns_timeout = dns_timeout.seconds end - # **This method has no effect right now** - # # Sets the number of seconds to wait when resolving a name with a `Time::Span`, before raising an `IO::TimeoutError`. # + # NOTE: *dns_timeout* is currently only supported on Windows. + # # ``` # require "http/client" # diff --git a/src/lib_c/x86_64-windows-msvc/c/winsock2.cr b/src/lib_c/x86_64-windows-msvc/c/winsock2.cr index 223c2366b072..68ce6f9ef421 100644 --- a/src/lib_c/x86_64-windows-msvc/c/winsock2.cr +++ b/src/lib_c/x86_64-windows-msvc/c/winsock2.cr @@ -20,6 +20,8 @@ lib LibC lpVendorInfo : Char* end + NS_DNS = 12_u32 + INVALID_SOCKET = ~SOCKET.new(0) SOCKET_ERROR = -1 @@ -111,6 +113,11 @@ lib LibC alias WSAOVERLAPPED_COMPLETION_ROUTINE = Proc(DWORD, DWORD, WSAOVERLAPPED*, DWORD, Void) + struct Timeval + tv_sec : Long + tv_usec : Long + end + struct Linger l_onoff : UShort l_linger : UShort diff --git a/src/lib_c/x86_64-windows-msvc/c/ws2def.cr b/src/lib_c/x86_64-windows-msvc/c/ws2def.cr index 9fc19857f4a3..41e0a1a408eb 100644 --- a/src/lib_c/x86_64-windows-msvc/c/ws2def.cr +++ b/src/lib_c/x86_64-windows-msvc/c/ws2def.cr @@ -208,4 +208,18 @@ lib LibC ai_addr : Sockaddr* ai_next : Addrinfo* end + + struct ADDRINFOEXW + ai_flags : Int + ai_family : Int + ai_socktype : Int + ai_protocol : Int + ai_addrlen : SizeT + ai_canonname : LPWSTR + ai_addr : Sockaddr* + ai_blob : Void* + ai_bloblen : SizeT + ai_provider : GUID* + ai_next : ADDRINFOEXW* + end end diff --git a/src/lib_c/x86_64-windows-msvc/c/ws2tcpip.cr b/src/lib_c/x86_64-windows-msvc/c/ws2tcpip.cr index 338063ccf6f6..3b3f61ba7fdb 100644 --- a/src/lib_c/x86_64-windows-msvc/c/ws2tcpip.cr +++ b/src/lib_c/x86_64-windows-msvc/c/ws2tcpip.cr @@ -17,4 +17,24 @@ lib LibC fun getaddrinfo(pNodeName : Char*, pServiceName : Char*, pHints : Addrinfo*, ppResult : Addrinfo**) : Int fun inet_ntop(family : Int, pAddr : Void*, pStringBuf : Char*, stringBufSize : SizeT) : Char* fun inet_pton(family : Int, pszAddrString : Char*, pAddrBuf : Void*) : Int + + fun FreeAddrInfoExW(pAddrInfoEx : ADDRINFOEXW*) + + alias LPLOOKUPSERVICE_COMPLETION_ROUTINE = DWORD, DWORD, WSAOVERLAPPED* -> + + fun GetAddrInfoExW( + pName : LPWSTR, + pServiceName : LPWSTR, + dwNameSpace : DWORD, + lpNspId : GUID*, + hints : ADDRINFOEXW*, + ppResult : ADDRINFOEXW**, + timeout : Timeval*, + lpOverlapped : OVERLAPPED*, + lpCompletionRoutine : LPLOOKUPSERVICE_COMPLETION_ROUTINE, + lpHandle : HANDLE*, + ) : Int + + fun GetAddrInfoExOverlappedResult(lpOverlapped : OVERLAPPED*) : Int + fun GetAddrInfoExCancel(lpHandle : HANDLE*) : Int end diff --git a/src/socket/addrinfo.cr b/src/socket/addrinfo.cr index cdf55c912601..ef76d0e285b6 100644 --- a/src/socket/addrinfo.cr +++ b/src/socket/addrinfo.cr @@ -23,6 +23,9 @@ class Socket # specified. # - *protocol* is the intended socket protocol (e.g. `Protocol::TCP`) and # should be specified. + # - *timeout* is optional and specifies the maximum time to wait before + # `IO::TimeoutError` is raised. Currently this is only supported on + # Windows. # # Example: # ``` @@ -107,8 +110,11 @@ class Socket "Hostname lookup for #{domain} failed" end - def self.os_error_message(os_error : Errno, *, type, service, protocol, **opts) - case os_error.value + def self.os_error_message(os_error : Errno | WinError, *, type, service, protocol, **opts) + # when `EAI_NONAME` etc. is an integer then only `os_error.value` can + # match; when `EAI_NONAME` is a `WinError` then `os_error` itself can + # match + case os_error.is_a?(Errno) ? os_error.value : os_error when LibC::EAI_NONAME "No address found" when LibC::EAI_SOCKTYPE @@ -116,12 +122,14 @@ class Socket when LibC::EAI_SERVICE "The requested service #{service} is not available for the requested socket type #{type}" else - {% unless flag?(:win32) %} - # There's no need for a special win32 branch because the os_error on Windows - # is of type WinError, which wouldn't match this overload anyways. - - String.new(LibC.gai_strerror(os_error.value)) + # Win32 also has this method, but `WinError` is already sufficient + {% if LibC.has_method?(:gai_strerror) %} + if os_error.is_a?(Errno) + return String.new(LibC.gai_strerror(os_error)) + end {% end %} + + super end end end @@ -148,13 +156,13 @@ class Socket # addrinfos = Socket::Addrinfo.tcp("example.org", 80) # ``` def self.tcp(domain : String, service, family = Family::UNSPEC, timeout = nil) : Array(Addrinfo) - resolve(domain, service, family, Type::STREAM, Protocol::TCP) + resolve(domain, service, family, Type::STREAM, Protocol::TCP, timeout) end # Resolves a domain for the TCP protocol with STREAM type, and yields each # possible `Addrinfo`. See `#resolve` for details. def self.tcp(domain : String, service, family = Family::UNSPEC, timeout = nil, &) - resolve(domain, service, family, Type::STREAM, Protocol::TCP) { |addrinfo| yield addrinfo } + resolve(domain, service, family, Type::STREAM, Protocol::TCP, timeout) { |addrinfo| yield addrinfo } end # Resolves *domain* for the UDP protocol and returns an `Array` of possible @@ -167,13 +175,13 @@ class Socket # addrinfos = Socket::Addrinfo.udp("example.org", 53) # ``` def self.udp(domain : String, service, family = Family::UNSPEC, timeout = nil) : Array(Addrinfo) - resolve(domain, service, family, Type::DGRAM, Protocol::UDP) + resolve(domain, service, family, Type::DGRAM, Protocol::UDP, timeout) end # Resolves a domain for the UDP protocol with DGRAM type, and yields each # possible `Addrinfo`. See `#resolve` for details. def self.udp(domain : String, service, family = Family::UNSPEC, timeout = nil, &) - resolve(domain, service, family, Type::DGRAM, Protocol::UDP) { |addrinfo| yield addrinfo } + resolve(domain, service, family, Type::DGRAM, Protocol::UDP, timeout) { |addrinfo| yield addrinfo } end # Returns an `IPAddress` matching this addrinfo. diff --git a/src/socket/tcp_socket.cr b/src/socket/tcp_socket.cr index 387417211a1a..4edcb3d08e5f 100644 --- a/src/socket/tcp_socket.cr +++ b/src/socket/tcp_socket.cr @@ -25,7 +25,7 @@ class TCPSocket < IPSocket # connection time to the remote server with `connect_timeout`. Both values # must be in seconds (integers or floats). # - # Note that `dns_timeout` is currently ignored. + # NOTE: *dns_timeout* is currently only supported on Windows. def initialize(host : String, port, dns_timeout = nil, connect_timeout = nil, blocking = false) Addrinfo.tcp(host, port, timeout: dns_timeout) do |addrinfo| super(addrinfo.family, addrinfo.type, addrinfo.protocol, blocking) diff --git a/src/winerror.cr b/src/winerror.cr index ab978769d553..fbb2fb553873 100644 --- a/src/winerror.cr +++ b/src/winerror.cr @@ -2305,6 +2305,7 @@ enum WinError : UInt32 ERROR_STATE_CONTAINER_NAME_SIZE_LIMIT_EXCEEDED = 15818_u32 ERROR_API_UNAVAILABLE = 15841_u32 - WSA_IO_PENDING = ERROR_IO_PENDING - WSA_IO_INCOMPLETE = ERROR_IO_INCOMPLETE + WSA_IO_PENDING = ERROR_IO_PENDING + WSA_IO_INCOMPLETE = ERROR_IO_INCOMPLETE + WSA_INVALID_HANDLE = ERROR_INVALID_HANDLE end From 833d90fc7fc0e8dd1f3ccc4b3084c9c8f0849922 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Johannes=20M=C3=BCller?= Date: Fri, 13 Sep 2024 12:39:03 +0200 Subject: [PATCH 25/36] Add error handling for linker flag sub commands (#14932) Co-authored-by: Quinton Miller --- src/compiler/crystal/compiler.cr | 19 ++++++++++++++++++- 1 file changed, 18 insertions(+), 1 deletion(-) diff --git a/src/compiler/crystal/compiler.cr b/src/compiler/crystal/compiler.cr index 38880ee9ed64..f25713c6385e 100644 --- a/src/compiler/crystal/compiler.cr +++ b/src/compiler/crystal/compiler.cr @@ -411,7 +411,24 @@ module Crystal if program.has_flag? "msvc" lib_flags = program.lib_flags # Execute and expand `subcommands`. - lib_flags = lib_flags.gsub(/`(.*?)`/) { `#{$1}` } if expand + if expand + lib_flags = lib_flags.gsub(/`(.*?)`/) do + command = $1 + begin + error_io = IO::Memory.new + output = Process.run(command, shell: true, output: :pipe, error: error_io) do |process| + process.output.gets_to_end + end + unless $?.success? + error_io.rewind + error "Error executing subcommand for linker flags: #{command.inspect}: #{error_io}" + end + output + rescue exc + error "Error executing subcommand for linker flags: #{command.inspect}: #{exc}" + end + end + end object_arg = Process.quote_windows(object_names) output_arg = Process.quote_windows("/Fe#{output_filename}") From 5125f6619f7c5ec0f9825463a87830723ccd5269 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Gustavo=20Gir=C3=A1ldez?= Date: Mon, 16 Sep 2024 08:12:48 -0300 Subject: [PATCH 26/36] Avoid unwinding the stack on hot path in method call lookups (#15002) The method lookup code uses exceptions to retry lookups using auto-casting. This is effectively using an exception for execution control, which is not what they are intended for. On `raise`, Crystal will try to unwind the call stack and save it to be able to report the original place where the exception was thrown, and this is a very expensive operation. To avoid that, we initialize the callstack of the special `RetryLookupWithLiterals` exception class always with the same fixed value. --- src/compiler/crystal/semantic/call.cr | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/src/compiler/crystal/semantic/call.cr b/src/compiler/crystal/semantic/call.cr index f581ea79d577..e265829a919e 100644 --- a/src/compiler/crystal/semantic/call.cr +++ b/src/compiler/crystal/semantic/call.cr @@ -13,6 +13,11 @@ class Crystal::Call property? uses_with_scope = false class RetryLookupWithLiterals < ::Exception + @@dummy_call_stack = Exception::CallStack.new + + def initialize + self.callstack = @@dummy_call_stack + end end def program From 849f71e2aa97453a45229df346c9127ecddaf5dc Mon Sep 17 00:00:00 2001 From: Quinton Miller Date: Mon, 16 Sep 2024 19:39:12 +0800 Subject: [PATCH 27/36] Drop the non-release Windows compiler artifact (#15000) Since #14964 we're building the compiler in release mode on every workflow run. We can use that build instead of the non-release build for workflow jobs that need a compiler. --- .github/workflows/win.yml | 15 ++++----------- 1 file changed, 4 insertions(+), 11 deletions(-) diff --git a/.github/workflows/win.yml b/.github/workflows/win.yml index 568828b17bee..a256a6806a3f 100644 --- a/.github/workflows/win.yml +++ b/.github/workflows/win.yml @@ -214,16 +214,16 @@ jobs: if: steps.cache-llvm-dlls.outputs.cache-hit != 'true' run: .\etc\win-ci\build-llvm.ps1 -BuildTree deps\llvm -Version ${{ env.CI_LLVM_VERSION }} -TargetsToBuild X86,AArch64 -Dynamic - x86_64-windows: + x86_64-windows-release: needs: [x86_64-windows-libs, x86_64-windows-dlls, x86_64-windows-llvm-libs, x86_64-windows-llvm-dlls] uses: ./.github/workflows/win_build_portable.yml with: - release: false + release: true llvm_version: "18.1.1" x86_64-windows-test: runs-on: windows-2022 - needs: [x86_64-windows] + needs: [x86_64-windows-release] steps: - name: Disable CRLF line ending substitution run: | @@ -238,7 +238,7 @@ jobs: - name: Download Crystal executable uses: actions/download-artifact@v4 with: - name: crystal + name: crystal-release path: build - name: Restore LLVM @@ -266,13 +266,6 @@ jobs: - name: Build samples run: make -f Makefile.win samples - x86_64-windows-release: - needs: [x86_64-windows-libs, x86_64-windows-dlls, x86_64-windows-llvm-libs, x86_64-windows-llvm-dlls] - uses: ./.github/workflows/win_build_portable.yml - with: - release: true - llvm_version: "18.1.1" - x86_64-windows-test-interpreter: runs-on: windows-2022 needs: [x86_64-windows-release] From 9134f9f3ba27b099ebbf048b20560c473cdeb87f Mon Sep 17 00:00:00 2001 From: Quinton Miller Date: Mon, 16 Sep 2024 19:56:21 +0800 Subject: [PATCH 28/36] Fix `Process.exec` stream redirection on Windows (#14986) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The following should print the compiler help message to the file `foo.txt`: ```crystal File.open("foo.txt", "w") do |f| Process.exec("crystal", output: f) end ``` It used to work on Windows in Crystal 1.12, but is now broken since 1.13. This is because `LibC._wexecvp` only inherits file descriptors in the C runtime, not arbitrary Win32 file handles; since we stopped calling `LibC._open_osfhandle`, the C runtime knows nothing about any reopened standard streams in Win32. Thus the above merely prints the help message to the standard output. This PR creates the missing C file descriptors right before `LibC._wexecvp`. It also fixes a different regression of #14947 where reconfiguring `STDIN.blocking` always fails. Co-authored-by: Johannes Müller --- spec/std/process_spec.cr | 21 ++++++++++ src/crystal/system/win32/process.cr | 59 +++++++++++++++++++-------- src/lib_c/x86_64-windows-msvc/c/io.cr | 5 ++- 3 files changed, 67 insertions(+), 18 deletions(-) diff --git a/spec/std/process_spec.cr b/spec/std/process_spec.cr index d41ee0bed242..01a154ccb010 100644 --- a/spec/std/process_spec.cr +++ b/spec/std/process_spec.cr @@ -484,6 +484,27 @@ describe Process do {% end %} describe ".exec" do + it "redirects STDIN and STDOUT to files", tags: %w[slow] do + with_tempfile("crystal-exec-stdin", "crystal-exec-stdout") do |stdin_path, stdout_path| + File.write(stdin_path, "foobar") + + status, _, _ = compile_and_run_source <<-CRYSTAL + command = #{stdin_to_stdout_command[0].inspect} + args = #{stdin_to_stdout_command[1].to_a} of String + stdin_path = #{stdin_path.inspect} + stdout_path = #{stdout_path.inspect} + File.open(stdin_path) do |input| + File.open(stdout_path, "w") do |output| + Process.exec(command, args, input: input, output: output) + end + end + CRYSTAL + + status.success?.should be_true + File.read(stdout_path).chomp.should eq("foobar") + end + end + it "gets error from exec" do expect_raises(File::NotFoundError, "Error executing process: 'foobarbaz'") do Process.exec("foobarbaz") diff --git a/src/crystal/system/win32/process.cr b/src/crystal/system/win32/process.cr index 2c6d81720636..7031654d2299 100644 --- a/src/crystal/system/win32/process.cr +++ b/src/crystal/system/win32/process.cr @@ -326,9 +326,9 @@ struct Crystal::System::Process end private def self.try_replace(command_args, env, clear_env, input, output, error, chdir) - reopen_io(input, ORIGINAL_STDIN) - reopen_io(output, ORIGINAL_STDOUT) - reopen_io(error, ORIGINAL_STDERR) + old_input_fd = reopen_io(input, ORIGINAL_STDIN) + old_output_fd = reopen_io(output, ORIGINAL_STDOUT) + old_error_fd = reopen_io(error, ORIGINAL_STDERR) ENV.clear if clear_env env.try &.each do |key, val| @@ -351,11 +351,18 @@ struct Crystal::System::Process argv << Pointer(LibC::WCHAR).null LibC._wexecvp(command, argv) + + # exec failed; restore the original C runtime file descriptors + errno = Errno.value + LibC._dup2(old_input_fd, 0) + LibC._dup2(old_output_fd, 1) + LibC._dup2(old_error_fd, 2) + errno end def self.replace(command_args, env, clear_env, input, output, error, chdir) : NoReturn - try_replace(command_args, env, clear_env, input, output, error, chdir) - raise_exception_from_errno(command_args.is_a?(String) ? command_args : command_args[0]) + errno = try_replace(command_args, env, clear_env, input, output, error, chdir) + raise_exception_from_errno(command_args.is_a?(String) ? command_args : command_args[0], errno) end private def self.raise_exception_from_errno(command, errno = Errno.value) @@ -367,21 +374,41 @@ struct Crystal::System::Process end end + # Replaces the C standard streams' file descriptors, not Win32's, since + # `try_replace` uses the C `LibC._wexecvp` and only cares about the former. + # Returns a duplicate of the original file descriptor private def self.reopen_io(src_io : IO::FileDescriptor, dst_io : IO::FileDescriptor) - src_io = to_real_fd(src_io) + unless src_io.system_blocking? + raise IO::Error.new("Non-blocking streams are not supported in `Process.exec`", target: src_io) + end - dst_io.reopen(src_io) - dst_io.blocking = true - dst_io.close_on_exec = false - end + src_fd = + case src_io + when STDIN then 0 + when STDOUT then 1 + when STDERR then 2 + else + LibC._open_osfhandle(src_io.windows_handle, 0) + end - private def self.to_real_fd(fd : IO::FileDescriptor) - case fd - when STDIN then ORIGINAL_STDIN - when STDOUT then ORIGINAL_STDOUT - when STDERR then ORIGINAL_STDERR - else fd + dst_fd = + case dst_io + when ORIGINAL_STDIN then 0 + when ORIGINAL_STDOUT then 1 + when ORIGINAL_STDERR then 2 + else + raise "BUG: Invalid destination IO" + end + + return src_fd if dst_fd == src_fd + + orig_src_fd = LibC._dup(src_fd) + + if LibC._dup2(src_fd, dst_fd) == -1 + raise IO::Error.from_errno("Failed to replace C file descriptor", target: dst_io) end + + orig_src_fd end def self.chroot(path) diff --git a/src/lib_c/x86_64-windows-msvc/c/io.cr b/src/lib_c/x86_64-windows-msvc/c/io.cr index 75da8c18e5b9..ccbaa15f2d1b 100644 --- a/src/lib_c/x86_64-windows-msvc/c/io.cr +++ b/src/lib_c/x86_64-windows-msvc/c/io.cr @@ -2,12 +2,13 @@ require "c/stdint" lib LibC fun _wexecvp(cmdname : WCHAR*, argv : WCHAR**) : IntPtrT + fun _open_osfhandle(osfhandle : HANDLE, flags : LibC::Int) : LibC::Int + fun _dup(fd : Int) : Int + fun _dup2(fd1 : Int, fd2 : Int) : Int # unused - fun _open_osfhandle(osfhandle : HANDLE, flags : LibC::Int) : LibC::Int fun _get_osfhandle(fd : Int) : IntPtrT fun _close(fd : Int) : Int - fun _dup2(fd1 : Int, fd2 : Int) : Int fun _isatty(fd : Int) : Int fun _write(fd : Int, buffer : UInt8*, count : UInt) : Int fun _read(fd : Int, buffer : UInt8*, count : UInt) : Int From 5c18900fb7fa65bfb6cdb4da1f3d763700022b8c Mon Sep 17 00:00:00 2001 From: Quinton Miller Date: Tue, 17 Sep 2024 18:47:36 +0800 Subject: [PATCH 29/36] Use Cygwin to build libiconv on Windows CI (#14999) This uses the official releases and build instructions. To compile code with this patch using a Windows Crystal compiler without this patch, either the new library files (`lib\iconv.lib`, `lib\iconv-dynamic.lib`, `iconv-2.dll`) shall be copied to that existing Crystal installation, or `CRYSTAL_LIBRARY_PATH` shall include the new `lib` directory so that the `@[Link]` annotation will pick up the new `iconv-2.dll` on program builds. Otherwise, compiled programs will continue to look for the old `libiconv.dll`, and silently break if it is not in `%PATH%` (which is hopefully rare since most of the time Crystal itself is also in `%PATH%`). Cygwin's location is currently hardcoded to `C:\cygwin64`, the default installation location for 64-bit Cygwin. Cygwin itself doesn't have native ARM64 support, but cross-compilation should be possible by simply using the x64-to-ARM64 cross tools MSVC developer prompt on an ARM64 machine. --- .github/workflows/win.yml | 20 ++++++++-- .github/workflows/win_build_portable.yml | 7 +++- etc/win-ci/build-iconv.ps1 | 47 +++++------------------- etc/win-ci/cygwin-build-iconv.sh | 32 ++++++++++++++++ src/crystal/lib_iconv.cr | 2 +- 5 files changed, 66 insertions(+), 42 deletions(-) create mode 100644 etc/win-ci/cygwin-build-iconv.sh diff --git a/.github/workflows/win.yml b/.github/workflows/win.yml index a256a6806a3f..d4b9316ef1a2 100644 --- a/.github/workflows/win.yml +++ b/.github/workflows/win.yml @@ -21,6 +21,13 @@ jobs: - name: Enable Developer Command Prompt uses: ilammy/msvc-dev-cmd@0b201ec74fa43914dc39ae48a89fd1d8cb592756 # v1.13.0 + - name: Set up Cygwin + uses: cygwin/cygwin-install-action@006ad0b0946ca6d0a3ea2d4437677fa767392401 # v4 + with: + packages: make + install-dir: C:\cygwin64 + add-to-path: false + - name: Download Crystal source uses: actions/checkout@v4 @@ -50,7 +57,7 @@ jobs: run: .\etc\win-ci\build-pcre2.ps1 -BuildTree deps\pcre2 -Version 10.43 - name: Build libiconv if: steps.cache-libs.outputs.cache-hit != 'true' - run: .\etc\win-ci\build-iconv.ps1 -BuildTree deps\iconv + run: .\etc\win-ci\build-iconv.ps1 -BuildTree deps\iconv -Version 1.17 - name: Build libffi if: steps.cache-libs.outputs.cache-hit != 'true' run: .\etc\win-ci\build-ffi.ps1 -BuildTree deps\ffi -Version 3.3 @@ -93,6 +100,13 @@ jobs: - name: Enable Developer Command Prompt uses: ilammy/msvc-dev-cmd@0b201ec74fa43914dc39ae48a89fd1d8cb592756 # v1.13.0 + - name: Set up Cygwin + uses: cygwin/cygwin-install-action@006ad0b0946ca6d0a3ea2d4437677fa767392401 # v4 + with: + packages: make + install-dir: C:\cygwin64 + add-to-path: false + - name: Download Crystal source uses: actions/checkout@v4 @@ -112,7 +126,7 @@ jobs: libs/xml2-dynamic.lib dlls/pcre.dll dlls/pcre2-8.dll - dlls/libiconv.dll + dlls/iconv-2.dll dlls/gc.dll dlls/libffi.dll dlls/zlib1.dll @@ -131,7 +145,7 @@ jobs: run: .\etc\win-ci\build-pcre2.ps1 -BuildTree deps\pcre2 -Version 10.43 -Dynamic - name: Build libiconv if: steps.cache-dlls.outputs.cache-hit != 'true' - run: .\etc\win-ci\build-iconv.ps1 -BuildTree deps\iconv -Dynamic + run: .\etc\win-ci\build-iconv.ps1 -BuildTree deps\iconv -Version 1.17 -Dynamic - name: Build libffi if: steps.cache-dlls.outputs.cache-hit != 'true' run: .\etc\win-ci\build-ffi.ps1 -BuildTree deps\ffi -Version 3.3 -Dynamic diff --git a/.github/workflows/win_build_portable.yml b/.github/workflows/win_build_portable.yml index 12ee17da9e68..98c428ee5bad 100644 --- a/.github/workflows/win_build_portable.yml +++ b/.github/workflows/win_build_portable.yml @@ -23,6 +23,7 @@ jobs: - name: Install Crystal uses: crystal-lang/install-crystal@v1 + id: install-crystal with: crystal: "1.13.2" @@ -68,7 +69,7 @@ jobs: libs/xml2-dynamic.lib dlls/pcre.dll dlls/pcre2-8.dll - dlls/libiconv.dll + dlls/iconv-2.dll dlls/gc.dll dlls/libffi.dll dlls/zlib1.dll @@ -107,6 +108,10 @@ jobs: run: | echo "CRYSTAL_LIBRARY_PATH=$(pwd)\libs" >> ${env:GITHUB_ENV} echo "LLVM_CONFIG=$(pwd)\llvm\bin\llvm-config.exe" >> ${env:GITHUB_ENV} + # NOTE: the name of the libiconv DLL has changed, so we manually copy + # the new one to the existing Crystal installation; remove after + # updating the base compiler to 1.14 + cp dlls/iconv-2.dll ${{ steps.install-crystal.outputs.path }} - name: Build LLVM extensions run: make -f Makefile.win deps diff --git a/etc/win-ci/build-iconv.ps1 b/etc/win-ci/build-iconv.ps1 index 56d0417bd729..541066c6327f 100644 --- a/etc/win-ci/build-iconv.ps1 +++ b/etc/win-ci/build-iconv.ps1 @@ -1,47 +1,20 @@ param( [Parameter(Mandatory)] [string] $BuildTree, + [Parameter(Mandatory)] [string] $Version, [switch] $Dynamic ) . "$(Split-Path -Parent $MyInvocation.MyCommand.Path)\setup.ps1" [void](New-Item -Name (Split-Path -Parent $BuildTree) -ItemType Directory -Force) -Setup-Git -Path $BuildTree -Url https://github.com/pffang/libiconv-for-Windows.git -Ref 1353455a6c4e15c9db6865fd9c2bf7203b59c0ec # master@{2022-10-11} +Invoke-WebRequest "https://ftp.gnu.org/pub/gnu/libiconv/libiconv-${Version}.tar.gz" -OutFile libiconv.tar.gz +tar -xzf libiconv.tar.gz +mv libiconv-* $BuildTree +rm libiconv.tar.gz Run-InDirectory $BuildTree { - Replace-Text libiconv\include\iconv.h '__declspec (dllimport) ' '' - - echo ' - - $(MsbuildThisFileDirectory)\Override.props - - ' > 'Directory.Build.props' - - echo " - - false - - - - None - false - - - false - - - - - MultiThreadedDLL - - - " > 'Override.props' - - if ($Dynamic) { - MSBuild.exe /p:Platform=x64 /p:Configuration=Release libiconv.vcxproj - } else { - MSBuild.exe /p:Platform=x64 /p:Configuration=ReleaseStatic libiconv.vcxproj - } + $env:CHERE_INVOKING = 1 + & 'C:\cygwin64\bin\bash.exe' --login "$PSScriptRoot\cygwin-build-iconv.sh" "$Version" "$(if ($Dynamic) { 1 })" if (-not $?) { Write-Host "Error: Failed to build libiconv" -ForegroundColor Red Exit 1 @@ -49,8 +22,8 @@ Run-InDirectory $BuildTree { } if ($Dynamic) { - mv -Force $BuildTree\output\x64\Release\libiconv.lib libs\iconv-dynamic.lib - mv -Force $BuildTree\output\x64\Release\libiconv.dll dlls\ + mv -Force $BuildTree\iconv\lib\iconv.dll.lib libs\iconv-dynamic.lib + mv -Force $BuildTree\iconv\bin\iconv-2.dll dlls\ } else { - mv -Force $BuildTree\output\x64\ReleaseStatic\libiconvStatic.lib libs\iconv.lib + mv -Force $BuildTree\iconv\lib\iconv.lib libs\ } diff --git a/etc/win-ci/cygwin-build-iconv.sh b/etc/win-ci/cygwin-build-iconv.sh new file mode 100644 index 000000000000..a8507542e646 --- /dev/null +++ b/etc/win-ci/cygwin-build-iconv.sh @@ -0,0 +1,32 @@ +#!/bin/sh + +set -eo pipefail + +Version=$1 +Dynamic=$2 + +export PATH="$(pwd)/build-aux:$PATH" +export CC="$(pwd)/build-aux/compile cl -nologo" +export CXX="$(pwd)/build-aux/compile cl -nologo" +export AR="$(pwd)/build-aux/ar-lib lib" +export LD="link" +export NM="dumpbin -symbols" +export STRIP=":" +export RANLIB=":" +if [ -n "$Dynamic" ]; then + export CFLAGS="-MD" + export CXXFLAGS="-MD" + enable_shared=yes + enable_static=no +else + export CFLAGS="-MT" + export CXXFLAGS="-MT" + enable_shared=no + enable_static=yes +fi +export CPPFLAGS="-D_WIN32_WINNT=_WIN32_WINNT_WIN7 -I$(pwd)/iconv/include" +export LDFLAGS="-L$(pwd)/iconv/lib" + +./configure --host=x86_64-w64-mingw32 --prefix="$(pwd)/iconv" --enable-shared="${enable_shared}" --enable-static="${enable_static}" +make +make install diff --git a/src/crystal/lib_iconv.cr b/src/crystal/lib_iconv.cr index 5f1506758454..07100ff9c1dc 100644 --- a/src/crystal/lib_iconv.cr +++ b/src/crystal/lib_iconv.cr @@ -6,7 +6,7 @@ require "c/stddef" @[Link("iconv")] {% if compare_versions(Crystal::VERSION, "1.11.0-dev") >= 0 %} - @[Link(dll: "libiconv.dll")] + @[Link(dll: "iconv-2.dll")] {% end %} lib LibIconv type IconvT = Void* From f17b565cd5cd9bd96dbba85d1e94c7d2cacbf21b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Johannes=20M=C3=BCller?= Date: Tue, 17 Sep 2024 21:17:40 +0200 Subject: [PATCH 30/36] Enable runners from `runs-on.com` for Aarch64 CI (#15007) https://runs-on.com is a service that provisions machines on AWS on demand to run workflow jobs. It triggers when a job is tagged as `runs-on` and you can configure what kind of machine you like this to run on. The machine specification could likely use some fine tuning (we can use other instance types, and theoretically 8GB should be sufficient). But that'll be a follow-up. For now we know that this works. We expect this to be more price-efficient setup than renting a fixed server or a CI runner service. This patch also includes an update to the latest base image. The old arm base images were using Crystal 1.0 and some outdated libraries which caused problems on the new runners (#15005). The build images are based on the docker images from [84codes/crystal](https://hub.docker.com/r/84codes/crystal). --- .github/workflows/aarch64.yml | 26 +++++++++++++------------- 1 file changed, 13 insertions(+), 13 deletions(-) diff --git a/.github/workflows/aarch64.yml b/.github/workflows/aarch64.yml index da252904fa37..85a8af2c8b37 100644 --- a/.github/workflows/aarch64.yml +++ b/.github/workflows/aarch64.yml @@ -8,13 +8,13 @@ concurrency: jobs: aarch64-musl-build: - runs-on: [linux, ARM64] + runs-on: [runs-on, runner=2cpu-linux-arm64, "run-id=${{ github.run_id }}"] if: github.repository == 'crystal-lang/crystal' steps: - name: Download Crystal source uses: actions/checkout@v4 - name: Build Crystal - uses: docker://jhass/crystal:1.0.0-alpine-build + uses: docker://crystallang/crystal:1.13.2-alpine-84codes-build with: args: make crystal - name: Upload Crystal executable @@ -26,7 +26,7 @@ jobs: src/llvm/ext/llvm_ext.o aarch64-musl-test-stdlib: needs: aarch64-musl-build - runs-on: [linux, ARM64] + runs-on: [runs-on, runner=4cpu-linux-arm64, "family=m7g", ram=16, "run-id=${{ github.run_id }}"] if: github.repository == 'crystal-lang/crystal' steps: - name: Download Crystal source @@ -38,12 +38,12 @@ jobs: - name: Mark downloaded compiler as executable run: chmod +x .build/crystal - name: Run stdlib specs - uses: docker://jhass/crystal:1.0.0-alpine-build + uses: docker://crystallang/crystal:1.13.2-alpine-84codes-build with: - args: make std_spec FLAGS=-Duse_pcre + args: make std_spec aarch64-musl-test-compiler: needs: aarch64-musl-build - runs-on: [linux, ARM64] + runs-on: [runs-on, runner=4cpu-linux-arm64, "family=m7g", ram=16, "run-id=${{ github.run_id }}"] if: github.repository == 'crystal-lang/crystal' steps: - name: Download Crystal source @@ -55,17 +55,17 @@ jobs: - name: Mark downloaded compiler as executable run: chmod +x .build/crystal - name: Run compiler specs - uses: docker://jhass/crystal:1.0.0-alpine-build + uses: docker://crystallang/crystal:1.13.2-alpine-84codes-build with: args: make primitives_spec compiler_spec FLAGS=-Dwithout_ffi aarch64-gnu-build: - runs-on: [linux, ARM64] + runs-on: [runs-on, runner=2cpu-linux-arm64, "run-id=${{ github.run_id }}"] if: github.repository == 'crystal-lang/crystal' steps: - name: Download Crystal source uses: actions/checkout@v4 - name: Build Crystal - uses: docker://jhass/crystal:1.0.0-build + uses: docker://crystallang/crystal:1.13.2-ubuntu-84codes-build with: args: make crystal - name: Upload Crystal executable @@ -77,7 +77,7 @@ jobs: src/llvm/ext/llvm_ext.o aarch64-gnu-test-stdlib: needs: aarch64-gnu-build - runs-on: [linux, ARM64] + runs-on: [runs-on, runner=4cpu-linux-arm64, "family=m7g", ram=16, "run-id=${{ github.run_id }}"] if: github.repository == 'crystal-lang/crystal' steps: - name: Download Crystal source @@ -89,12 +89,12 @@ jobs: - name: Mark downloaded compiler as executable run: chmod +x .build/crystal - name: Run stdlib specs - uses: docker://jhass/crystal:1.0.0-build + uses: docker://crystallang/crystal:1.13.2-ubuntu-84codes-build with: args: make std_spec aarch64-gnu-test-compiler: needs: aarch64-gnu-build - runs-on: [linux, ARM64] + runs-on: [runs-on, runner=4cpu-linux-arm64, "family=m7g", ram=16, "run-id=${{ github.run_id }}"] if: github.repository == 'crystal-lang/crystal' steps: - name: Download Crystal source @@ -106,6 +106,6 @@ jobs: - name: Mark downloaded compiler as executable run: chmod +x .build/crystal - name: Run compiler specs - uses: docker://jhass/crystal:1.0.0-build + uses: docker://crystallang/crystal:1.13.2-ubuntu-84codes-build with: args: make primitives_spec compiler_spec From 80b2484c3fb95cae09bce60cde3930a0df826cf0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Johannes=20M=C3=BCller?= Date: Tue, 17 Sep 2024 21:17:40 +0200 Subject: [PATCH 31/36] Enable runners from `runs-on.com` for Aarch64 CI (#15007) https://runs-on.com is a service that provisions machines on AWS on demand to run workflow jobs. It triggers when a job is tagged as `runs-on` and you can configure what kind of machine you like this to run on. The machine specification could likely use some fine tuning (we can use other instance types, and theoretically 8GB should be sufficient). But that'll be a follow-up. For now we know that this works. We expect this to be more price-efficient setup than renting a fixed server or a CI runner service. This patch also includes an update to the latest base image. The old arm base images were using Crystal 1.0 and some outdated libraries which caused problems on the new runners (#15005). The build images are based on the docker images from [84codes/crystal](https://hub.docker.com/r/84codes/crystal). --- .github/workflows/aarch64.yml | 26 +++++++++++++------------- 1 file changed, 13 insertions(+), 13 deletions(-) diff --git a/.github/workflows/aarch64.yml b/.github/workflows/aarch64.yml index da252904fa37..85a8af2c8b37 100644 --- a/.github/workflows/aarch64.yml +++ b/.github/workflows/aarch64.yml @@ -8,13 +8,13 @@ concurrency: jobs: aarch64-musl-build: - runs-on: [linux, ARM64] + runs-on: [runs-on, runner=2cpu-linux-arm64, "run-id=${{ github.run_id }}"] if: github.repository == 'crystal-lang/crystal' steps: - name: Download Crystal source uses: actions/checkout@v4 - name: Build Crystal - uses: docker://jhass/crystal:1.0.0-alpine-build + uses: docker://crystallang/crystal:1.13.2-alpine-84codes-build with: args: make crystal - name: Upload Crystal executable @@ -26,7 +26,7 @@ jobs: src/llvm/ext/llvm_ext.o aarch64-musl-test-stdlib: needs: aarch64-musl-build - runs-on: [linux, ARM64] + runs-on: [runs-on, runner=4cpu-linux-arm64, "family=m7g", ram=16, "run-id=${{ github.run_id }}"] if: github.repository == 'crystal-lang/crystal' steps: - name: Download Crystal source @@ -38,12 +38,12 @@ jobs: - name: Mark downloaded compiler as executable run: chmod +x .build/crystal - name: Run stdlib specs - uses: docker://jhass/crystal:1.0.0-alpine-build + uses: docker://crystallang/crystal:1.13.2-alpine-84codes-build with: - args: make std_spec FLAGS=-Duse_pcre + args: make std_spec aarch64-musl-test-compiler: needs: aarch64-musl-build - runs-on: [linux, ARM64] + runs-on: [runs-on, runner=4cpu-linux-arm64, "family=m7g", ram=16, "run-id=${{ github.run_id }}"] if: github.repository == 'crystal-lang/crystal' steps: - name: Download Crystal source @@ -55,17 +55,17 @@ jobs: - name: Mark downloaded compiler as executable run: chmod +x .build/crystal - name: Run compiler specs - uses: docker://jhass/crystal:1.0.0-alpine-build + uses: docker://crystallang/crystal:1.13.2-alpine-84codes-build with: args: make primitives_spec compiler_spec FLAGS=-Dwithout_ffi aarch64-gnu-build: - runs-on: [linux, ARM64] + runs-on: [runs-on, runner=2cpu-linux-arm64, "run-id=${{ github.run_id }}"] if: github.repository == 'crystal-lang/crystal' steps: - name: Download Crystal source uses: actions/checkout@v4 - name: Build Crystal - uses: docker://jhass/crystal:1.0.0-build + uses: docker://crystallang/crystal:1.13.2-ubuntu-84codes-build with: args: make crystal - name: Upload Crystal executable @@ -77,7 +77,7 @@ jobs: src/llvm/ext/llvm_ext.o aarch64-gnu-test-stdlib: needs: aarch64-gnu-build - runs-on: [linux, ARM64] + runs-on: [runs-on, runner=4cpu-linux-arm64, "family=m7g", ram=16, "run-id=${{ github.run_id }}"] if: github.repository == 'crystal-lang/crystal' steps: - name: Download Crystal source @@ -89,12 +89,12 @@ jobs: - name: Mark downloaded compiler as executable run: chmod +x .build/crystal - name: Run stdlib specs - uses: docker://jhass/crystal:1.0.0-build + uses: docker://crystallang/crystal:1.13.2-ubuntu-84codes-build with: args: make std_spec aarch64-gnu-test-compiler: needs: aarch64-gnu-build - runs-on: [linux, ARM64] + runs-on: [runs-on, runner=4cpu-linux-arm64, "family=m7g", ram=16, "run-id=${{ github.run_id }}"] if: github.repository == 'crystal-lang/crystal' steps: - name: Download Crystal source @@ -106,6 +106,6 @@ jobs: - name: Mark downloaded compiler as executable run: chmod +x .build/crystal - name: Run compiler specs - uses: docker://jhass/crystal:1.0.0-build + uses: docker://crystallang/crystal:1.13.2-ubuntu-84codes-build with: args: make primitives_spec compiler_spec From 47cd33b259ac46533f8898bdd70fbd15fa9ab306 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Johannes=20M=C3=BCller?= Date: Fri, 6 Sep 2024 08:19:38 +0200 Subject: [PATCH 32/36] Fix use global paths in macro bodies (#14965) Macros inject code into other scopes. Paths are resolved in the expanded scope and there can be namespace conflicts. This fixes non-global paths in macro bodies that expand into uncontrolled scopes where namespaces could clash. This is a fixup for #14282 (released in 1.12.0). --- src/crystal/pointer_linked_list.cr | 4 ++-- src/ecr/macros.cr | 2 +- src/intrinsics.cr | 34 +++++++++++++++--------------- src/json/serialization.cr | 6 +++--- src/number.cr | 8 +++---- src/object.cr | 12 +++++------ src/slice.cr | 6 +++--- src/spec/dsl.cr | 4 ++-- src/spec/helpers/iterate.cr | 8 +++---- src/static_array.cr | 2 +- src/syscall/aarch64-linux.cr | 2 +- src/syscall/arm-linux.cr | 2 +- src/syscall/i386-linux.cr | 2 +- src/syscall/x86_64-linux.cr | 2 +- src/yaml/serialization.cr | 14 ++++++------ 15 files changed, 54 insertions(+), 54 deletions(-) diff --git a/src/crystal/pointer_linked_list.cr b/src/crystal/pointer_linked_list.cr index 03109979d662..cde9b0b79ddc 100644 --- a/src/crystal/pointer_linked_list.cr +++ b/src/crystal/pointer_linked_list.cr @@ -7,8 +7,8 @@ struct Crystal::PointerLinkedList(T) module Node macro included - property previous : Pointer(self) = Pointer(self).null - property next : Pointer(self) = Pointer(self).null + property previous : ::Pointer(self) = ::Pointer(self).null + property next : ::Pointer(self) = ::Pointer(self).null end end diff --git a/src/ecr/macros.cr b/src/ecr/macros.cr index 92c02cc4284a..5e051232271b 100644 --- a/src/ecr/macros.cr +++ b/src/ecr/macros.cr @@ -34,7 +34,7 @@ module ECR # ``` macro def_to_s(filename) def to_s(__io__ : IO) : Nil - ECR.embed {{filename}}, "__io__" + ::ECR.embed {{filename}}, "__io__" end end diff --git a/src/intrinsics.cr b/src/intrinsics.cr index c5ae837d8931..7cdc462ce543 100644 --- a/src/intrinsics.cr +++ b/src/intrinsics.cr @@ -179,7 +179,7 @@ end module Intrinsics macro debugtrap - LibIntrinsics.debugtrap + ::LibIntrinsics.debugtrap end def self.pause @@ -191,15 +191,15 @@ module Intrinsics end macro memcpy(dest, src, len, is_volatile) - LibIntrinsics.memcpy({{dest}}, {{src}}, {{len}}, {{is_volatile}}) + ::LibIntrinsics.memcpy({{dest}}, {{src}}, {{len}}, {{is_volatile}}) end macro memmove(dest, src, len, is_volatile) - LibIntrinsics.memmove({{dest}}, {{src}}, {{len}}, {{is_volatile}}) + ::LibIntrinsics.memmove({{dest}}, {{src}}, {{len}}, {{is_volatile}}) end macro memset(dest, val, len, is_volatile) - LibIntrinsics.memset({{dest}}, {{val}}, {{len}}, {{is_volatile}}) + ::LibIntrinsics.memset({{dest}}, {{val}}, {{len}}, {{is_volatile}}) end def self.read_cycle_counter @@ -263,43 +263,43 @@ module Intrinsics end macro countleading8(src, zero_is_undef) - LibIntrinsics.countleading8({{src}}, {{zero_is_undef}}) + ::LibIntrinsics.countleading8({{src}}, {{zero_is_undef}}) end macro countleading16(src, zero_is_undef) - LibIntrinsics.countleading16({{src}}, {{zero_is_undef}}) + ::LibIntrinsics.countleading16({{src}}, {{zero_is_undef}}) end macro countleading32(src, zero_is_undef) - LibIntrinsics.countleading32({{src}}, {{zero_is_undef}}) + ::LibIntrinsics.countleading32({{src}}, {{zero_is_undef}}) end macro countleading64(src, zero_is_undef) - LibIntrinsics.countleading64({{src}}, {{zero_is_undef}}) + ::LibIntrinsics.countleading64({{src}}, {{zero_is_undef}}) end macro countleading128(src, zero_is_undef) - LibIntrinsics.countleading128({{src}}, {{zero_is_undef}}) + ::LibIntrinsics.countleading128({{src}}, {{zero_is_undef}}) end macro counttrailing8(src, zero_is_undef) - LibIntrinsics.counttrailing8({{src}}, {{zero_is_undef}}) + ::LibIntrinsics.counttrailing8({{src}}, {{zero_is_undef}}) end macro counttrailing16(src, zero_is_undef) - LibIntrinsics.counttrailing16({{src}}, {{zero_is_undef}}) + ::LibIntrinsics.counttrailing16({{src}}, {{zero_is_undef}}) end macro counttrailing32(src, zero_is_undef) - LibIntrinsics.counttrailing32({{src}}, {{zero_is_undef}}) + ::LibIntrinsics.counttrailing32({{src}}, {{zero_is_undef}}) end macro counttrailing64(src, zero_is_undef) - LibIntrinsics.counttrailing64({{src}}, {{zero_is_undef}}) + ::LibIntrinsics.counttrailing64({{src}}, {{zero_is_undef}}) end macro counttrailing128(src, zero_is_undef) - LibIntrinsics.counttrailing128({{src}}, {{zero_is_undef}}) + ::LibIntrinsics.counttrailing128({{src}}, {{zero_is_undef}}) end def self.fshl8(a, b, count) : UInt8 @@ -343,14 +343,14 @@ module Intrinsics end macro va_start(ap) - LibIntrinsics.va_start({{ap}}) + ::LibIntrinsics.va_start({{ap}}) end macro va_end(ap) - LibIntrinsics.va_end({{ap}}) + ::LibIntrinsics.va_end({{ap}}) end end macro debugger - Intrinsics.debugtrap + ::Intrinsics.debugtrap end diff --git a/src/json/serialization.cr b/src/json/serialization.cr index b1eb86d15082..15d948f02f40 100644 --- a/src/json/serialization.cr +++ b/src/json/serialization.cr @@ -164,7 +164,7 @@ module JSON private def self.new_from_json_pull_parser(pull : ::JSON::PullParser) instance = allocate instance.initialize(__pull_for_json_serializable: pull) - GC.add_finalizer(instance) if instance.responds_to?(:finalize) + ::GC.add_finalizer(instance) if instance.responds_to?(:finalize) instance end @@ -422,8 +422,8 @@ module JSON # Try to find the discriminator while also getting the raw # string value of the parsed JSON, so then we can pass it # to the final type. - json = String.build do |io| - JSON.build(io) do |builder| + json = ::String.build do |io| + ::JSON.build(io) do |builder| builder.start_object pull.read_object do |key| if key == {{field.id.stringify}} diff --git a/src/number.cr b/src/number.cr index f7c82aa4cded..9d955c065df3 100644 --- a/src/number.cr +++ b/src/number.cr @@ -59,7 +59,7 @@ struct Number # :nodoc: macro expand_div(rhs_types, result_type) {% for rhs in rhs_types %} - @[AlwaysInline] + @[::AlwaysInline] def /(other : {{rhs}}) : {{result_type}} {{result_type}}.new(self) / {{result_type}}.new(other) end @@ -84,7 +84,7 @@ struct Number # [1, 2, 3, 4] of Int64 # : Array(Int64) # ``` macro [](*nums) - Array({{@type}}).build({{nums.size}}) do |%buffer| + ::Array({{@type}}).build({{nums.size}}) do |%buffer| {% for num, i in nums %} %buffer[{{i}}] = {{@type}}.new({{num}}) {% end %} @@ -113,7 +113,7 @@ struct Number # Slice[1_i64, 2_i64, 3_i64, 4_i64] # : Slice(Int64) # ``` macro slice(*nums, read_only = false) - %slice = Slice({{@type}}).new({{nums.size}}, read_only: {{read_only}}) + %slice = ::Slice({{@type}}).new({{nums.size}}, read_only: {{read_only}}) {% for num, i in nums %} %slice.to_unsafe[{{i}}] = {{@type}}.new!({{num}}) {% end %} @@ -139,7 +139,7 @@ struct Number # StaticArray[1_i64, 2_i64, 3_i64, 4_i64] # : StaticArray(Int64) # ``` macro static_array(*nums) - %array = uninitialized StaticArray({{@type}}, {{nums.size}}) + %array = uninitialized ::StaticArray({{@type}}, {{nums.size}}) {% for num, i in nums %} %array.to_unsafe[{{i}}] = {{@type}}.new!({{num}}) {% end %} diff --git a/src/object.cr b/src/object.cr index ba818ac2979e..800736687788 100644 --- a/src/object.cr +++ b/src/object.cr @@ -562,7 +562,7 @@ class Object def {{method_prefix}}\{{name.var.id}} : \{{name.type}} if (value = {{var_prefix}}\{{name.var.id}}).nil? - ::raise NilAssertionError.new("\{{@type}}\{{"{{doc_prefix}}".id}}\{{name.var.id}} cannot be nil") + ::raise ::NilAssertionError.new("\{{@type}}\{{"{{doc_prefix}}".id}}\{{name.var.id}} cannot be nil") else value end @@ -574,7 +574,7 @@ class Object def {{method_prefix}}\{{name.id}} if (value = {{var_prefix}}\{{name.id}}).nil? - ::raise NilAssertionError.new("\{{@type}}\{{"{{doc_prefix}}".id}}\{{name.id}} cannot be nil") + ::raise ::NilAssertionError.new("\{{@type}}\{{"{{doc_prefix}}".id}}\{{name.id}} cannot be nil") else value end @@ -1293,7 +1293,7 @@ class Object # wrapper.capitalize # => "Hello" # ``` macro delegate(*methods, to object) - {% if compare_versions(Crystal::VERSION, "1.12.0-dev") >= 0 %} + {% if compare_versions(::Crystal::VERSION, "1.12.0-dev") >= 0 %} {% eq_operators = %w(<= >= == != []= ===) %} {% for method in methods %} {% if method.id.ends_with?('=') && !eq_operators.includes?(method.id.stringify) %} @@ -1427,18 +1427,18 @@ class Object macro def_clone # Returns a copy of `self` with all instance variables cloned. def clone - \{% if @type < Reference && !@type.instance_vars.map(&.type).all? { |t| t == ::Bool || t == ::Char || t == ::Symbol || t == ::String || t < ::Number::Primitive } %} + \{% if @type < ::Reference && !@type.instance_vars.map(&.type).all? { |t| t == ::Bool || t == ::Char || t == ::Symbol || t == ::String || t < ::Number::Primitive } %} exec_recursive_clone do |hash| clone = \{{@type}}.allocate hash[object_id] = clone.object_id clone.initialize_copy(self) - GC.add_finalizer(clone) if clone.responds_to?(:finalize) + ::GC.add_finalizer(clone) if clone.responds_to?(:finalize) clone end \{% else %} clone = \{{@type}}.allocate clone.initialize_copy(self) - GC.add_finalizer(clone) if clone.responds_to?(:finalize) + ::GC.add_finalizer(clone) if clone.responds_to?(:finalize) clone \{% end %} end diff --git a/src/slice.cr b/src/slice.cr index 196a29a768dd..087679d37cb7 100644 --- a/src/slice.cr +++ b/src/slice.cr @@ -34,14 +34,14 @@ struct Slice(T) macro [](*args, read_only = false) # TODO: there should be a better way to check this, probably # asking if @type was instantiated or if T is defined - {% if @type.name != "Slice(T)" && T < Number %} + {% if @type.name != "Slice(T)" && T < ::Number %} {{T}}.slice({{args.splat(", ")}}read_only: {{read_only}}) {% else %} - %ptr = Pointer(typeof({{args.splat}})).malloc({{args.size}}) + %ptr = ::Pointer(typeof({{args.splat}})).malloc({{args.size}}) {% for arg, i in args %} %ptr[{{i}}] = {{arg}} {% end %} - Slice.new(%ptr, {{args.size}}, read_only: {{read_only}}) + ::Slice.new(%ptr, {{args.size}}, read_only: {{read_only}}) {% end %} end diff --git a/src/spec/dsl.cr b/src/spec/dsl.cr index 578076b86d69..d712aa59da4f 100644 --- a/src/spec/dsl.cr +++ b/src/spec/dsl.cr @@ -298,8 +298,8 @@ module Spec # If the "log" module is required it is configured to emit no entries by default. def log_setup defined?(::Log) do - if Log.responds_to?(:setup) - Log.setup_from_env(default_level: :none) + if ::Log.responds_to?(:setup) + ::Log.setup_from_env(default_level: :none) end end end diff --git a/src/spec/helpers/iterate.cr b/src/spec/helpers/iterate.cr index be302ebb49c2..7a70f83408ca 100644 --- a/src/spec/helpers/iterate.cr +++ b/src/spec/helpers/iterate.cr @@ -47,7 +47,7 @@ module Spec::Methods # See `.it_iterates` for details. macro assert_iterates_yielding(expected, method, *, infinite = false, tuple = false) %remaining = ({{expected}}).size - %ary = [] of typeof(Enumerable.element_type({{ expected }})) + %ary = [] of typeof(::Enumerable.element_type({{ expected }})) {{ method.id }} do |{% if tuple %}*{% end %}x| if %remaining == 0 if {{ infinite }} @@ -73,11 +73,11 @@ module Spec::Methods # # See `.it_iterates` for details. macro assert_iterates_iterator(expected, method, *, infinite = false) - %ary = [] of typeof(Enumerable.element_type({{ expected }})) + %ary = [] of typeof(::Enumerable.element_type({{ expected }})) %iter = {{ method.id }} ({{ expected }}).size.times do %v = %iter.next - if %v.is_a?(Iterator::Stop) + if %v.is_a?(::Iterator::Stop) # Compare the actual value directly. Since there are less # then expected values, the expectation will fail and raise. %ary.should eq({{ expected }}) @@ -86,7 +86,7 @@ module Spec::Methods %ary << %v end unless {{ infinite }} - %iter.next.should be_a(Iterator::Stop) + %iter.next.should be_a(::Iterator::Stop) end %ary.should eq({{ expected }}) diff --git a/src/static_array.cr b/src/static_array.cr index 2c09e21df166..3d00705bc21a 100644 --- a/src/static_array.cr +++ b/src/static_array.cr @@ -50,7 +50,7 @@ struct StaticArray(T, N) # * `Number.static_array` is a convenient alternative for designating a # specific numerical item type. macro [](*args) - %array = uninitialized StaticArray(typeof({{args.splat}}), {{args.size}}) + %array = uninitialized ::StaticArray(typeof({{args.splat}}), {{args.size}}) {% for arg, i in args %} %array.to_unsafe[{{i}}] = {{arg}} {% end %} diff --git a/src/syscall/aarch64-linux.cr b/src/syscall/aarch64-linux.cr index 5a61e8e7eed8..77b891fe2a7c 100644 --- a/src/syscall/aarch64-linux.cr +++ b/src/syscall/aarch64-linux.cr @@ -334,7 +334,7 @@ module Syscall end macro def_syscall(name, return_type, *args) - @[AlwaysInline] + @[::AlwaysInline] def self.{{name.id}}({{args.splat}}) : {{return_type}} ret = uninitialized {{return_type}} diff --git a/src/syscall/arm-linux.cr b/src/syscall/arm-linux.cr index 97119fc4b3f3..da349dd45301 100644 --- a/src/syscall/arm-linux.cr +++ b/src/syscall/arm-linux.cr @@ -409,7 +409,7 @@ module Syscall end macro def_syscall(name, return_type, *args) - @[AlwaysInline] + @[::AlwaysInline] def self.{{name.id}}({{args.splat}}) : {{return_type}} ret = uninitialized {{return_type}} diff --git a/src/syscall/i386-linux.cr b/src/syscall/i386-linux.cr index 843b2d1fd856..a0f94a51160a 100644 --- a/src/syscall/i386-linux.cr +++ b/src/syscall/i386-linux.cr @@ -445,7 +445,7 @@ module Syscall end macro def_syscall(name, return_type, *args) - @[AlwaysInline] + @[::AlwaysInline] def self.{{name.id}}({{args.splat}}) : {{return_type}} ret = uninitialized {{return_type}} diff --git a/src/syscall/x86_64-linux.cr b/src/syscall/x86_64-linux.cr index 1f01c9226658..5a63b6ee2e1a 100644 --- a/src/syscall/x86_64-linux.cr +++ b/src/syscall/x86_64-linux.cr @@ -368,7 +368,7 @@ module Syscall end macro def_syscall(name, return_type, *args) - @[AlwaysInline] + @[::AlwaysInline] def self.{{name.id}}({{args.splat}}) : {{return_type}} ret = uninitialized {{return_type}} diff --git a/src/yaml/serialization.cr b/src/yaml/serialization.cr index d5fae8dfe9c0..4a1521469dea 100644 --- a/src/yaml/serialization.cr +++ b/src/yaml/serialization.cr @@ -156,11 +156,11 @@ module YAML # Define a `new` directly in the included type, # so it overloads well with other possible initializes - def self.new(ctx : YAML::ParseContext, node : YAML::Nodes::Node) + def self.new(ctx : ::YAML::ParseContext, node : ::YAML::Nodes::Node) new_from_yaml_node(ctx, node) end - private def self.new_from_yaml_node(ctx : YAML::ParseContext, node : YAML::Nodes::Node) + private def self.new_from_yaml_node(ctx : ::YAML::ParseContext, node : ::YAML::Nodes::Node) ctx.read_alias(node, self) do |obj| return obj end @@ -170,7 +170,7 @@ module YAML ctx.record_anchor(node, instance) instance.initialize(__context_for_yaml_serializable: ctx, __node_for_yaml_serializable: node) - GC.add_finalizer(instance) if instance.responds_to?(:finalize) + ::GC.add_finalizer(instance) if instance.responds_to?(:finalize) instance end @@ -178,7 +178,7 @@ module YAML # so it can compete with other possible initializes macro inherited - def self.new(ctx : YAML::ParseContext, node : YAML::Nodes::Node) + def self.new(ctx : ::YAML::ParseContext, node : ::YAML::Nodes::Node) new_from_yaml_node(ctx, node) end end @@ -409,17 +409,17 @@ module YAML {% mapping.raise "Mapping argument must be a HashLiteral or a NamedTupleLiteral, not #{mapping.class_name.id}" %} {% end %} - def self.new(ctx : YAML::ParseContext, node : YAML::Nodes::Node) + def self.new(ctx : ::YAML::ParseContext, node : ::YAML::Nodes::Node) ctx.read_alias(node, \{{@type}}) do |obj| return obj end - unless node.is_a?(YAML::Nodes::Mapping) + unless node.is_a?(::YAML::Nodes::Mapping) node.raise "Expected YAML mapping, not #{node.class}" end node.each do |key, value| - next unless key.is_a?(YAML::Nodes::Scalar) && value.is_a?(YAML::Nodes::Scalar) + next unless key.is_a?(::YAML::Nodes::Scalar) && value.is_a?(::YAML::Nodes::Scalar) next unless key.value == {{field.id.stringify}} discriminator_value = value.value From 0f257ef57bdf34f156dd7bb5bcb0624258eb07cb Mon Sep 17 00:00:00 2001 From: Quinton Miller Date: Mon, 16 Sep 2024 19:56:21 +0800 Subject: [PATCH 33/36] Fix `Process.exec` stream redirection on Windows (#14986) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The following should print the compiler help message to the file `foo.txt`: ```crystal File.open("foo.txt", "w") do |f| Process.exec("crystal", output: f) end ``` It used to work on Windows in Crystal 1.12, but is now broken since 1.13. This is because `LibC._wexecvp` only inherits file descriptors in the C runtime, not arbitrary Win32 file handles; since we stopped calling `LibC._open_osfhandle`, the C runtime knows nothing about any reopened standard streams in Win32. Thus the above merely prints the help message to the standard output. This PR creates the missing C file descriptors right before `LibC._wexecvp`. It also fixes a different regression of #14947 where reconfiguring `STDIN.blocking` always fails. Co-authored-by: Johannes Müller --- spec/std/process_spec.cr | 21 ++++++++++ src/crystal/system/win32/process.cr | 59 +++++++++++++++++++-------- src/lib_c/x86_64-windows-msvc/c/io.cr | 5 ++- 3 files changed, 67 insertions(+), 18 deletions(-) diff --git a/spec/std/process_spec.cr b/spec/std/process_spec.cr index f067d2f5c775..f1437656fffa 100644 --- a/spec/std/process_spec.cr +++ b/spec/std/process_spec.cr @@ -465,6 +465,27 @@ pending_interpreted describe: Process do {% end %} describe ".exec" do + it "redirects STDIN and STDOUT to files", tags: %w[slow] do + with_tempfile("crystal-exec-stdin", "crystal-exec-stdout") do |stdin_path, stdout_path| + File.write(stdin_path, "foobar") + + status, _, _ = compile_and_run_source <<-CRYSTAL + command = #{stdin_to_stdout_command[0].inspect} + args = #{stdin_to_stdout_command[1].to_a} of String + stdin_path = #{stdin_path.inspect} + stdout_path = #{stdout_path.inspect} + File.open(stdin_path) do |input| + File.open(stdout_path, "w") do |output| + Process.exec(command, args, input: input, output: output) + end + end + CRYSTAL + + status.success?.should be_true + File.read(stdout_path).chomp.should eq("foobar") + end + end + it "gets error from exec" do expect_raises(File::NotFoundError, "Error executing process: 'foobarbaz'") do Process.exec("foobarbaz") diff --git a/src/crystal/system/win32/process.cr b/src/crystal/system/win32/process.cr index 05b2ea36584e..1817be9ee27f 100644 --- a/src/crystal/system/win32/process.cr +++ b/src/crystal/system/win32/process.cr @@ -326,9 +326,9 @@ struct Crystal::System::Process end private def self.try_replace(command_args, env, clear_env, input, output, error, chdir) - reopen_io(input, ORIGINAL_STDIN) - reopen_io(output, ORIGINAL_STDOUT) - reopen_io(error, ORIGINAL_STDERR) + old_input_fd = reopen_io(input, ORIGINAL_STDIN) + old_output_fd = reopen_io(output, ORIGINAL_STDOUT) + old_error_fd = reopen_io(error, ORIGINAL_STDERR) ENV.clear if clear_env env.try &.each do |key, val| @@ -351,11 +351,18 @@ struct Crystal::System::Process argv << Pointer(LibC::WCHAR).null LibC._wexecvp(command, argv) + + # exec failed; restore the original C runtime file descriptors + errno = Errno.value + LibC._dup2(old_input_fd, 0) + LibC._dup2(old_output_fd, 1) + LibC._dup2(old_error_fd, 2) + errno end def self.replace(command_args, env, clear_env, input, output, error, chdir) : NoReturn - try_replace(command_args, env, clear_env, input, output, error, chdir) - raise_exception_from_errno(command_args.is_a?(String) ? command_args : command_args[0]) + errno = try_replace(command_args, env, clear_env, input, output, error, chdir) + raise_exception_from_errno(command_args.is_a?(String) ? command_args : command_args[0], errno) end private def self.raise_exception_from_errno(command, errno = Errno.value) @@ -367,21 +374,41 @@ struct Crystal::System::Process end end + # Replaces the C standard streams' file descriptors, not Win32's, since + # `try_replace` uses the C `LibC._wexecvp` and only cares about the former. + # Returns a duplicate of the original file descriptor private def self.reopen_io(src_io : IO::FileDescriptor, dst_io : IO::FileDescriptor) - src_io = to_real_fd(src_io) + unless src_io.system_blocking? + raise IO::Error.new("Non-blocking streams are not supported in `Process.exec`", target: src_io) + end - dst_io.reopen(src_io) - dst_io.blocking = true - dst_io.close_on_exec = false - end + src_fd = + case src_io + when STDIN then 0 + when STDOUT then 1 + when STDERR then 2 + else + LibC._open_osfhandle(src_io.windows_handle, 0) + end - private def self.to_real_fd(fd : IO::FileDescriptor) - case fd - when STDIN then ORIGINAL_STDIN - when STDOUT then ORIGINAL_STDOUT - when STDERR then ORIGINAL_STDERR - else fd + dst_fd = + case dst_io + when ORIGINAL_STDIN then 0 + when ORIGINAL_STDOUT then 1 + when ORIGINAL_STDERR then 2 + else + raise "BUG: Invalid destination IO" + end + + return src_fd if dst_fd == src_fd + + orig_src_fd = LibC._dup(src_fd) + + if LibC._dup2(src_fd, dst_fd) == -1 + raise IO::Error.from_errno("Failed to replace C file descriptor", target: dst_io) end + + orig_src_fd end def self.chroot(path) diff --git a/src/lib_c/x86_64-windows-msvc/c/io.cr b/src/lib_c/x86_64-windows-msvc/c/io.cr index 75da8c18e5b9..ccbaa15f2d1b 100644 --- a/src/lib_c/x86_64-windows-msvc/c/io.cr +++ b/src/lib_c/x86_64-windows-msvc/c/io.cr @@ -2,12 +2,13 @@ require "c/stdint" lib LibC fun _wexecvp(cmdname : WCHAR*, argv : WCHAR**) : IntPtrT + fun _open_osfhandle(osfhandle : HANDLE, flags : LibC::Int) : LibC::Int + fun _dup(fd : Int) : Int + fun _dup2(fd1 : Int, fd2 : Int) : Int # unused - fun _open_osfhandle(osfhandle : HANDLE, flags : LibC::Int) : LibC::Int fun _get_osfhandle(fd : Int) : IntPtrT fun _close(fd : Int) : Int - fun _dup2(fd1 : Int, fd2 : Int) : Int fun _isatty(fd : Int) : Int fun _write(fd : Int, buffer : UInt8*, count : UInt) : Int fun _read(fd : Int, buffer : UInt8*, count : UInt) : Int From ced75401dac79f9abfe77e2aa6181a62c1283107 Mon Sep 17 00:00:00 2001 From: Quinton Miller Date: Sun, 25 Aug 2024 19:15:11 +0800 Subject: [PATCH 34/36] Fix `String#index` and `#rindex` for `Char::REPLACEMENT` (#14937) If the string consists only of ASCII characters and invalid UTF-8 byte sequences, all the latter should correspond to `Char::REPLACEMENT`, and so `#index` and `#rindex` should detect them, but this was broken since #14461. --- spec/std/string_spec.cr | 6 ++++++ src/string.cr | 36 ++++++++++++++++++++++++++++-------- 2 files changed, 34 insertions(+), 8 deletions(-) diff --git a/spec/std/string_spec.cr b/spec/std/string_spec.cr index 00310bfcbc47..9c4f3cfc5d12 100644 --- a/spec/std/string_spec.cr +++ b/spec/std/string_spec.cr @@ -945,6 +945,7 @@ describe "String" do it { "日本語".index('本').should eq(1) } it { "bar".index('あ').should be_nil } it { "あいう_えお".index('_').should eq(3) } + it { "xyz\xFFxyz".index('\u{FFFD}').should eq(3) } describe "with offset" do it { "foobarbaz".index('a', 5).should eq(7) } @@ -952,6 +953,8 @@ describe "String" do it { "foo".index('g', 1).should be_nil } it { "foo".index('g', -20).should be_nil } it { "日本語日本語".index('本', 2).should eq(4) } + it { "xyz\xFFxyz".index('\u{FFFD}', 2).should eq(3) } + it { "xyz\xFFxyz".index('\u{FFFD}', 4).should be_nil } # Check offset type it { "foobarbaz".index('a', 5_i64).should eq(7) } @@ -1094,6 +1097,7 @@ describe "String" do it { "foobar".rindex('g').should be_nil } it { "日本語日本語".rindex('本').should eq(4) } it { "あいう_えお".rindex('_').should eq(3) } + it { "xyz\xFFxyz".rindex('\u{FFFD}').should eq(3) } describe "with offset" do it { "bbbb".rindex('b', 2).should eq(2) } @@ -1106,6 +1110,8 @@ describe "String" do it { "faobar".rindex('a', 3).should eq(1) } it { "faobarbaz".rindex('a', -3).should eq(4) } it { "日本語日本語".rindex('本', 3).should eq(1) } + it { "xyz\xFFxyz".rindex('\u{FFFD}', 4).should eq(3) } + it { "xyz\xFFxyz".rindex('\u{FFFD}', 2).should be_nil } # Check offset type it { "bbbb".rindex('b', 2_i64).should eq(2) } diff --git a/src/string.cr b/src/string.cr index d3bc7d6998b2..ab107c454e8c 100644 --- a/src/string.cr +++ b/src/string.cr @@ -3335,11 +3335,21 @@ class String def index(search : Char, offset = 0) : Int32? # If it's ASCII we can delegate to slice if single_byte_optimizable? - # With `single_byte_optimizable?` there are only ASCII characters and invalid UTF-8 byte - # sequences and we can immediately reject any non-ASCII codepoint. - return unless search.ascii? + # With `single_byte_optimizable?` there are only ASCII characters and + # invalid UTF-8 byte sequences, and we can reject anything that is neither + # ASCII nor the replacement character. + case search + when .ascii? + return to_slice.fast_index(search.ord.to_u8!, offset) + when Char::REPLACEMENT + offset.upto(bytesize - 1) do |i| + if to_unsafe[i] >= 0x80 + return i.to_i + end + end + end - return to_slice.fast_index(search.ord.to_u8, offset) + return nil end offset += size if offset < 0 @@ -3455,11 +3465,21 @@ class String def rindex(search : Char, offset = size - 1) # If it's ASCII we can delegate to slice if single_byte_optimizable? - # With `single_byte_optimizable?` there are only ASCII characters and invalid UTF-8 byte - # sequences and we can immediately reject any non-ASCII codepoint. - return unless search.ascii? + # With `single_byte_optimizable?` there are only ASCII characters and + # invalid UTF-8 byte sequences, and we can reject anything that is neither + # ASCII nor the replacement character. + case search + when .ascii? + return to_slice.rindex(search.ord.to_u8!, offset) + when Char::REPLACEMENT + offset.downto(0) do |i| + if to_unsafe[i] >= 0x80 + return i.to_i + end + end + end - return to_slice.rindex(search.ord.to_u8, offset) + return nil end offset += size if offset < 0 From d14d04562125ee1f2c3985d756c5f0e4cd68a9c2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Johannes=20M=C3=BCller?= Date: Wed, 18 Sep 2024 15:43:39 +0200 Subject: [PATCH 35/36] Changelog for 1.13.3 (#14991) --- CHANGELOG.md | 24 ++++++++++++++++++++++++ shard.yml | 2 +- src/SOURCE_DATE_EPOCH | 2 +- src/VERSION | 2 +- 4 files changed, 27 insertions(+), 3 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index f97d0bedeb1b..341586a8fb95 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,29 @@ # Changelog +## [1.13.3] (2024-09-18) + +[1.13.3]: https://github.com/crystal-lang/crystal/releases/1.13.3 + +### Bugfixes + +#### stdlib + +- **[regression]** Fix use global paths in macro bodies ([#14965], thanks @straight-shoota) +- *(system)* **[regression]** Fix `Process.exec` stream redirection on Windows ([#14986], thanks @HertzDevil) +- *(text)* **[regression]** Fix `String#index` and `#rindex` for `Char::REPLACEMENT` ([#14937], thanks @HertzDevil) + +[#14965]: https://github.com/crystal-lang/crystal/pull/14965 +[#14986]: https://github.com/crystal-lang/crystal/pull/14986 +[#14937]: https://github.com/crystal-lang/crystal/pull/14937 + +### Infrastructure + +- Changelog for 1.13.3 ([#14991], thanks @straight-shoota) +- *(ci)* Enable runners from `runs-on.com` for Aarch64 CI ([#15007], thanks @straight-shoota) + +[#14991]: https://github.com/crystal-lang/crystal/pull/14991 +[#15007]: https://github.com/crystal-lang/crystal/pull/15007 + ## [1.13.2] (2024-08-20) [1.13.2]: https://github.com/crystal-lang/crystal/releases/1.13.2 diff --git a/shard.yml b/shard.yml index 0dd8c2abf3a1..6463a5681c65 100644 --- a/shard.yml +++ b/shard.yml @@ -1,5 +1,5 @@ name: crystal -version: 1.13.2 +version: 1.13.3 authors: - Crystal Core Team diff --git a/src/SOURCE_DATE_EPOCH b/src/SOURCE_DATE_EPOCH index 0ea6bd82d669..13a79f624e9d 100644 --- a/src/SOURCE_DATE_EPOCH +++ b/src/SOURCE_DATE_EPOCH @@ -1 +1 @@ -1724112000 +1726617600 diff --git a/src/VERSION b/src/VERSION index 61ce01b30118..01b7568230eb 100644 --- a/src/VERSION +++ b/src/VERSION @@ -1 +1 @@ -1.13.2 +1.13.3 From 626e8f7c55cc573c321476b5fc4dd2e0986167bb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Johannes=20M=C3=BCller?= Date: Wed, 18 Sep 2024 23:01:49 +0200 Subject: [PATCH 36/36] Remove `XML::Error.errors` (#14936) Co-authored-by: Beta Ziliani --- spec/std/xml/reader_spec.cr | 10 ---------- src/xml/error.cr | 15 +-------------- src/xml/reader.cr | 4 +--- 3 files changed, 2 insertions(+), 27 deletions(-) diff --git a/spec/std/xml/reader_spec.cr b/spec/std/xml/reader_spec.cr index d89593620970..4ec3d8cddc5c 100644 --- a/spec/std/xml/reader_spec.cr +++ b/spec/std/xml/reader_spec.cr @@ -577,15 +577,5 @@ module XML reader.errors.map(&.to_s).should eq ["Opening and ending tag mismatch: people line 1 and foo"] end - - it "adds errors to `XML::Error.errors` (deprecated)" do - XML::Error.errors # clear class error list - - reader = XML::Reader.new(%()) - reader.read - reader.expand? - - XML::Error.errors.try(&.map(&.to_s)).should eq ["Opening and ending tag mismatch: people line 1 and foo"] - end end end diff --git a/src/xml/error.cr b/src/xml/error.cr index 868dfeb4bd00..389aa53910c2 100644 --- a/src/xml/error.cr +++ b/src/xml/error.cr @@ -11,22 +11,9 @@ class XML::Error < Exception super(message) end - @@errors = [] of self - - # :nodoc: - protected def self.add_errors(errors) - @@errors.concat(errors) - end - @[Deprecated("This class accessor is deprecated. XML errors are accessible directly in the respective context via `XML::Reader#errors` and `XML::Node#errors`.")] def self.errors : Array(XML::Error)? - if @@errors.empty? - nil - else - errors = @@errors.dup - @@errors.clear - errors - end + {% raise "`XML::Error.errors` was removed because it leaks memory when it's not used. XML errors are accessible directly in the respective context via `XML::Reader#errors` and `XML::Node#errors`.\nSee https://github.com/crystal-lang/crystal/issues/14934 for details. " %} end def self.collect(errors, &) diff --git a/src/xml/reader.cr b/src/xml/reader.cr index decdd8468185..d4dbe91f7eeb 100644 --- a/src/xml/reader.cr +++ b/src/xml/reader.cr @@ -198,9 +198,7 @@ class XML::Reader end private def collect_errors(&) - Error.collect(@errors) { yield }.tap do - Error.add_errors(@errors) - end + Error.collect(@errors) { yield } end private def check_no_null_byte(attribute)