Skip to content

Fix async append to file in IOCP #15681

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
27 changes: 27 additions & 0 deletions spec/std/file_spec.cr
Original file line number Diff line number Diff line change
Expand Up @@ -127,6 +127,33 @@ describe "File" do
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
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)

file.write "12345".to_slice
file.tell.should eq(10)

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
end

it "reads entire file" do
Expand Down
16 changes: 15 additions & 1 deletion src/crystal/event_loop/iocp.cr
Original file line number Diff line number Diff line change
Expand Up @@ -228,10 +228,24 @@ class Crystal::EventLoop::IOCP < Crystal::EventLoop
end

def write(file_descriptor : Crystal::System::FileDescriptor, slice : Bytes) : Int32
System::IOCP.overlapped_operation(file_descriptor, "WriteFile", file_descriptor.write_timeout, writing: true) do |overlapped|
bytes_written = System::IOCP.overlapped_operation(file_descriptor, "WriteFile", file_descriptor.write_timeout, writing: true) do |overlapped|
overlapped.offset = UInt64::MAX if file_descriptor.system_append?

ret = LibC.WriteFile(file_descriptor.windows_handle, slice, slice.size, out byte_count, overlapped)
{ret, byte_count}
end.to_i32

# The overlapped offset forced a write to the end of the file, but unlike
# synchronous writes, an asynchronous write incorrectly updates the file
# pointer: it merely adds the number of written bytes to the current
# position, disregarding that the offset might have changed it.
#
# We could seek before the async write (it works), but a concurrent fiber or
# parallel thread could also seek and we'd end up overwriting instead of
# appending; we need both the offset + explicit seek.
file_descriptor.system_seek(0, IO::Seek::End) if file_descriptor.system_append?

bytes_written
end

def wait_writable(file_descriptor : Crystal::System::FileDescriptor) : Nil
Expand Down
2 changes: 1 addition & 1 deletion src/crystal/system/win32/file.cr
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ module Crystal::System::File
# On Windows we cannot rely on the system mode `FILE_APPEND_DATA` and
# keep track of append mode explicitly. When writing data, this ensures to only
# write at the end of the file.
@system_append = false
getter? system_append = false

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
Expand Down
6 changes: 5 additions & 1 deletion src/crystal/system/win32/file_descriptor.cr
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,10 @@ module Crystal::System::FileDescriptor

@system_blocking = true

def system_append?
false
end

private def system_read(slice : Bytes) : Int32
handle = windows_handle
if ConsoleUtils.console?(handle)
Expand Down Expand Up @@ -160,7 +164,7 @@ module Crystal::System::FileDescriptor
FileDescriptor.system_info windows_handle
end

private def system_seek(offset, whence : IO::Seek) : Nil
def system_seek(offset, whence : IO::Seek) : Nil
if LibC.SetFilePointerEx(windows_handle, offset, nil, whence) == 0
raise IO::Error.from_winerror("Unable to seek", target: self)
end
Expand Down
5 changes: 5 additions & 0 deletions src/crystal/system/win32/iocp.cr
Original file line number Diff line number Diff line change
Expand Up @@ -264,6 +264,11 @@ struct Crystal::System::IOCP
def initialize(@handle : LibC::HANDLE)
end

def offset=(value : UInt64)
@overlapped.union.offset.offset = LibC::DWORD.new!(value)
@overlapped.union.offset.offsetHigh = LibC::DWORD.new!(value >> 32)
end

def wait_for_result(timeout, & : WinError ->)
wait_for_completion(timeout)

Expand Down
Loading