chriskohlhoff / asio

Asio C++ Library
http://think-async.com/Asio
4.95k stars 1.22k forks source link

boost::asio::async_read_until is incompatible with other async read functions #113

Open paulfertser opened 8 years ago

paulfertser commented 8 years ago

When dealing with certain protocols one might think that it's a good idea to mix fix-sized reads with read_until invocations. This seems to be unsupported but I didn't manage to find any clear "DO NOT" notice in the official documentation. Cursory internet search suggests I wasn't the only one confused.

Since I do not have enough experience in this area, I'm not able to provide a reasonable patch; so I am asking you to please consider improving the documentation in this regard.

vinniefalco commented 8 years ago

I'm not quite sure I follow. You say you can't intermix calls to boost::asio::read with calls to boost::asio::read_until? It works fine for me, I do it in my WebSocket library. Can you provide an example? Or links to evidence to the contrary?

paulfertser commented 6 years ago

Quoting Gavin Lambert:

... note that read_until may (and usually does) read data beyond the delimiter into the buffer. It is therefore essential to preserve this buffer between calls (ie. don't make it a local variable), and to properly mark the bytes that you are extracting from it as consumed so that you don't keep getting the same line again. This also means that you shouldn't mix calls to read_until with other read calls on the same socket, at least not without taking the buffer's existing contents into account.

http://boost.2283326.n4.nabble.com/Cannot-get-boost-asio-read-until-to-properly-read-a-line-new-to-boost-asio-td4681406.html

mabrarov commented 6 years ago

Sorry for being obvious, but doesn't documentation of async_read_until cover requested warning?

async_read_until (1 of 8 overloads):

This operation is implemented in terms of zero or more calls to the stream's async_read_some function, and is known as a composed operation. If the dynamic buffer sequence's get area already contains the delimiter, this asynchronous operation completes immediately. The program must ensure that the stream performs no other read operations (such as async_read, async_read_until, the stream's async_read_some function, or any other composed operations that perform reads) until this operation completes.

paulfertser commented 6 years ago

To be honest, that sounds like a thread-safeness warning to me. Or am I misinterpreting that? The original issue I had was caused by boost::asio::read not returning data that was already read by read_until past the delimiter.

mabrarov commented 6 years ago

@paulfertser, regarding

sounds like a thread-safeness warning to me

Sorry, but there is no "thread" word in that doc and it clearly states:

until this operation completes.

Anyway, be aware of "compose operations" (consisting of async_read_some / async_write_some calls) and their restrictions because of their nature.

Please note also that if some compose operation (if both conditions are met):

  1. Uses some completion condition callback (like MatchCondition in async_read_until).
  2. Uses completion handler which is wrapped with some strand.

then completion condition callback is called using the same strand instance which was used to wrap completion handler, i.e. all callbacks and background work performed by composed operation is executed within the same execution context (like strand instance) as completion handler uses (actually provides / exposes with associated_executor / asio_handler_invoke).

paulfertser commented 6 years ago

I agree using the word thread wasn't fully appropriate here. So what does "completes" mean in this context? I would assume it's when the result of read_until is ready (either the corresponding callback is called or an std::future becomes available etc). But the problem is that some bytes past the delimiter might have been read into a buffer and they can be retrieved only by another subsequent read_until call but not with read call as it bypasses that buffer.

mabrarov commented 6 years ago

So what does "completes" mean in this context?

Your assumption is correct.

But the problem is that some bytes past the delimiter might have been read into a buffer

That's documented too and this decision is easy to understand / to allow taking into account stream nature (it's impossible to read more than 1 byte each time - to keep performance - and do not keep in buffer smth more than caller requested when end of requested sequence of bytes was reached):

Remarks

After a successful async_read_until operation, the dynamic buffer sequence may contain additional data beyond that which matched the function object. An application will typically leave that data in the dynamic buffer sequence for a subsequent async_read_until operation to examine.

paulfertser commented 6 years ago

Probably the documentation was different in 2016, probably I wasn't reading it properly but it was really not obvious that async_read bypasses the buffer. I remember searching the Internet back then and being able to find reports of few other confused developers. If you think the docs are fine now, please close the ticket, and sorry for the noise.

mabrarov commented 6 years ago

If you think the docs are fine now

Not sure if docs are fine :) because I heard similar questions many times and I personally used sources of Asio to understand some concepts before I found them in docs. Maybe there is a need of additional section for concept of "composed operation" and maybe there is a need of FAQ too.

So let's wait for official maintainer(s) to take decision on this ticket and feel free to ask if you have questions (here or at asio_samples which was created with intention to provide more complex examples built on top of Asio examples taken from official documentation).