diff --git a/spec/std/file_spec.cr b/spec/std/file_spec.cr index 9b3b7bfce6d3..2151ad883ab3 100644 --- a/spec/std/file_spec.cr +++ b/spec/std/file_spec.cr @@ -1,5 +1,6 @@ require "./spec_helper" require "../support/thread" +require "wait_group" private def it_raises_on_null_byte(operation, file = __FILE__, line = __LINE__, end_line = __END_LINE__, &block) it "errors on #{operation}", file, line, end_line do @@ -47,112 +48,64 @@ describe "File" do end end - describe "blocking" do - it "opens regular file as blocking" do - with_tempfile("regular") do |path| - File.open(path, "w") do |file| - file.blocking.should be_true - end + {% if LibC.has_method?(:mkfifo) %} + # interpreter doesn't support threads yet (#14287) + pending_interpreted "can read/write fifo file without blocking" do + path = File.tempname("chardev") + ret = LibC.mkfifo(path, File::DEFAULT_CREATE_PERMISSIONS) + raise RuntimeError.from_errno("mkfifo") unless ret == 0 - File.open(path, "w", blocking: nil) do |file| - file.blocking.should be_true - end - end - end + # FIXME: open(2) will block when opening a fifo file until another thread + # or process also opened the file + writer = nil + new_thread { writer = File.new(path, "w") } - it "opens regular file as non-blocking" do - with_tempfile("regular") do |path| - File.open(path, "w", blocking: false) do |file| - file.blocking.should be_false - end - end - end + rbuf = Bytes.new(5120) + wbuf = Bytes.new(5120) + Random::DEFAULT.random_bytes(wbuf) - {% if flag?(:unix) %} - if File.exists?("/dev/tty") - it "opens character device" do - File.open("/dev/tty", "r") do |file| - file.blocking.should be_true - end - - File.open("/dev/tty", "r", blocking: false) do |file| - file.blocking.should be_false - end - - File.open("/dev/tty", "r", blocking: nil) do |file| - file.blocking.should be_false + File.open(path, "r") do |reader| + WaitGroup.wait do |wg| + wg.spawn do + 64.times do |i| + reader.read_fully(rbuf) + end end - rescue File::Error - # The TTY may not be available (e.g. Docker CI) - end - end - - {% if LibC.has_method?(:mkfifo) %} - # interpreter doesn't support threads yet (#14287) - pending_interpreted "opens fifo file as non-blocking" do - path = File.tempname("chardev") - ret = LibC.mkfifo(path, File::DEFAULT_CREATE_PERMISSIONS) - raise RuntimeError.from_errno("mkfifo") unless ret == 0 - # FIXME: open(2) will block when opening a fifo file until another - # thread or process also opened the file; we should pass - # O_NONBLOCK to the open(2) call itself, not afterwards - file = nil - new_thread { file = File.new(path, "w", blocking: nil) } - - begin - File.open(path, "r", blocking: false) do |file| - file.blocking.should be_false + wg.spawn do + 64.times do |i| + writer.not_nil!.write(wbuf) end - ensure - File.delete(path) - file.try(&.close) + writer.not_nil!.close end end - {% end %} - {% end %} - - it "reads non-blocking file" do - File.open(datapath("test_file.txt"), "r", blocking: false) do |f| - f.gets_to_end.should eq("Hello World\n" * 20) end - end - it "writes and reads large non-blocking file" do - with_tempfile("non-blocking-io.txt") do |path| - File.open(path, "w+", blocking: false) do |f| - f.puts "Hello World\n" * 40000 - f.pos = 0 - f.gets_to_end.should eq("Hello World\n" * 40000) - end - end - end - - it "can append non-blocking to an existing file" do - with_tempfile("append-existing.txt") do |path| - File.write(path, "hello") - File.write(path, " world", mode: "a", blocking: false) - File.read(path).should eq("hello world") - end + rbuf.should eq(wbuf) + ensure + File.delete(path) if path + writer.try(&.close) end + {% end %} - it "returns the actual position after non-blocking append" do - with_tempfile("delete-file.txt") do |filename| - File.write(filename, "hello") - - File.open(filename, "a", blocking: false) do |file| - file.tell.should eq(0) + # This test verifies that the workaround for a win32 bug with the O_APPEND + # equivalent with OVERLAPPED operations is working as expected. + it "returns the actual position after append" do + with_tempfile("delete-file.txt") do |filename| + File.write(filename, "hello") - file.write "12345".to_slice - file.tell.should eq(10) + File.open(filename, "a") do |file| + file.tell.should eq(0) - file.seek(5, IO::Seek::Set) - file.write "6789".to_slice - file.tell.should eq(14) - end + file.write "12345".to_slice + file.tell.should eq(10) - File.read(filename).should eq("hello123456789") + file.seek(5, IO::Seek::Set) + file.write "6789".to_slice + file.tell.should eq(14) end + + File.read(filename).should eq("hello123456789") end end diff --git a/src/crystal/event_loop/iocp.cr b/src/crystal/event_loop/iocp.cr index 8f06889d98c5..015213bde215 100644 --- a/src/crystal/event_loop/iocp.cr +++ b/src/crystal/event_loop/iocp.cr @@ -237,14 +237,16 @@ class Crystal::EventLoop::IOCP < Crystal::EventLoop def pipe(read_blocking : Bool?, write_blocking : Bool?) : {IO::FileDescriptor, IO::FileDescriptor} r, w = System::FileDescriptor.system_pipe(!!read_blocking, !!write_blocking) + create_completion_port(LibC::HANDLE.new(r)) unless read_blocking + create_completion_port(LibC::HANDLE.new(w)) unless write_blocking { - IO::FileDescriptor.new(r, !!read_blocking), - IO::FileDescriptor.new(w, !!write_blocking), + IO::FileDescriptor.new(handle: r, blocking: !!read_blocking), + IO::FileDescriptor.new(handle: w, blocking: !!write_blocking), } end def open(path : String, flags : Int32, permissions : File::Permissions, blocking : Bool?) : {System::FileDescriptor::Handle, Bool} | WinError - access, disposition, attributes = System::File.posix_to_open_opts(flags, permissions, blocking) + access, disposition, attributes = System::File.posix_to_open_opts(flags, permissions, !!blocking) handle = LibC.CreateFileW( System.to_wstr(path), diff --git a/src/crystal/event_loop/libevent.cr b/src/crystal/event_loop/libevent.cr index 271732fa5fda..5f63452c86f3 100644 --- a/src/crystal/event_loop/libevent.cr +++ b/src/crystal/event_loop/libevent.cr @@ -118,9 +118,11 @@ class Crystal::EventLoop::LibEvent < Crystal::EventLoop def pipe(read_blocking : Bool?, write_blocking : Bool?) : {IO::FileDescriptor, IO::FileDescriptor} r, w = System::FileDescriptor.system_pipe + System::FileDescriptor.set_blocking(r, false) unless read_blocking + System::FileDescriptor.set_blocking(w, false) unless write_blocking { - IO::FileDescriptor.new(r, !!read_blocking), - IO::FileDescriptor.new(w, !!write_blocking), + IO::FileDescriptor.new(handle: r), + IO::FileDescriptor.new(handle: w), } end @@ -130,13 +132,8 @@ class Crystal::EventLoop::LibEvent < Crystal::EventLoop fd = LibC.open(path, flags | LibC::O_CLOEXEC, permissions) return Errno.value if fd == -1 - blocking = !System::File.special_type?(fd) if blocking.nil? - unless blocking - status_flags = System::FileDescriptor.fcntl(fd, LibC::F_GETFL) - System::FileDescriptor.fcntl(fd, LibC::F_SETFL, status_flags | LibC::O_NONBLOCK) - end - - {fd, blocking} + System::FileDescriptor.set_blocking(fd, false) unless blocking + {fd, !!blocking} end def read(file_descriptor : Crystal::System::FileDescriptor, slice : Bytes) : Int32 diff --git a/src/crystal/event_loop/polling.cr b/src/crystal/event_loop/polling.cr index b9226e60c268..b4bbea32b640 100644 --- a/src/crystal/event_loop/polling.cr +++ b/src/crystal/event_loop/polling.cr @@ -164,9 +164,11 @@ abstract class Crystal::EventLoop::Polling < Crystal::EventLoop def pipe(read_blocking : Bool?, write_blocking : Bool?) : {IO::FileDescriptor, IO::FileDescriptor} r, w = System::FileDescriptor.system_pipe + System::FileDescriptor.set_blocking(r, false) unless read_blocking + System::FileDescriptor.set_blocking(w, false) unless write_blocking { - IO::FileDescriptor.new(r, !!read_blocking), - IO::FileDescriptor.new(w, !!write_blocking), + IO::FileDescriptor.new(handle: r), + IO::FileDescriptor.new(handle: w), } end @@ -176,13 +178,8 @@ abstract class Crystal::EventLoop::Polling < Crystal::EventLoop fd = LibC.open(path, flags | LibC::O_CLOEXEC, permissions) return Errno.value if fd == -1 - blocking = !System::File.special_type?(fd) if blocking.nil? - unless blocking - status_flags = System::FileDescriptor.fcntl(fd, LibC::F_GETFL) - System::FileDescriptor.fcntl(fd, LibC::F_SETFL, status_flags | LibC::O_NONBLOCK) - end - - {fd, blocking} + System::FileDescriptor.set_blocking(fd, false) unless blocking + {fd, !!blocking} end def read(file_descriptor : System::FileDescriptor, slice : Bytes) : Int32 diff --git a/src/crystal/system/process.cr b/src/crystal/system/process.cr index 11bd2363cb89..48ffcb2382a3 100644 --- a/src/crystal/system/process.cr +++ b/src/crystal/system/process.cr @@ -84,9 +84,9 @@ struct Crystal::System::Process end module Crystal::System - ORIGINAL_STDIN = IO::FileDescriptor.new(Crystal::System::FileDescriptor::STDIN_HANDLE, blocking: true) - ORIGINAL_STDOUT = IO::FileDescriptor.new(Crystal::System::FileDescriptor::STDOUT_HANDLE, blocking: true) - ORIGINAL_STDERR = IO::FileDescriptor.new(Crystal::System::FileDescriptor::STDERR_HANDLE, blocking: true) + ORIGINAL_STDIN = IO::FileDescriptor.new(handle: Crystal::System::FileDescriptor::STDIN_HANDLE, blocking: true) + ORIGINAL_STDOUT = IO::FileDescriptor.new(handle: Crystal::System::FileDescriptor::STDOUT_HANDLE, blocking: true) + ORIGINAL_STDERR = IO::FileDescriptor.new(handle: Crystal::System::FileDescriptor::STDERR_HANDLE, blocking: true) end {% if flag?(:wasi) %} diff --git a/src/crystal/system/unix/file_descriptor.cr b/src/crystal/system/unix/file_descriptor.cr index 41e856623b59..f671fd5da842 100644 --- a/src/crystal/system/unix/file_descriptor.cr +++ b/src/crystal/system/unix/file_descriptor.cr @@ -24,14 +24,18 @@ module Crystal::System::FileDescriptor end private def system_blocking=(value) - current_flags = fcntl(LibC::F_GETFL) + FileDescriptor.set_blocking(fd, value) + end + + protected def self.set_blocking(fd, value) + current_flags = fcntl(fd, LibC::F_GETFL) new_flags = current_flags if value new_flags &= ~LibC::O_NONBLOCK else new_flags |= LibC::O_NONBLOCK end - fcntl(LibC::F_SETFL, new_flags) unless new_flags == current_flags + fcntl(fd, LibC::F_SETFL, new_flags) unless new_flags == current_flags end protected def system_blocking_init(blocking : Bool?) diff --git a/src/crystal/system/unix/process.cr b/src/crystal/system/unix/process.cr index 59714a71bf50..36d8da14be21 100644 --- a/src/crystal/system/unix/process.cr +++ b/src/crystal/system/unix/process.cr @@ -361,7 +361,7 @@ struct Crystal::System::Process ret = LibC.dup2(src_io.fd, dst_io.fd) raise IO::Error.from_errno("dup2") if ret == -1 - dst_io.blocking = true + FileDescriptor.set_blocking(dst_io.fd, true) dst_io.close_on_exec = false end end diff --git a/src/crystal/system/unix/socket.cr b/src/crystal/system/unix/socket.cr index 5f7307c46cc6..301ca9b1f11e 100644 --- a/src/crystal/system/unix/socket.cr +++ b/src/crystal/system/unix/socket.cr @@ -160,13 +160,17 @@ module Crystal::System::Socket end private def system_blocking=(value) - flags = fcntl(LibC::F_GETFL) + Socket.set_blocking(fd, value) + end + + def self.set_blocking(fd, value) + flags = fcntl(fd, LibC::F_GETFL) if value flags &= ~LibC::O_NONBLOCK else flags |= LibC::O_NONBLOCK end - fcntl(LibC::F_SETFL, flags) + fcntl(fd, LibC::F_SETFL, flags) end private def system_close_on_exec? diff --git a/src/crystal/system/win32/file_descriptor.cr b/src/crystal/system/win32/file_descriptor.cr index d6a9d8fc4ab6..d38df58b036e 100644 --- a/src/crystal/system/win32/file_descriptor.cr +++ b/src/crystal/system/win32/file_descriptor.cr @@ -99,7 +99,7 @@ module Crystal::System::FileDescriptor end private def system_blocking=(blocking) - unless blocking == self.blocking + unless blocking == system_blocking? raise IO::Error.new("Cannot reconfigure `IO::FileDescriptor#blocking` after creation") end end diff --git a/src/file.cr b/src/file.cr index 429eb8b3dab4..f2c581bd71fe 100644 --- a/src/file.cr +++ b/src/file.cr @@ -128,7 +128,7 @@ class File < IO::FileDescriptor # This constructor is for constructors to be able to initialize a `File` with # a *path* and *fd*. The *blocking* param is informational and must reflect # the non/blocking state of the underlying fd. - private def initialize(@path, fd : Int, mode = "", blocking = true, encoding = nil, invalid = nil) + private def initialize(@path, fd : Int, mode = "", blocking = nil, encoding = nil, invalid = nil) super(handle: fd) system_init(mode, blocking) set_encoding(encoding, invalid: invalid) if encoding @@ -156,19 +156,12 @@ class File < IO::FileDescriptor # Line endings are preserved on all platforms. The `b` mode flag has no # effect; it is provided only for POSIX compatibility. # - # *blocking* is set to `true` by default because system event queues (e.g. - # epoll, kqueue) will always report the file descriptor of regular disk files - # as ready. - # - # *blocking* must be set to `false` on POSIX targets when the file to open - # isn't a regular file but a character device (e.g. `/dev/tty`) or fifo. These - # files depend on another process or thread to also be reading or writing, and - # system event queues will properly report readiness. - # - # *blocking* may also be set to `nil` in which case the blocking or - # non-blocking flag will be determined automatically, at the expense of an - # additional syscall. - def self.new(filename : Path | String, mode = "r", perm = DEFAULT_CREATE_PERMISSIONS, encoding = nil, invalid = nil, blocking = true) + # NOTE: The *blocking* arg is deprecated since Crystal 1.17. It used to be + # true by default to denote a regular disk file (always ready in system event + # loops) and could be set to false when the file was known to be a fifo, pipe, + # or character device (for example `/dev/tty`). Event loops now properly + # handle this and there are no reasons to change the blocking mode anymore. + def self.new(filename : Path | String, mode = "r", perm = DEFAULT_CREATE_PERMISSIONS, encoding = nil, invalid = nil, blocking = nil) filename = filename.to_s fd, blocking = Crystal::System::File.open(filename, mode, perm: perm, blocking: blocking) new(filename, fd, mode, blocking, encoding, invalid) @@ -505,7 +498,9 @@ class File < IO::FileDescriptor # permissions may be set using the *perm* parameter. # # See `self.new` for what *mode* can be. - def self.open(filename : Path | String, mode = "r", perm = DEFAULT_CREATE_PERMISSIONS, encoding = nil, invalid = nil, blocking = true) : self + # + # NOTE: The *blocking* arg is deprecated since Crystal 1.17. + def self.open(filename : Path | String, mode = "r", perm = DEFAULT_CREATE_PERMISSIONS, encoding = nil, invalid = nil, blocking = nil) : self new filename, mode, perm, encoding, invalid, blocking end @@ -514,7 +509,9 @@ class File < IO::FileDescriptor # file as an argument, the file will be automatically closed when the block returns. # # See `self.new` for what *mode* can be. - def self.open(filename : Path | String, mode = "r", perm = DEFAULT_CREATE_PERMISSIONS, encoding = nil, invalid = nil, blocking = true, &) + # + # NOTE: The *blocking* arg is deprecated since Crystal 1.17. + def self.open(filename : Path | String, mode = "r", perm = DEFAULT_CREATE_PERMISSIONS, encoding = nil, invalid = nil, blocking = nil, &) file = new filename, mode, perm, encoding, invalid, blocking begin yield file @@ -529,7 +526,9 @@ class File < IO::FileDescriptor # File.write("bar", "foo") # File.read("bar") # => "foo" # ``` - def self.read(filename : Path | String, encoding = nil, invalid = nil, blocking = true) : String + # + # NOTE: The *blocking* arg is deprecated since Crystal 1.17. + def self.read(filename : Path | String, encoding = nil, invalid = nil, blocking = nil) : String open(filename, "r", blocking: blocking) do |file| if encoding file.set_encoding(encoding, invalid: invalid) @@ -558,7 +557,9 @@ class File < IO::FileDescriptor # end # array # => ["foo", "bar"] # ``` - def self.each_line(filename : Path | String, encoding = nil, invalid = nil, chomp = true, blocking = true, &) + # + # NOTE: The *blocking* arg is deprecated since Crystal 1.17. + def self.each_line(filename : Path | String, encoding = nil, invalid = nil, chomp = true, blocking = nil, &) open(filename, "r", encoding: encoding, invalid: invalid, blocking: blocking) do |file| file.each_line(chomp: chomp) do |line| yield line @@ -572,7 +573,9 @@ class File < IO::FileDescriptor # File.write("foobar", "foo\nbar") # File.read_lines("foobar") # => ["foo", "bar"] # ``` - def self.read_lines(filename : Path | String, encoding = nil, invalid = nil, chomp = true, blocking = true) : Array(String) + # + # NOTE: The *blocking* arg is deprecated since Crystal 1.17. + def self.read_lines(filename : Path | String, encoding = nil, invalid = nil, chomp = true, blocking = nil) : Array(String) lines = [] of String each_line(filename, encoding: encoding, invalid: invalid, chomp: chomp, blocking: blocking) do |line| lines << line @@ -597,7 +600,9 @@ class File < IO::FileDescriptor # (the result of invoking `to_s` on *content*). # # See `self.new` for what *mode* can be. - def self.write(filename : Path | String, content, perm = DEFAULT_CREATE_PERMISSIONS, encoding = nil, invalid = nil, mode = "w", blocking = true) + # + # NOTE: The *blocking* arg is deprecated since Crystal 1.17. + def self.write(filename : Path | String, content, perm = DEFAULT_CREATE_PERMISSIONS, encoding = nil, invalid = nil, mode = "w", blocking = nil) open(filename, mode, perm, encoding: encoding, invalid: invalid, blocking: blocking) do |file| case content when Bytes diff --git a/src/io/file_descriptor.cr b/src/io/file_descriptor.cr index 7ca360eabdf0..1baf3d340b77 100644 --- a/src/io/file_descriptor.cr +++ b/src/io/file_descriptor.cr @@ -8,6 +8,12 @@ class IO::FileDescriptor < IO @volatile_fd : Atomic(Handle) # Returns the raw file-descriptor handle. Its type is platform-specific. + # + # The file-descriptor handle has been configured for the IO system + # requirements. If it must be in a specific mode or have a specific set of + # flags set, then they must be applied, even when when it feels redundant, + # because even the same target isn't guaranteed to have the same requirements + # at runtime. def fd : Handle @volatile_fd.get end @@ -39,17 +45,37 @@ class IO::FileDescriptor < IO write_timeout end - def self.new(fd : Handle, blocking = nil, *, close_on_finalize = true) + # Creates an IO::FileDescriptor from an existing system file descriptor or + # handle. + # + # This adopts *fd* into the IO system that will reconfigure it as per the + # event loop runtime requirements. + # + # NOTE: On Windows the handle should have been created with `FILE_FLAG_OVERLAPPED`. + def self.new(fd : Handle, *, close_on_finalize = true) + file_descriptor = new(handle: fd, close_on_finalize: close_on_finalize) + file_descriptor.system_blocking_init(nil) unless file_descriptor.closed? + file_descriptor + end + + @[Deprecated("The blocking argument is deprecated with no replacement.")] + def self.new(fd : Handle, blocking, *, close_on_finalize = true) file_descriptor = new(handle: fd, close_on_finalize: close_on_finalize) file_descriptor.system_blocking_init(blocking) unless file_descriptor.closed? file_descriptor end # :nodoc: - def initialize(*, handle : Handle, @close_on_finalize = true) + # + # Internal constructor to wrap a system *handle*. The *blocking* arg is purely + # informational. + def initialize(*, handle : Handle, @close_on_finalize = true, blocking = nil) @volatile_fd = Atomic.new(handle) @closed = true # This is necessary so we can reference `self` in `system_closed?` (in case of an exception) @closed = system_closed? + {% if flag?(:win32) %} + @system_blocking = !!blocking + {% end %} end # :nodoc: @@ -63,12 +89,21 @@ class IO::FileDescriptor < IO # 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. + @[Deprecated("There are no replacement.")] def blocking emulated = emulated_blocking? return emulated unless emulated.nil? system_blocking? end + # Changes the file descriptor's mode to blocking (true) or non blocking + # (false). + # + # WARNING: the file descriptor has been configured to behave correctly with + # the event loop runtime requirements. Changing the blocking mode can cause + # the event loop to misbehave, for example block the entire program when a + # fiber tries to read from this file descriptor. + @[Deprecated("There are no replacement.")] def blocking=(value) self.system_blocking = value end diff --git a/src/process.cr b/src/process.cr index 63b78bf0f716..661cb42fafff 100644 --- a/src/process.cr +++ b/src/process.cr @@ -296,7 +296,7 @@ class Process # regular files will report an error and those require a separate pipe # (https://github.com/crystal-lang/crystal/pull/13362#issuecomment-1519082712) {% if flag?(:win32) %} - unless stdio.blocking || stdio.info.type.pipe? + unless stdio.system_blocking? || stdio.info.type.pipe? return io_to_fd(stdio, for: dst_io) end {% end %} diff --git a/src/socket.cr b/src/socket.cr index a9833dc255d4..aeb712a27793 100644 --- a/src/socket.cr +++ b/src/socket.cr @@ -13,6 +13,12 @@ class Socket < IO # # * on POSIX platforms, this is a file descriptor (`Int32`) # * on Windows, this is a SOCKET handle (`LibC::SOCKET`) + # + # The returned system socket has been configured as per the IO system runtime + # requirements. If the returned socket must be in a specific mode or have a + # specific set of flags set, then they must be applied, even when it feels + # redundant, because even the same target isn't guaranteed to have the same + # requirements at runtime. def fd @volatile_fd.get end @@ -45,22 +51,40 @@ class Socket < IO # Creates a TCP socket. Consider using `TCPSocket` or `TCPServer` unless you # need full control over the socket. - def self.tcp(family : Family, blocking = nil) : self + def self.tcp(family : Family) : self + new(family, Type::STREAM, Protocol::TCP) + end + + @[Deprecated("The blocking argument is deprecated. Use Socket.set_blocking instead.")] + def self.tcp(family : Family, blocking) : self new(family, Type::STREAM, Protocol::TCP, blocking) end # Creates an UDP socket. Consider using `UDPSocket` unless you need full # control over the socket. - def self.udp(family : Family, blocking = nil) + def self.udp(family : Family) : self + new(family, Type::DGRAM, Protocol::UDP) + end + + @[Deprecated("The blocking argument is deprecated. Use Socket.set_blocking instead.")] + def self.udp(family : Family, blocking) : self new(family, Type::DGRAM, Protocol::UDP, blocking) end # Creates an UNIX socket. Consider using `UNIXSocket` or `UNIXServer` unless # you need full control over the socket. + # + # NOTE: The *blocking* argument is deprecated since Crystal 1.17. Use + # `Socket.set_blocking` to change it after creating the socket. def self.unix(type : Type = Type::STREAM, blocking = nil) : self new(Family::UNIX, type, blocking: blocking) end + # Creates a socket. Consider using `TCPSocket`, `TCPServer`, `UDPSocket`, + # `UNIXSocket` or `UNIXServer` unless you need full control over the socket. + # + # NOTE: The *blocking* argument is deprecated since Crystal 1.17. Use + # `Socket.set_blocking` to change it after creating the socket. def initialize(family : Family, type : Type, protocol : Protocol = Protocol::IP, blocking = nil) # This method is `#initialize` instead of `.new` because it is used as super # constructor from subclasses. @@ -69,11 +93,18 @@ class Socket < IO self.sync = true end - # Creates a Socket from an existing socket file descriptor / handle. + # Creates a Socket from an existing system file descriptor or socket handle. + # + # This adopts *fd* into the IO system that will reconfigure it as per the + # event loop runtime requirements. + # + # NOTE: On Windows the handle must have been created with `WSA_FLAG_OVERLAPPED`. + # NOTE: The *blocking* argument is deprecated since Crystal 1.17. Use + # `Socket.set_blocking` to change it after creating the socket. def initialize(fd, @family : Family, @type : Type, @protocol : Protocol = Protocol::IP, blocking = nil) initialize(handle: fd, family: family, type: type, protocol: protocol) blocking = Crystal::EventLoop.default_socket_blocking? if blocking.nil? - self.blocking = blocking unless blocking + Crystal::System::Socket.set_blocking(fd, blocking) unless blocking self.sync = true end @@ -214,10 +245,10 @@ class Socket < IO def accept? : Socket? if rs = Crystal::EventLoop.current.accept(self) sock = Socket.new(handle: rs[0], family: family, type: type, protocol: protocol, blocking: rs[1]) - unless (blocking = self.blocking) == rs[1] + unless (blocking = system_blocking?) == rs[1] # FIXME: unlike the overloads in TCPServer and UNIXServer, this version # carries the blocking mode from the server socket to the client socket - sock.blocking = blocking + Crystal::System::Socket.set_blocking(fd, blocking) end sock.sync = sync? sock @@ -406,14 +437,29 @@ class Socket < IO optval end + # Returns where the socket's mode is blocking (true) or non blocking (false). + @[Deprecated("There are no replacement.")] def blocking system_blocking? end + # Changes the socket's mode to blocking (true) or non blocking (false). + # + # WARNING: the socket has been configured to behave correctly with the event + # loop runtime requirements. Changing the blocking mode can cause the event + # loop to misbehave, for example block the entire program when a fiber tries + # to read from this socket. + @[Deprecated("Use Socket.set_blocking(fd, value) instead.")] def blocking=(value) self.system_blocking = value end + # Changes the blocking mode of *fd* to be blocking (true) or non blocking + # (false). + def self.set_blocking(fd : Handle, value : Bool) + Crystal::System::Socket.set_blocking(fd, value) + end + def close_on_exec? system_close_on_exec? end diff --git a/src/socket/tcp_server.cr b/src/socket/tcp_server.cr index 287b5d394250..638e18c66472 100644 --- a/src/socket/tcp_server.cr +++ b/src/socket/tcp_server.cr @@ -52,7 +52,13 @@ class TCPServer < TCPSocket end end - # Creates a TCPServer from an already configured raw file descriptor + # Creates a TCPServer from an existing system file descriptor or socket + # handle. + # + # This adopts *fd* into the IO system that will reconfigure it as per the + # event loop runtime requirements. + # + # NOTE: On Windows the handle must have been created with `WSA_FLAG_OVERLAPPED`. def initialize(*, fd : Handle, family : Family = Family::INET) super(fd: fd, family: family) end diff --git a/src/socket/tcp_socket.cr b/src/socket/tcp_socket.cr index 95c6521237cc..3c1e4dc88e4c 100644 --- a/src/socket/tcp_socket.cr +++ b/src/socket/tcp_socket.cr @@ -15,6 +15,9 @@ require "./ip_socket" # ``` class TCPSocket < IPSocket # Creates a new `TCPSocket`, waiting to be connected. + # + # NOTE: The *blocking* argument is deprecated since Crystal 1.17. Use + # `Socket.set_blocking` to change it after creating the socket. def self.new(family : Family = Family::INET, blocking = nil) super(family, Type::STREAM, Protocol::TCP, blocking) end @@ -24,6 +27,9 @@ class TCPSocket < IPSocket # You may limit the DNS resolution time with `dns_timeout` and limit the # connection time to the remote server with `connect_timeout`. Both values # must be in seconds (integers or floats). + # + # NOTE: The *blocking* argument is deprecated since Crystal 1.17. Use + # `Socket.set_blocking` to change it after creating the socket. def initialize(host : String, port, dns_timeout = nil, connect_timeout = nil, blocking = nil) Addrinfo.tcp(host, port, timeout: dns_timeout) do |addrinfo| super(addrinfo.family, addrinfo.type, addrinfo.protocol, blocking) @@ -38,12 +44,21 @@ class TCPSocket < IPSocket super family, type, protocol end - # constructor for TCPServer#accept? + # Internal constructor for TCPServer#accept? + # The *blocking* arg is purely informational. protected def initialize(*, handle, family, type, protocol, blocking) super(handle: handle, family: family, type: type, protocol: protocol, blocking: blocking) end - # Creates a TCPSocket from an already configured raw file descriptor + # Creates an UNIXSocket from an existing system file descriptor or socket + # handle. + # + # This adopts *fd* into the IO system that will reconfigure it as per the + # event loop runtime requirements. + # + # NOTE: On Windows the handle must have been created with `WSA_FLAG_OVERLAPPED`. + # NOTE: The *blocking* argument is deprecated since Crystal 1.17. Use + # `Socket.set_blocking` to change it after creating the socket. def initialize(*, fd : Handle, family : Family = Family::INET, blocking = nil) super fd, family, Type::STREAM, Protocol::TCP, blocking end diff --git a/src/socket/unix_server.cr b/src/socket/unix_server.cr index 19d09a43aed0..05e48b171c05 100644 --- a/src/socket/unix_server.cr +++ b/src/socket/unix_server.cr @@ -53,7 +53,13 @@ class UNIXServer < UNIXSocket end end - # Creates a UNIXServer from an already configured raw file descriptor + # Creates an UNIXServer from an existing system file descriptor or socket + # handle. + # + # This adopts *fd* into the IO system that will reconfigure it as per the + # event loop runtime requirements. + # + # NOTE: On Windows the handle must have been created with `WSA_FLAG_OVERLAPPED`. def initialize(*, fd : Handle, type : Type = Type::STREAM, path : Path | String? = nil) @path = path = path.to_s super(fd: fd, type: type, path: path) diff --git a/src/socket/unix_socket.cr b/src/socket/unix_socket.cr index fd864af4320d..f761669a3b03 100644 --- a/src/socket/unix_socket.cr +++ b/src/socket/unix_socket.cr @@ -32,13 +32,20 @@ class UNIXSocket < Socket super family, type, Protocol::IP end - # Constructor for #pair and UNIXServer#accept? + # Internal constructor for UNIXSocket#pair and UNIXServer#accept? + # The *blocking* arg is purely informational. protected def initialize(*, handle : Handle, type : Type = Type::STREAM, path : Path | String? = nil, blocking : Bool = nil) @path = path.to_s super handle: handle, family: Family::UNIX, type: type, protocol: Protocol::IP, blocking: blocking end - # Creates a UNIXSocket from an already configured raw file descriptor + # Creates an UNIXSocket from an existing system file descriptor or socket + # handle. + # + # This adopts the system socket into the IO system that will reconfigure it + # as per the event loop runtime requirements. + # + # NOTE: On Windows the handle must have been created with `WSA_FLAG_OVERLAPPED`. def initialize(*, fd : Handle, type : Type = Type::STREAM, path : Path | String? = nil) @path = path.to_s super fd, Family::UNIX, type, Protocol::IP