Execute a memory barrier when adding a new page to a space to synchronize access with concurrent sw…

src/spaces.cc

Index: src/spaces.cc
diff --git a/src/spaces.cc b/src/spaces.cc
index f5f7d6af0e6b45cc3637a91b46048484cc5689b1..4e276f47d85a71f5c6a7128215e1b40365e52c63 100644
--- a/src/spaces.cc
+++ b/src/spaces.cc
@@ -537,6 +537,15 @@ bool MemoryChunk::CommitArea(size_t requested) {
 void MemoryChunk::InsertAfter(MemoryChunk* other) {
   next_chunk_ = other->next_chunk_;
   prev_chunk_ = other;
+
+  // This memory barrier is needed since concurrent sweeper threads may iterate
+  // over the list of pages while a new page is inserted. 
+  MemoryBarrier();

[Michael Starzinger, 2013/03/11 11:40:36]

As discussed in length offline: We agree that starting to spread memory barriers
through the code (outside of dedicated synchronized data structures) is a
slippery slope. But since we don't have a cleaner solution for this data
structure that is embedded into the memory chunks, I agree to this temporary
solution.

But please add a TODO here, that we should find a cleaner way to guarantee that
the page list can be expanded concurrently. You can even attribute that TODO to
my name if you like.

+
+  // The following two write operations can take effect in arbitrary order
+  // since pages are always iterated by the sweeper threads in LIFO order, i.e,
+  // the inserted page becomes visible for the sweeper threads after 
+  // other->next_chunk_ = this;
   other->next_chunk_->prev_chunk_ = this;
   other->next_chunk_ = this;
 }