Skip to content

Fix stream_file position accounting for multi-buffer operations on Windows - #1762

Open
gycherish wants to merge 1 commit into
chriskohlhoff:masterfrom
gycherish:fix/win-iocp-stream-file-offset
Open

Fix stream_file position accounting for multi-buffer operations on Windows#1762
gycherish wants to merge 1 commit into
chriskohlhoff:masterfrom
gycherish:fix/win-iocp-stream-file-offset

Conversation

@gycherish

Copy link
Copy Markdown

Fixes #1346.

win_iocp_file_service advances the file position by the size of the whole buffer sequence:

uint64_t offset = impl.offset_;
impl.offset_ += asio::buffer_size(buffers);
handle_service_.async_write_some_at(impl, offset, buffers, handler, io_ex);

but win_iocp_handle_service submits only the sequence's first non-empty buffer, ReadFile and
WriteFile taking a single buffer each:

asio::const_buffer buffer =
  buffer_sequence_adapter<asio::const_buffer, ConstBufferSequence>::first(buffers);
return do_write(impl, offset, buffer, ec);

So the position moves past data that has not been transferred, and the composed operation resumes
from there. A scatter write leaves a hole in the file; a scatter read reports end_of_file with the
later buffers unfilled and no error the caller can act on.

All four sites are affected: write_some, async_write_some, read_some, async_read_some.

The fix

Advance by the buffer that is actually submitted. That size is known before the operation is
initiated, so the accounting stays exactly where it is today — no completion handler is involved and
nothing has to reach impl after the operation finishes.

A single-buffer sequence is unaffected: its first buffer is the whole sequence, so
first(buffers).size() == buffer_size(buffers).

buffer_sequence_adapter is already visible in this header through win_iocp_handle_service.hpp.

Scope

This makes a multi-buffer sequence behave like a single-buffer one. It does not change what happens
when the OS transfers less than the buffer it was given — a short read at EOF still over-advances,
exactly as it does today for a one-buffer sequence. Covering that would mean advancing by
bytes_transferred from the completion handler; that is a separate concern from the gaps reported
in #1346, and this change does not depend on it.

Only Windows is affected, and structurally so: IOCP needs an explicit offset per operation, so
win_iocp_file_service is the one backend that tracks the stream position itself.
io_uring_file_service forwards to the descriptor service and keeps no offset_ — the kernel owns
the position, and io_uring_prep_writev takes the whole sequence in one SQE. random_access_file
and the *_at overloads are unaffected either way, taking an explicit offset and never touching
impl.offset_.

…ndows.

win_iocp_file_service advances the file position by the size of the whole
buffer sequence, but win_iocp_handle_service submits only the sequence's first
non-empty buffer, ReadFile and WriteFile taking one buffer each. The position
therefore moves past data that has not been transferred, and the composed
read/write operations resume from there: a scatter write leaves a hole in the
file, and a scatter read reports end_of_file with the later buffers unfilled.

Advance by the buffer that is actually submitted. That size is known before the
operation is initiated, so the accounting stays where it is today and no
completion handler is involved. A single-buffer sequence is unaffected, its
first buffer being the whole sequence.

Only the Windows backend maintains the position itself, IOCP requiring an
explicit offset per operation; io_uring and the reactive backends leave it to
the kernel and pass the whole sequence to writev in one operation.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

asio::stream_file on windows

1 participant