Use file handles directly instead of C file descriptors on Win32 (#14501

)

spec/compiler/macro/macro_methods_spec.cr

@@ -3732,7 +3732,7 @@ module Crystal
         assert_error <<-CRYSTAL,
           {{read_file("#{__DIR__}/../data/build_foo")}}
           CRYSTAL
-          "No such file or directory"
+          "Error opening file with mode 'r'"
       end
     end
 
@@ -3747,7 +3747,7 @@ module Crystal
         assert_error <<-CRYSTAL,
           {{read_file("spec/compiler/data/build_foo")}}
           CRYSTAL
-          "No such file or directory"
+          "Error opening file with mode 'r'"
       end
     end
   end

src/crystal/system/file.cr

@@ -49,7 +49,7 @@ module Crystal::System::File
 
   LOWER_ALPHANUM = "0123456789abcdefghijklmnopqrstuvwxyz".to_slice
 
-  def self.mktemp(prefix : String?, suffix : String?, dir : String, random : ::Random = ::Random::DEFAULT) : {LibC::Int, String}
+  def self.mktemp(prefix : String?, suffix : String?, dir : String, random : ::Random = ::Random::DEFAULT) : {FileDescriptor::Handle, String}
     mode = LibC::O_RDWR | LibC::O_CREAT | LibC::O_EXCL
     perm = ::File::Permissions.new(0o600)
 
@@ -65,10 +65,10 @@ module Crystal::System::File
         io << suffix
       end
 
-      fd, errno = open(path, mode, perm)
+      handle, errno = open(path, mode, perm)
 
-      if errno.none?
-        return {fd, path}
+      if error_is_none?(errno)
+        return {handle, path}
       elsif error_is_file_exists?(errno)
         # retry
         next
@@ -80,8 +80,12 @@ module Crystal::System::File
     raise ::File::AlreadyExistsError.new("Error creating temporary file", file: "#{prefix}********#{suffix}")
   end
 
+  private def self.error_is_none?(errno)
+    errno.in?(Errno::NONE, WinError::ERROR_SUCCESS)
+  end
+
   private def self.error_is_file_exists?(errno)
-    errno.in?(Errno::EEXIST, WinError::ERROR_ALREADY_EXISTS)
+    errno.in?(Errno::EEXIST, WinError::ERROR_FILE_EXISTS)
   end
 
   # Closes the internal file descriptor without notifying libevent.

src/crystal/system/process.cr

@@ -84,9 +84,9 @@ struct Crystal::System::Process
 end
 
 module Crystal::System
-  ORIGINAL_STDIN  = IO::FileDescriptor.new(0, blocking: true)
-  ORIGINAL_STDOUT = IO::FileDescriptor.new(1, blocking: true)
-  ORIGINAL_STDERR = IO::FileDescriptor.new(2, blocking: true)
+  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)
 end
 
 {% if flag?(:wasi) %}

src/crystal/system/unix/file_descriptor.cr

@@ -13,6 +13,10 @@ module Crystal::System::FileDescriptor
   # system.
   alias Handle = Int32
 
+  STDIN_HANDLE  = 0
+  STDOUT_HANDLE = 1
+  STDERR_HANDLE = 2
+
   private def unbuffered_read(slice : Bytes) : Int32
     evented_read(slice, "Error reading file") do
       LibC.read(fd, slice, slice.size).tap do |return_code|

src/crystal/system/win32/file.cr

@@ -9,7 +9,7 @@ require "c/ntifs"
 require "c/winioctl"
 
 module Crystal::System::File
-  def self.open(filename : String, mode : String, perm : Int32 | ::File::Permissions) : LibC::Int
+  def self.open(filename : String, mode : String, perm : Int32 | ::File::Permissions) : FileDescriptor::Handle
     perm = ::File::Permissions.new(perm) if perm.is_a? Int32
     # Only the owner writable bit is used, since windows only supports
     # the read only attribute.
@@ -19,15 +19,15 @@ module Crystal::System::File
       perm = LibC::S_IREAD
     end
 
-    fd, errno = open(filename, open_flag(mode), ::File::Permissions.new(perm))
-    unless errno.none?
-      raise ::File::Error.from_os_error("Error opening file with mode '#{mode}'", errno, file: filename)
+    handle, error = open(filename, open_flag(mode), ::File::Permissions.new(perm))
+    unless error.error_success?
+      raise ::File::Error.from_os_error("Error opening file with mode '#{mode}'", error, file: filename)
     end
 
-    fd
+    handle
   end
 
-  def self.open(filename : String, flags : Int32, perm : ::File::Permissions) : {LibC::Int, Errno}
+  def self.open(filename : String, flags : Int32, perm : ::File::Permissions) : {FileDescriptor::Handle, WinError}
     access, disposition, attributes = self.posix_to_open_opts flags, perm
 
     handle = LibC.CreateFileW(
@@ -40,17 +40,7 @@ module Crystal::System::File
       LibC::HANDLE.null
     )
 
-    if handle == LibC::INVALID_HANDLE_VALUE
-      return {-1, WinError.value.to_errno}
-    end
-
-    fd = LibC._open_osfhandle handle, flags
-
-    if fd == -1
-      return {-1, Errno.value}
-    end
-
-    {fd, Errno::NONE}
+    {handle.address, handle == LibC::INVALID_HANDLE_VALUE ? WinError.value : WinError::ERROR_SUCCESS}
   end
 
   private def self.posix_to_open_opts(flags : Int32, perm : ::File::Permissions)

src/crystal/system/win32/file_descriptor.cr

@@ -9,7 +9,13 @@ module Crystal::System::FileDescriptor
 
   # Platform-specific type to represent a file descriptor handle to the operating
   # system.
-  alias Handle = ::LibC::Int
+  # NOTE: this should really be `LibC::HANDLE`, here it is an integer type of
+  # the same size so that `IO::FileDescriptor#fd` continues to return an `Int`
+  alias Handle = ::LibC::UIntPtrT
+
+  STDIN_HANDLE  = LibC.GetStdHandle(LibC::STD_INPUT_HANDLE).address
+  STDOUT_HANDLE = LibC.GetStdHandle(LibC::STD_OUTPUT_HANDLE).address
+  STDERR_HANDLE = LibC.GetStdHandle(LibC::STD_ERROR_HANDLE).address
 
   @system_blocking = true
 
@@ -92,21 +98,12 @@ module Crystal::System::FileDescriptor
     raise NotImplementedError.new "Crystal::System::FileDescriptor.fcntl"
   end
 
-  private def windows_handle
-    FileDescriptor.windows_handle!(fd)
+  protected def windows_handle
+    FileDescriptor.windows_handle(fd)
   end
 
   def self.windows_handle(fd)
-    ret = LibC._get_osfhandle(fd)
-    return LibC::INVALID_HANDLE_VALUE if ret == -1 || ret == -2
-    LibC::HANDLE.new(ret)
-  end
-
-  def self.windows_handle!(fd)
-    ret = LibC._get_osfhandle(fd)
-    raise RuntimeError.from_errno("_get_osfhandle") if ret == -1
-    raise RuntimeError.new("_get_osfhandle returned -2") if ret == -2
-    LibC::HANDLE.new(ret)
+    LibC::HANDLE.new(fd)
   end
 
   def self.system_info(handle, file_type = nil)
@@ -152,10 +149,11 @@ module Crystal::System::FileDescriptor
   end
 
   private def system_reopen(other : IO::FileDescriptor)
-    # Windows doesn't implement the CLOEXEC flag
-    if LibC._dup2(other.fd, self.fd) == -1
-      raise IO::Error.from_errno("Could not reopen file descriptor")
+    cur_proc = LibC.GetCurrentProcess
+    if LibC.DuplicateHandle(cur_proc, other.windows_handle, cur_proc, out new_handle, 0, true, LibC::DUPLICATE_SAME_ACCESS) == 0
+      raise IO::Error.from_winerror("Could not reopen file descriptor")
     end
+    @volatile_fd.set(new_handle.address)
 
     # Mark the handle open, since we had to have dup'd a live handle.
     @closed = false
@@ -168,13 +166,8 @@ module Crystal::System::FileDescriptor
   end
 
   def file_descriptor_close
-    if LibC._close(fd) != 0
-      case Errno.value
-      when Errno::EINTR
-        # ignore
-      else
-        raise IO::Error.from_errno("Error closing file", target: self)
-      end
+    if LibC.CloseHandle(windows_handle) == 0
+      raise IO::Error.from_winerror("Error closing file", target: self)
     end
   end
 
@@ -253,15 +246,15 @@ module Crystal::System::FileDescriptor
     raise IO::Error.from_winerror("CreateFileW") if r_pipe == LibC::INVALID_HANDLE_VALUE
     Crystal::Scheduler.event_loop.create_completion_port(r_pipe) unless read_blocking
 
-    r = IO::FileDescriptor.new(LibC._open_osfhandle(r_pipe, 0), read_blocking)
-    w = IO::FileDescriptor.new(LibC._open_osfhandle(w_pipe, 0), write_blocking)
+    r = IO::FileDescriptor.new(r_pipe.address, read_blocking)
+    w = IO::FileDescriptor.new(w_pipe.address, write_blocking)
     w.sync = true
 
     {r, w}
   end
 
   def self.pread(fd, buffer, offset)
-    handle = windows_handle!(fd)
+    handle = windows_handle(fd)
 
     overlapped = LibC::OVERLAPPED.new
     overlapped.union.offset.offset = LibC::DWORD.new(offset)
@@ -276,8 +269,14 @@ module Crystal::System::FileDescriptor
   end
 
   def self.from_stdio(fd)
+    handle = case fd
+             when 0 then LibC.GetStdHandle(LibC::STD_INPUT_HANDLE)
+             when 1 then LibC.GetStdHandle(LibC::STD_OUTPUT_HANDLE)
+             when 2 then LibC.GetStdHandle(LibC::STD_ERROR_HANDLE)
+             else        LibC::INVALID_HANDLE_VALUE
+             end
+
     console_handle = false
-    handle = windows_handle(fd)
     if handle != LibC::INVALID_HANDLE_VALUE
       # TODO: use `out old_mode` after implementing interpreter out closured var
       old_mode = uninitialized LibC::DWORD
@@ -291,7 +290,7 @@ module Crystal::System::FileDescriptor
       end
     end
 
-    io = IO::FileDescriptor.new(fd, blocking: true)
+    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

src/crystal/system/win32/process.cr

@@ -256,7 +256,7 @@ struct Crystal::System::Process
   end
 
   private def self.handle_from_io(io : IO::FileDescriptor, parent_io)
-    source_handle = FileDescriptor.windows_handle!(io.fd)
+    source_handle = io.windows_handle
 
     cur_proc = LibC.GetCurrentProcess
     if LibC.DuplicateHandle(cur_proc, source_handle, cur_proc, out new_handle, 0, true, LibC::DUPLICATE_SAME_ACCESS) == 0

src/io/file_descriptor.cr

@@ -39,7 +39,7 @@ class IO::FileDescriptor < IO
     write_timeout
   end
 
-  def initialize(fd, blocking = nil, *, @close_on_finalize = true)
+  def initialize(fd : Handle, blocking = nil, *, @close_on_finalize = true)
     @volatile_fd = Atomic.new(fd)
     @closed = system_closed?
 
@@ -57,7 +57,7 @@ class IO::FileDescriptor < IO
   end
 
   # :nodoc:
-  def self.from_stdio(fd) : self
+  def self.from_stdio(fd : Handle) : self
     Crystal::System::FileDescriptor.from_stdio(fd)
   end
 

src/lib_c/x86_64-windows-msvc/c/io.cr

@@ -1,13 +1,13 @@
 require "c/stdint"
 
 lib LibC
-  fun _close(fd : Int) : Int
   fun _wexecvp(cmdname : WCHAR*, argv : WCHAR**) : IntPtrT
-  fun _get_osfhandle(fd : Int) : IntPtrT
-  fun _dup2(fd1 : Int, fd2 : Int) : Int
-  fun _open_osfhandle(osfhandle : HANDLE, flags : LibC::Int) : LibC::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

src/lib_c/x86_64-windows-msvc/c/winbase.cr

@@ -12,7 +12,9 @@ lib LibC
   FORMAT_MESSAGE_ARGUMENT_ARRAY  = 0x00002000_u32
   FORMAT_MESSAGE_MAX_WIDTH_MASK  = 0x000000FF_u32
 
-  STD_ERROR_HANDLE = 0xFFFFFFF4_u32
+  STD_INPUT_HANDLE  = 0xFFFFFFF6_u32
+  STD_OUTPUT_HANDLE = 0xFFFFFFF5_u32
+  STD_ERROR_HANDLE  = 0xFFFFFFF4_u32
 
   fun FormatMessageA(dwFlags : DWORD, lpSource : Void*, dwMessageId : DWORD, dwLanguageId : DWORD,
                      lpBuffer : LPSTR, nSize : DWORD, arguments : Void*) : DWORD