RemoteStore::addToStore(): Fix race between stderrThread and NAR writer

As pointed out by @B4dM4n, the call to to.flush() on stderrThread is
unsafe because the NAR writer thread is also writing to 'to'.

Fixes #3943.
This commit is contained in:
Eelco Dolstra 2020-08-27 14:48:08 +02:00
parent 3ccf3801fb
commit a0f19d9f3a
3 changed files with 13 additions and 8 deletions

View file

@ -284,9 +284,9 @@ struct ConnectionHandle
RemoteStore::Connection * operator -> () { return &*handle; }
void processStderr(Sink * sink = 0, Source * source = 0)
void processStderr(Sink * sink = 0, Source * source = 0, bool flush = true)
{
auto ex = handle->processStderr(sink, source);
auto ex = handle->processStderr(sink, source, flush);
if (ex) {
daemonException = true;
std::rethrow_exception(ex);
@ -535,6 +535,8 @@ void RemoteStore::addToStore(const ValidPathInfo & info, Source & source,
if (GET_PROTOCOL_MINOR(conn->daemonVersion) >= 23) {
conn->to.flush();
std::exception_ptr ex;
struct FramedSink : BufferedSink
@ -574,7 +576,7 @@ void RemoteStore::addToStore(const ValidPathInfo & info, Source & source,
std::thread stderrThread([&]()
{
try {
conn.processStderr();
conn.processStderr(nullptr, nullptr, false);
} catch (...) {
ex = std::current_exception();
}
@ -884,8 +886,9 @@ static Logger::Fields readFields(Source & from)
}
std::exception_ptr RemoteStore::Connection::processStderr(Sink * sink, Source * source)
std::exception_ptr RemoteStore::Connection::processStderr(Sink * sink, Source * source, bool flush)
{
if (flush)
to.flush();
while (true) {

View file

@ -114,7 +114,7 @@ protected:
virtual ~Connection();
std::exception_ptr processStderr(Sink * sink = 0, Source * source = 0);
std::exception_ptr processStderr(Sink * sink = 0, Source * source = 0, bool flush = true);
};
ref<Connection> openConnectionWrapper();

View file

@ -23,7 +23,8 @@ struct Sink
};
/* A buffered abstract sink. */
/* A buffered abstract sink. Warning: a BufferedSink should not be
used from multiple threads concurrently. */
struct BufferedSink : virtual Sink
{
size_t bufSize, bufPos;
@ -66,7 +67,8 @@ struct Source
};
/* A buffered abstract source. */
/* A buffered abstract source. Warning: a BufferedSink should not be
used from multiple threads concurrently. */
struct BufferedSource : Source
{
size_t bufSize, bufPosIn, bufPosOut;