Skip to content

Commit

Permalink
Make sure to not write partial deps entries.
Browse files Browse the repository at this point in the history
When two ninja instances run in parallel in the same build directory,
one instance could write a deps entry header and a few ids, and then the
other instance could write a file name in the middle of the deps header.
When ninja reads this deps log on the next run, it will deserialize the
file name as indices, which will cause an out-of-bounds read.

(This can happen if a user runs a "compile this file" that uses ninja to
compile the current buffer in an editor, and also does a full build in a
terminal at the same time for example.)

While running two ninja instances in parallel in the same build
directory isn't a good idea, it happens to mostly work in non-deps mode:
There's redundant edge execution, but nothing crashes. This is partially
because the command log is line-buffered and a single log entry only
consists of a single line.

This change makes sure that deps entries are always written in one go,
like command log entries currently are. Running two ninja binaries in
parallel on the same build directory still isn't a great idea, but it's
less likely to lead to crashes.

See issue ninja-build#595.
  • Loading branch information
nico committed Jun 14, 2013
1 parent 75918b8 commit 13bad8d
Showing 1 changed file with 9 additions and 0 deletions.
9 changes: 9 additions & 0 deletions src/deps_log.cc
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,11 @@
const char kFileSignature[] = "# ninjadeps\n";
const int kCurrentVersion = 1;

// Since the size field is 2 bytes and the top bit marks deps entries, a single
// record can be at most 32 kB. Set the buffer size to this and flush the file
// buffer after every record to make sure records aren't written partially.
const int kMaxBufferSize = 1 << 15;

DepsLog::~DepsLog() {
Close();
}
Expand All @@ -48,6 +53,7 @@ bool DepsLog::OpenForWrite(const string& path, string* err) {
*err = strerror(errno);
return false;
}
setvbuf(file_, NULL, _IOFBF, kMaxBufferSize);
SetCloseOnExec(fileno(file_));

// Opening a file in append mode doesn't set the file pointer to the file's
Expand All @@ -64,6 +70,7 @@ bool DepsLog::OpenForWrite(const string& path, string* err) {
return false;
}
}
fflush(file_);

return true;
}
Expand Down Expand Up @@ -124,6 +131,7 @@ bool DepsLog::RecordDeps(Node* node, TimeStamp mtime,
id = nodes[i]->id();
fwrite(&id, 4, 1, file_);
}
fflush(file_);

// Update in-memory representation.
Deps* deps = new Deps(mtime, node_count);
Expand Down Expand Up @@ -318,6 +326,7 @@ bool DepsLog::RecordId(Node* node) {
uint16_t size = (uint16_t)node->path().size();
fwrite(&size, 2, 1, file_);
fwrite(node->path().data(), node->path().size(), 1, file_);
fflush(file_);

node->set_id(nodes_.size());
nodes_.push_back(node);
Expand Down

0 comments on commit 13bad8d

Please sign in to comment.