Skip to content
Snippets Groups Projects
Commit 75f41254 authored by Nat Goodspeed's avatar Nat Goodspeed
Browse files

Break large buffer into chunks to write to LLProcess child pipe.

On Windows we ran into trouble trying to write a biggish (~1 MB) buffer of
data to the child process's stdin pipe with a single apr_file_write() call.
The child actually received corrupted data -- suggesting a possible bug in
either APR or Windows pipes; the same test driving the same logic worked fine
on Mac and Linux. Empirically, iterating over chunks of the buffered data is
more robust.
parent d72d9f73
Branches
Tags
No related merge requests found
...@@ -168,15 +168,23 @@ class WritePipeImpl: public LLProcess::WritePipe ...@@ -168,15 +168,23 @@ class WritePipeImpl: public LLProcess::WritePipe
const_buffer_sequence bufs = mStreambuf.data(); const_buffer_sequence bufs = mStreambuf.data();
// In general, our streambuf might contain a number of different // In general, our streambuf might contain a number of different
// physical buffers; iterate over those. // physical buffers; iterate over those.
bool keepwriting = true;
for (const_buffer_sequence::const_iterator bufi(bufs.begin()), bufend(bufs.end()); for (const_buffer_sequence::const_iterator bufi(bufs.begin()), bufend(bufs.end());
bufi != bufend; ++bufi) bufi != bufend && keepwriting; ++bufi)
{ {
// http://www.boost.org/doc/libs/1_49_0_beta1/doc/html/boost_asio/reference/buffer.html#boost_asio.reference.buffer.accessing_buffer_contents // http://www.boost.org/doc/libs/1_49_0_beta1/doc/html/boost_asio/reference/buffer.html#boost_asio.reference.buffer.accessing_buffer_contents
std::size_t towrite(boost::asio::buffer_size(*bufi)); // Although apr_file_write() accepts const void*, we
// manipulate const char* so we can increment the pointer.
const char* remainptr = boost::asio::buffer_cast<const char*>(*bufi);
std::size_t remainlen = boost::asio::buffer_size(*bufi);
while (remainlen)
{
// Tackle the current buffer in discrete chunks. On
// Windows, we've observed strange failures when trying to
// write big lengths (~1 MB) in a single operation.
std::size_t towrite((std::min)(remainlen, std::size_t(32*1024)));
apr_size_t written(towrite); apr_size_t written(towrite);
apr_status_t err = apr_file_write(mPipe, apr_status_t err = apr_file_write(mPipe, remainptr, &written);
boost::asio::buffer_cast<const void*>(*bufi),
&written);
// EAGAIN is exactly what we want from a nonblocking pipe. // EAGAIN is exactly what we want from a nonblocking pipe.
// Rather than waiting for data, it should return immediately. // Rather than waiting for data, it should return immediately.
if (! (err == APR_SUCCESS || APR_STATUS_IS_EAGAIN(err))) if (! (err == APR_SUCCESS || APR_STATUS_IS_EAGAIN(err)))
...@@ -190,16 +198,28 @@ class WritePipeImpl: public LLProcess::WritePipe ...@@ -190,16 +198,28 @@ class WritePipeImpl: public LLProcess::WritePipe
// written. Make sure we consume those later. (Don't consume them // written. Make sure we consume those later. (Don't consume them
// now, that would invalidate the buffer iterator sequence!) // now, that would invalidate the buffer iterator sequence!)
consumed += written; consumed += written;
// don't forget to advance to next chunk of current buffer
remainptr += written;
remainlen -= written;
char msgbuf[512];
LL_DEBUGS("LLProcess") << "wrote " << written << " of " << towrite LL_DEBUGS("LLProcess") << "wrote " << written << " of " << towrite
<< " bytes to " << mDesc << " bytes to " << mDesc
<< " (original " << total << ")" << LL_ENDL; << " (original " << total << "),"
<< " code " << err << ": "
<< apr_strerror(err, msgbuf, sizeof(msgbuf))
<< LL_ENDL;
// The parent end of this pipe is nonblocking. If we weren't able // The parent end of this pipe is nonblocking. If we weren't able
// to write everything we wanted, don't keep banging on it -- that // to write everything we wanted, don't keep banging on it -- that
// won't change until the child reads some. Wait for next tick(). // won't change until the child reads some. Wait for next tick().
if (written < towrite) if (written < towrite)
{
keepwriting = false; // break outer loop over buffers too
break; break;
} }
} // next chunk of current buffer
} // next buffer
// In all, we managed to write 'consumed' bytes. Remove them from the // In all, we managed to write 'consumed' bytes. Remove them from the
// streambuf so we don't keep trying to send them. This could be // streambuf so we don't keep trying to send them. This could be
// anywhere from 0 up to mStreambuf.size(); anything we haven't yet // anywhere from 0 up to mStreambuf.size(); anything we haven't yet
......
0% Loading or .
You are about to add 0 people to the discussion. Proceed with caution.
Please register or to comment