Skip to content

Commit dba6a54

Browse files
sgerbinosgerbino
committed
fix(io): treat a fully transferred read/write as success
read and write reported any error_code surfaced by the final read_some/write_some, even when that completion delivered the last bytes and filled the buffer. A generic observer (when_any/when_all) then saw a completed transfer as a failure. The most common trigger is end-of-stream coincident with a full read (read_some returns [eof, n] that fills the buffer). Report the contingency only when the buffer was not fully transferred: when n == buffer_size(buffers) the transfer succeeded and ec is success. Tighten the documented postcondition accordingly. Codify the same invariant in the ReadStream and WriteStream concept contracts: a read_some/write_some that fills its buffer reports success, so n == buffer_size(buffers) implies no error (a coincident condition such as end-of-stream surfaces on a subsequent call). Add a mock stream that reports a contingency in the same completion that transfers bytes (the test read/write streams report errors with zero bytes) and cover both the filled-is-success and short-transfer-is-reported cases.
1 parent 880619c commit dba6a54

6 files changed

Lines changed: 178 additions & 9 deletions

File tree

‎include/boost/capy/concept/read_stream.hpp‎

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -43,10 +43,17 @@ namespace capy {
4343
4444
@li If `!ec`, then `n >= 1 && n <= buffer_size( buffers )`.
4545
`n` bytes were read into the buffer sequence.
46-
@li If `ec`, then `n >= 0 && n <= buffer_size( buffers )`.
46+
@li If `ec`, then `n >= 0 && n < buffer_size( buffers )`.
4747
`n` is the number of bytes read before the I/O
4848
condition arose.
4949
50+
Equivalently, `n == buffer_size( buffers )` implies `!ec`: a
51+
completion that fills the buffer sequence is a success, even when
52+
the underlying operation also signals a condition such as
53+
end-of-stream. That condition is reported on a subsequent read.
54+
This lets generic composition algorithms such as `when_all` and
55+
`when_any` distinguish a completed transfer from a failure.
56+
5057
If `buffer_empty( buffers )` is `true`, `n` is 0. The empty
5158
buffer is not itself a cause for error, but `ec` may reflect
5259
the state of the stream.

‎include/boost/capy/concept/write_stream.hpp‎

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -47,10 +47,17 @@ namespace capy {
4747
4848
@li If `!ec`, then `n >= 1 && n <= buffer_size( buffers )`.
4949
`n` bytes were written from the buffer sequence.
50-
@li If `ec`, then `n >= 0 && n <= buffer_size( buffers )`.
50+
@li If `ec`, then `n >= 0 && n < buffer_size( buffers )`.
5151
`n` is the number of bytes written before the I/O
5252
condition arose.
5353
54+
Equivalently, `n == buffer_size( buffers )` implies `!ec`: a
55+
completion that writes the entire buffer sequence is a success, even
56+
when the underlying operation also signals a condition. That
57+
condition is reported on a subsequent write. This lets generic
58+
composition algorithms such as `when_all` and `when_any` distinguish
59+
a completed transfer from a failure.
60+
5461
If `buffer_empty( buffers )` is `true`, `n` is 0. The empty
5562
buffer is not itself a cause for error, but `ec` may reflect
5663
the state of the stream.

‎include/boost/capy/read.hpp‎

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -46,15 +46,19 @@ namespace capy {
4646
4747
Contingencies:
4848
49-
@li The first contingency reported from awaiting @c stream.read_some .
49+
@li The first contingency reported from awaiting @c stream.read_some
50+
while `buffers` is not yet filled. A contingency that accompanies
51+
the read which fills `buffers` is not reported: a completed
52+
transfer is a success.
5053
5154
Notable conditions:
5255
5356
@li @c cond::canceled — Operation was cancelled,
5457
@li @c cond::eof — Stream reached end before `buffers` was filled.
5558
5659
@par Await-postcondition
57-
`ec || n == buffer_size(buffers)`.
60+
If `n == buffer_size(buffers)` the transfer completed and `ec` is
61+
success; otherwise `ec` is set.
5862
5963
@param stream The stream to read from. If the lifetime of `stream` ends
6064
before the coroutine finishes, the behavior is undefined.
@@ -100,7 +104,9 @@ read(S& stream, MB buffers) ->
100104
auto [ec, n] = co_await stream.read_some(consuming.data());
101105
consuming.remove_prefix(n);
102106
total_read += n;
103-
if(ec)
107+
// A contingency that still completed the transfer is a success:
108+
// report it only when the buffer was not filled.
109+
if(ec && total_read < total_size)
104110
co_return {ec, total_read};
105111
}
106112

‎include/boost/capy/write.hpp‎

Lines changed: 9 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -48,8 +48,10 @@ namespace capy {
4848
4949
Contingencies:
5050
51-
@li The first contingency reported from
52-
awaiting @c stream.write_some .
51+
@li The first contingency reported from awaiting @c stream.write_some
52+
while not all bytes have been written. A contingency that accompanies
53+
the write which transfers the last bytes is not reported: a completed
54+
transfer is a success.
5355
5456
Notable conditions:
5557
@@ -59,7 +61,8 @@ namespace capy {
5961
6062
@par Await-postcondition
6163
62-
`ec || n == buffer_size(buffers)`.
64+
If `n == buffer_size(buffers)` the transfer completed and `ec` is
65+
success; otherwise `ec` is set.
6366
6467
6568
@param stream The stream to write to. If the lifetime of `stream` ends
@@ -100,7 +103,9 @@ auto write(S& stream, CB buffers) -> io_task<std::size_t>
100103
auto [ec, n] = co_await stream.write_some(consuming.data());
101104
consuming.remove_prefix(n);
102105
total_written += n;
103-
if(ec)
106+
// A contingency that still completed the transfer is a success:
107+
// report it only when not all bytes were written.
108+
if(ec && total_written < total_size)
104109
co_return {ec, total_written};
105110
}
106111

‎test/unit/read.cpp‎

Lines changed: 80 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -180,6 +180,43 @@ struct circular_dynamic_buffer_factory
180180

181181
} // namespace
182182

183+
// Mock whose read_some reports a contingency in the SAME completion that
184+
// transfers bytes. The test read_stream cannot do this (it reports errors
185+
// and eof with zero bytes), so it is needed to exercise the
186+
// "buffer filled but ec set" boundary.
187+
struct contingent_read_stream
188+
{
189+
std::error_code ec;
190+
std::size_t deliver;
191+
192+
template<MutableBufferSequence MB>
193+
auto
194+
read_some(MB buffers)
195+
{
196+
struct awaitable
197+
{
198+
contingent_read_stream* self_;
199+
MB buffers_;
200+
201+
bool await_ready() const noexcept { return true; }
202+
203+
void await_suspend(
204+
std::coroutine_handle<>, io_env const*) const noexcept {}
205+
206+
io_result<std::size_t>
207+
await_resume()
208+
{
209+
std::size_t const cap = buffer_size(buffers_);
210+
std::size_t const n =
211+
self_->deliver < cap ? self_->deliver : cap;
212+
self_->deliver -= n;
213+
return {self_->ec, n};
214+
}
215+
};
216+
return awaitable{this, buffers};
217+
}
218+
};
219+
183220
struct read_test
184221
{
185222
//----------------------------------------------------------
@@ -351,13 +388,56 @@ struct read_test
351388
}));
352389
}
353390

391+
void
392+
testFullTransferContingency()
393+
{
394+
// A contingency on the read that fills the buffer is a success
395+
// (n == buffer_size); a contingency on a short transfer is
396+
// reported.
397+
398+
// eof coincident with a full fill -> success
399+
BOOST_TEST(test::fuse().inert([](test::fuse&) -> task<void>
400+
{
401+
contingent_read_stream rs{error::eof, 8};
402+
single_buffer_factory bf(8);
403+
auto [ec, n] = co_await read(rs, bf.buffer());
404+
BOOST_TEST(! ec);
405+
BOOST_TEST_EQ(n, 8u);
406+
}));
407+
408+
// contingency with a short transfer -> reported
409+
BOOST_TEST(test::fuse().inert([](test::fuse&) -> task<void>
410+
{
411+
contingent_read_stream rs{error::eof, 5};
412+
single_buffer_factory bf(8);
413+
auto [ec, n] = co_await read(rs, bf.buffer());
414+
BOOST_TEST(ec == cond::eof);
415+
BOOST_TEST_EQ(n, 5u);
416+
}));
417+
418+
// the suppressed condition is deferred, not lost: the next read
419+
// surfaces it (here the stream is at eof with no more data).
420+
BOOST_TEST(test::fuse().inert([](test::fuse&) -> task<void>
421+
{
422+
contingent_read_stream rs{error::eof, 8};
423+
single_buffer_factory bf(8);
424+
auto [ec1, n1] = co_await read(rs, bf.buffer());
425+
BOOST_TEST(! ec1);
426+
BOOST_TEST_EQ(n1, 8u);
427+
auto [ec2, n2] = co_await read(rs, bf.buffer());
428+
BOOST_TEST(ec2 == cond::eof);
429+
BOOST_TEST_EQ(n2, 0u);
430+
}));
431+
}
432+
354433
void
355434
testReadStream()
356435
{
357436
testReadSingleBuffer();
358437
testReadBufferArray();
359438
testReadBufferPair();
360439
testReadStoredAwaitableTemporarySequence();
440+
testFullTransferContingency();
361441
}
362442

363443
//----------------------------------------------------------

‎test/unit/write.cpp‎

Lines changed: 64 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -120,6 +120,42 @@ struct buffer_pair_factory
120120

121121
} // namespace
122122

123+
// Mock whose write_some reports a contingency in the SAME completion that
124+
// transfers bytes, to exercise the "all bytes written but ec set"
125+
// boundary. The test write_stream reports errors with zero bytes.
126+
struct contingent_write_stream
127+
{
128+
std::error_code ec;
129+
std::size_t accept;
130+
131+
template<ConstBufferSequence CB>
132+
auto
133+
write_some(CB buffers)
134+
{
135+
struct awaitable
136+
{
137+
contingent_write_stream* self_;
138+
CB buffers_;
139+
140+
bool await_ready() const noexcept { return true; }
141+
142+
void await_suspend(
143+
std::coroutine_handle<>, io_env const*) const noexcept {}
144+
145+
io_result<std::size_t>
146+
await_resume()
147+
{
148+
std::size_t const cap = buffer_size(buffers_);
149+
std::size_t const n =
150+
self_->accept < cap ? self_->accept : cap;
151+
self_->accept -= n;
152+
return {self_->ec, n};
153+
}
154+
};
155+
return awaitable{this, buffers};
156+
}
157+
};
158+
123159
struct write_test
124160
{
125161
//----------------------------------------------------------
@@ -283,13 +319,41 @@ struct write_test
283319
}));
284320
}
285321

322+
void
323+
testFullTransferContingency()
324+
{
325+
// A contingency on the write that transfers all bytes is a
326+
// success (n == buffer_size); a short transfer reports it.
327+
328+
// contingency coincident with a full write -> success
329+
BOOST_TEST(test::fuse().inert([](test::fuse&) -> task<void>
330+
{
331+
contingent_write_stream ws{error::canceled, 8};
332+
single_buffer_factory bf("12345678");
333+
auto [ec, n] = co_await write(ws, bf.buffer());
334+
BOOST_TEST(! ec);
335+
BOOST_TEST_EQ(n, 8u);
336+
}));
337+
338+
// contingency with a short write -> reported
339+
BOOST_TEST(test::fuse().inert([](test::fuse&) -> task<void>
340+
{
341+
contingent_write_stream ws{error::canceled, 5};
342+
single_buffer_factory bf("12345678");
343+
auto [ec, n] = co_await write(ws, bf.buffer());
344+
BOOST_TEST(!! ec);
345+
BOOST_TEST_EQ(n, 5u);
346+
}));
347+
}
348+
286349
void
287350
testWriteStream()
288351
{
289352
testWriteSingleBuffer();
290353
testWriteBufferArray();
291354
testWriteBufferPair();
292355
testWriteStoredAwaitableTemporarySequence();
356+
testFullTransferContingency();
293357
}
294358

295359
//----------------------------------------------------------

0 commit comments

Comments
 (0)