diff --git a/spec/std/file/tempfile_spec.cr b/spec/std/file/tempfile_spec.cr index 3ede9e52e44d..84d9cd553398 100644 --- a/spec/std/file/tempfile_spec.cr +++ b/spec/std/file/tempfile_spec.cr @@ -200,7 +200,7 @@ describe Crystal::System::File do fd, path = Crystal::System::File.mktemp("A", "Z", dir: tempdir, random: TestRNG.new([7, 8, 9, 10, 11, 12, 13, 14])) path.should eq Path[tempdir, "A789abcdeZ"].to_s ensure - File.from_fd(path, fd).close if fd && path + IO::FileDescriptor.new(fd).close if fd end end @@ -212,7 +212,7 @@ describe Crystal::System::File do fd, path = Crystal::System::File.mktemp("A", "Z", dir: tempdir, random: TestRNG.new([7, 8, 9, 10, 11, 12, 13, 14, 15, 16, 17, 18, 19, 20, 21, 22])) path.should eq File.join(tempdir, "AfghijklmZ") ensure - File.from_fd(path, fd).close if fd && path + IO::FileDescriptor.new(fd).close if fd end end @@ -223,7 +223,7 @@ describe Crystal::System::File do expect_raises(File::AlreadyExistsError, "Error creating temporary file") do fd, path = Crystal::System::File.mktemp("A", "Z", dir: tempdir, random: TestRNG.new([7, 8, 9, 10, 11, 12, 13, 14])) ensure - File.from_fd(path, fd).close if fd && path + IO::FileDescriptor.new(fd).close if fd end end end diff --git a/src/crystal/system/file.cr b/src/crystal/system/file.cr index 75985c107fd5..2283a7329177 100644 --- a/src/crystal/system/file.cr +++ b/src/crystal/system/file.cr @@ -65,7 +65,7 @@ module Crystal::System::File io << suffix end - handle, errno = open(path, mode, perm) + handle, errno = open(path, mode, perm, blocking: true) if error_is_none?(errno) return {handle, path} diff --git a/src/crystal/system/unix/file.cr b/src/crystal/system/unix/file.cr index a353cf29cd3c..722f5eda1d1d 100644 --- a/src/crystal/system/unix/file.cr +++ b/src/crystal/system/unix/file.cr @@ -3,10 +3,10 @@ require "file/error" # :nodoc: module Crystal::System::File - def self.open(filename : String, mode : String, perm : Int32 | ::File::Permissions) + def self.open(filename : String, mode : String, perm : Int32 | ::File::Permissions, blocking) perm = ::File::Permissions.new(perm) if perm.is_a? Int32 - fd, errno = open(filename, open_flag(mode), perm) + fd, errno = open(filename, open_flag(mode), perm, blocking) unless errno.none? raise ::File::Error.from_os_error("Error opening file with mode '#{mode}'", errno, file: filename) @@ -15,7 +15,7 @@ module Crystal::System::File fd end - def self.open(filename : String, flags : Int32, perm : ::File::Permissions) : {LibC::Int, Errno} + def self.open(filename : String, flags : Int32, perm : ::File::Permissions, blocking _blocking) : {LibC::Int, Errno} filename.check_no_null_byte flags |= LibC::O_CLOEXEC diff --git a/src/crystal/system/unix/file_descriptor.cr b/src/crystal/system/unix/file_descriptor.cr index c0404e7652a2..d6544a49c39f 100644 --- a/src/crystal/system/unix/file_descriptor.cr +++ b/src/crystal/system/unix/file_descriptor.cr @@ -227,8 +227,8 @@ module Crystal::System::FileDescriptor {r, w} end - def self.pread(fd, buffer, offset) - bytes_read = LibC.pread(fd, buffer, buffer.size, offset).to_i64 + def self.pread(file, buffer, offset) + bytes_read = LibC.pread(file.fd, buffer, buffer.size, offset).to_i64 if bytes_read == -1 raise IO::Error.from_errno "Error reading file" diff --git a/src/crystal/system/win32/file.cr b/src/crystal/system/win32/file.cr index 459cb86d977d..ed970055d35b 100644 --- a/src/crystal/system/win32/file.cr +++ b/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) : FileDescriptor::Handle + def self.open(filename : String, mode : String, perm : Int32 | ::File::Permissions, blocking : Bool?) : 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,7 +19,7 @@ module Crystal::System::File perm = LibC::S_IREAD end - handle, error = open(filename, open_flag(mode), ::File::Permissions.new(perm)) + handle, error = open(filename, open_flag(mode), ::File::Permissions.new(perm), blocking != false) unless error.error_success? raise ::File::Error.from_os_error("Error opening file with mode '#{mode}'", error, file: filename) end @@ -27,8 +27,8 @@ module Crystal::System::File handle end - def self.open(filename : String, flags : Int32, perm : ::File::Permissions) : {FileDescriptor::Handle, WinError} - access, disposition, attributes = self.posix_to_open_opts flags, perm + def self.open(filename : String, flags : Int32, perm : ::File::Permissions, blocking : Bool) : {FileDescriptor::Handle, WinError} + access, disposition, attributes = self.posix_to_open_opts flags, perm, blocking handle = LibC.CreateFileW( System.to_wstr(filename), @@ -43,7 +43,7 @@ module Crystal::System::File {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) + private def self.posix_to_open_opts(flags : Int32, perm : ::File::Permissions, blocking : Bool) access = if flags.bits_set? LibC::O_WRONLY LibC::FILE_GENERIC_WRITE elsif flags.bits_set? LibC::O_RDWR @@ -73,7 +73,7 @@ module Crystal::System::File disposition = LibC::OPEN_EXISTING end - attributes = LibC::FILE_ATTRIBUTE_NORMAL + attributes = 0 unless perm.owner_write? attributes |= LibC::FILE_ATTRIBUTE_READONLY end @@ -93,6 +93,10 @@ module Crystal::System::File attributes |= LibC::FILE_FLAG_RANDOM_ACCESS end + unless blocking + attributes |= LibC::FILE_FLAG_OVERLAPPED + end + {access, disposition, attributes} end diff --git a/src/crystal/system/win32/file_descriptor.cr b/src/crystal/system/win32/file_descriptor.cr index 161032cd307b..20dc1cd057d5 100644 --- a/src/crystal/system/win32/file_descriptor.cr +++ b/src/crystal/system/win32/file_descriptor.cr @@ -36,7 +36,8 @@ module Crystal::System::FileDescriptor end bytes_read.to_i32 else - overlapped_operation(handle, "ReadFile", read_timeout) do |overlapped| + seekable = LibC.SetFilePointerEx(handle, 0, out offset, IO::Seek::Current) != 0 + overlapped_operation(handle, seekable ? offset : nil, "ReadFile", read_timeout, writing: false) do |overlapped| ret = LibC.ReadFile(handle, slice, slice.size, out byte_count, overlapped) {ret, byte_count} end.to_i32 @@ -58,7 +59,8 @@ module Crystal::System::FileDescriptor end end else - bytes_written = overlapped_operation(handle, "WriteFile", write_timeout, writing: true) do |overlapped| + seekable = LibC.SetFilePointerEx(handle, 0, out offset, IO::Seek::Current) != 0 + bytes_written = overlapped_operation(handle, seekable ? offset : nil, "WriteFile", write_timeout, writing: true) do |overlapped| ret = LibC.WriteFile(handle, slice, slice.size, out byte_count, overlapped) {ret, byte_count} end @@ -80,6 +82,7 @@ module Crystal::System::FileDescriptor private def system_blocking_init(value) @system_blocking = value + Crystal::Scheduler.event_loop.create_completion_port(windows_handle) unless value end private def system_close_on_exec? @@ -184,41 +187,71 @@ module Crystal::System::FileDescriptor end private def flock(exclusive, retry) - flags = LibC::LOCKFILE_FAIL_IMMEDIATELY + flags = 0_u32 + flags |= LibC::LOCKFILE_FAIL_IMMEDIATELY unless retry && !system_blocking? flags |= LibC::LOCKFILE_EXCLUSIVE_LOCK if exclusive handle = windows_handle - if retry + if retry && system_blocking? until lock_file(handle, flags) sleep 0.1 end else - lock_file(handle, flags) || raise IO::Error.from_winerror("Error applying file lock: file is already locked") + lock_file(handle, flags) || raise IO::Error.from_winerror("Error applying file lock: file is already locked", target: self) end end private def lock_file(handle, flags) - # lpOverlapped must be provided despite the synchronous use of this method. - overlapped = LibC::OVERLAPPED.new - # lock the entire file with offset 0 in overlapped and number of bytes set to max value - if 0 != LibC.LockFileEx(handle, flags, 0, 0xFFFF_FFFF, 0xFFFF_FFFF, pointerof(overlapped)) - true - else - winerror = WinError.value - if winerror == WinError::ERROR_LOCK_VIOLATION - false + IO::Overlapped::OverlappedOperation.run(handle) do |operation| + overlapped = operation.start + result = LibC.LockFileEx(handle, flags, 0, 0xFFFF_FFFF, 0xFFFF_FFFF, overlapped) + + if result == 0 + case error = WinError.value + when .error_io_pending? + # the operation is running asynchronously; do nothing + when .error_lock_violation? + # synchronous failure + operation.synchronous = true + return false + else + raise IO::Error.from_os_error("LockFileEx", error, target: self) + end else - raise IO::Error.from_os_error("LockFileEx", winerror, target: self) + operation.synchronous = true + return true end + + schedule_overlapped(nil) + + operation.result(handle) do |error| + raise IO::Error.from_os_error("LockFileEx", error, target: self) + end + + true end end private def unlock_file(handle) - # lpOverlapped must be provided despite the synchronous use of this method. - overlapped = LibC::OVERLAPPED.new - # unlock the entire file with offset 0 in overlapped and number of bytes set to max value - if 0 == LibC.UnlockFileEx(handle, 0, 0xFFFF_FFFF, 0xFFFF_FFFF, pointerof(overlapped)) - raise IO::Error.from_winerror("UnLockFileEx") + IO::Overlapped::OverlappedOperation.run(handle) do |operation| + overlapped = operation.start + result = LibC.UnlockFileEx(handle, 0, 0xFFFF_FFFF, 0xFFFF_FFFF, overlapped) + + if result == 0 + error = WinError.value + unless error.error_io_pending? + raise IO::Error.from_os_error("UnlockFileEx", error, target: self) + end + else + operation.synchronous = true + return + end + + schedule_overlapped(nil) + + operation.result(handle) do |error| + raise IO::Error.from_os_error("UnlockFileEx", error, target: self) + end end end @@ -238,13 +271,11 @@ module Crystal::System::FileDescriptor w_pipe_flags |= LibC::FILE_FLAG_OVERLAPPED unless write_blocking w_pipe = LibC.CreateNamedPipeA(pipe_name, w_pipe_flags, pipe_mode, 1, PIPE_BUFFER_SIZE, PIPE_BUFFER_SIZE, 0, nil) raise IO::Error.from_winerror("CreateNamedPipeA") if w_pipe == LibC::INVALID_HANDLE_VALUE - Crystal::Scheduler.event_loop.create_completion_port(w_pipe) unless write_blocking r_pipe_flags = LibC::FILE_FLAG_NO_BUFFERING r_pipe_flags |= LibC::FILE_FLAG_OVERLAPPED unless read_blocking r_pipe = LibC.CreateFileW(System.to_wstr(pipe_name), LibC::GENERIC_READ | LibC::FILE_WRITE_ATTRIBUTES, 0, nil, LibC::OPEN_EXISTING, r_pipe_flags, nil) 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(r_pipe.address, read_blocking) w = IO::FileDescriptor.new(w_pipe.address, write_blocking) @@ -253,19 +284,12 @@ module Crystal::System::FileDescriptor {r, w} end - def self.pread(fd, buffer, offset) - handle = windows_handle(fd) - - overlapped = LibC::OVERLAPPED.new - overlapped.union.offset.offset = LibC::DWORD.new(offset) - overlapped.union.offset.offsetHigh = LibC::DWORD.new(offset >> 32) - if LibC.ReadFile(handle, buffer, buffer.size, out bytes_read, pointerof(overlapped)) == 0 - error = WinError.value - return 0_i64 if error == WinError::ERROR_HANDLE_EOF - raise IO::Error.from_os_error "Error reading file", error, target: self - end - - bytes_read.to_i64 + def self.pread(file, buffer, offset) + handle = windows_handle(file.fd) + file.overlapped_operation(handle, offset, "ReadFile", file.read_timeout, writing: false) do |overlapped| + ret = LibC.ReadFile(handle, buffer, buffer.size, out byte_count, overlapped) + {ret, byte_count} + end.to_i64 end def self.from_stdio(fd) diff --git a/src/file.cr b/src/file.cr index a011979dcc9b..4390bc8a38ea 100644 --- a/src/file.cr +++ b/src/file.cr @@ -172,7 +172,7 @@ class File < IO::FileDescriptor # additional syscall. def self.new(filename : Path | String, mode = "r", perm = DEFAULT_CREATE_PERMISSIONS, encoding = nil, invalid = nil, blocking = true) filename = filename.to_s - fd = Crystal::System::File.open(filename, mode, perm: perm) + fd = Crystal::System::File.open(filename, mode, perm: perm, blocking: blocking) new(filename, fd, blocking: blocking, encoding: encoding, invalid: invalid) end diff --git a/src/file/preader.cr b/src/file/preader.cr index d366457314ce..9f7d09643305 100644 --- a/src/file/preader.cr +++ b/src/file/preader.cr @@ -20,7 +20,7 @@ class File::PReader < IO count = slice.size count = Math.min(count, @bytesize - @pos) - bytes_read = Crystal::System::FileDescriptor.pread(@file.fd, slice[0, count], @offset + @pos) + bytes_read = Crystal::System::FileDescriptor.pread(@file, slice[0, count], @offset + @pos) @pos += bytes_read diff --git a/src/io/overlapped.cr b/src/io/overlapped.cr index 0ab1a24fc794..c870f45539ae 100644 --- a/src/io/overlapped.cr +++ b/src/io/overlapped.cr @@ -167,9 +167,14 @@ module IO::Overlapped Crystal::Scheduler.event_loop.dequeue(timeout_event) end - def overlapped_operation(handle, method, timeout, *, writing = false, &) + protected def overlapped_operation(handle, offset, method, timeout, *, writing = false, &) : UInt32 OverlappedOperation.run(handle) do |operation| - result, value = yield operation.start + overlapped = operation.start + if offset + overlapped.value.union.offset.offset = LibC::DWORD.new(offset) + overlapped.value.union.offset.offsetHigh = LibC::DWORD.new(offset >> 32) + end + result, value = yield overlapped if result == 0 case error = WinError.value @@ -186,12 +191,13 @@ module IO::Overlapped end else operation.synchronous = true + LibC.SetFilePointerEx(handle, value, nil, IO::Seek::Current) if offset return value end schedule_overlapped(timeout) - operation.result(handle) do |error| + byte_count = operation.result(handle) do |error| case error when .error_io_incomplete? raise IO::TimeoutError.new("#{method} timed out") @@ -202,6 +208,11 @@ module IO::Overlapped return 0_u32 end end + + # the file pointer might have been changed while the operation was in + # progress, so we can't use `IO::Seek::Current` here + LibC.SetFilePointerEx(handle, offset + byte_count, nil, IO::Seek::Set) if offset + byte_count end end diff --git a/src/process.cr b/src/process.cr index a1b827d73754..b1bdaba2fb80 100644 --- a/src/process.cr +++ b/src/process.cr @@ -291,9 +291,16 @@ class Process private def stdio_to_fd(stdio : Stdio, for dst_io : IO::FileDescriptor) : IO::FileDescriptor case stdio - when IO::FileDescriptor - stdio - when IO + in IO + if stdio.is_a?(IO::FileDescriptor) + # on Windows, only async pipes can be passed to child processes, async + # files will report an error and those require a separate pipe + # (https://github.com/crystal-lang/crystal/pull/13362#issuecomment-1519082712) + if {{ !flag?(:win32) }} || stdio.blocking || stdio.info.type.pipe? + return stdio + end + end + if dst_io == STDIN fork_io, process_io = IO.pipe(read_blocking: true) @@ -309,7 +316,7 @@ class Process end fork_io - when Redirect::Pipe + in Redirect::Pipe case dst_io when STDIN fork_io, @input = IO.pipe(read_blocking: true) @@ -322,16 +329,14 @@ class Process end fork_io - when Redirect::Inherit + in Redirect::Inherit dst_io - when Redirect::Close + in Redirect::Close if dst_io == STDIN File.open(File::NULL, "r") else File.open(File::NULL, "w") end - else - raise "BUG: Impossible type in stdio #{stdio.class}" end end