Docs: Socket and IO::FileDescriptor fd ownership & soft deprecate their blocking args

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
 

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),

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

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

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) %}

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?)

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

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?

src/crystal/system/win32/file_descriptor.cr

@@ -99,7 +99,7 @@
   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
@@ -366,7 +366,7 @@
     # `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