Fix bug when transferring SharedArrayBuffer to multiple Workers.

src/d8.cc

Index: src/d8.cc
diff --git a/src/d8.cc b/src/d8.cc
index 7db6f3ed9e442081528e05140bd25b8911e26cb6..a66213e19f83185607d217f606b63994b3fbac2b 100644
--- a/src/d8.cc
+++ b/src/d8.cc
@@ -1533,22 +1533,26 @@ void SourceGroup::WaitForThread() {
 
 
 SerializationData::~SerializationData() {
-  // Any ArrayBuffer::Contents are owned by this SerializationData object.
-  // SharedArrayBuffer::Contents may be used by other threads, so must be
+  // Any ArrayBuffer::Contents are owned by this SerializationData object if
+  // ownership hasn't been transferred out via ReadArrayBufferContents.
+  // SharedArrayBuffer::Contents may be used by multiple threads, so must be
   // cleaned up by the main thread in Shell::CleanupWorkers().
-  for (int i = 0; i < array_buffer_contents.length(); ++i) {
-    ArrayBuffer::Contents& contents = array_buffer_contents[i];
-    Shell::array_buffer_allocator->Free(contents.Data(), contents.ByteLength());
+  for (int i = 0; i < array_buffer_contents_.length(); ++i) {
+    ArrayBuffer::Contents& contents = array_buffer_contents_[i];
+    if (contents.Data()) {

[jochen (gone - plz use gerrit), 2015/07/03 14:29:37]

how can data ever be null?

[Jarin, 2015/07/06 05:04:19]

As the comment above says, ReadArrayBufferContents can set the contents to empty
(line 1600). No?

[binji, 2015/07/06 06:23:27]

Yes, that's correct.

+      Shell::array_buffer_allocator->Free(contents.Data(),
+                                          contents.ByteLength());
+    }
   }
 }
 
 
-void SerializationData::WriteTag(SerializationTag tag) { data.Add(tag); }
+void SerializationData::WriteTag(SerializationTag tag) { data_.Add(tag); }
 
 
 void SerializationData::WriteMemory(const void* p, int length) {
   if (length > 0) {
-    i::Vector<uint8_t> block = data.AddBlock(0, length);
+    i::Vector<uint8_t> block = data_.AddBlock(0, length);
     memcpy(&block[0], p, length);
   }
 }
@@ -1556,18 +1560,18 @@ void SerializationData::WriteMemory(const void* p, int length) {
 
 void SerializationData::WriteArrayBufferContents(
     const ArrayBuffer::Contents& contents) {
-  array_buffer_contents.Add(contents);
+  array_buffer_contents_.Add(contents);
   WriteTag(kSerializationTagTransferredArrayBuffer);
-  int index = array_buffer_contents.length() - 1;
+  int index = array_buffer_contents_.length() - 1;
   Write(index);
 }
 
 
 void SerializationData::WriteSharedArrayBufferContents(
     const SharedArrayBuffer::Contents& contents) {
-  shared_array_buffer_contents.Add(contents);
+  shared_array_buffer_contents_.Add(contents);
   WriteTag(kSerializationTagTransferredSharedArrayBuffer);
-  int index = shared_array_buffer_contents.length() - 1;
+  int index = shared_array_buffer_contents_.length() - 1;
   Write(index);
 }
 
@@ -1579,7 +1583,7 @@ SerializationTag SerializationData::ReadTag(int* offset) const {
 
 void SerializationData::ReadMemory(void* p, int length, int* offset) const {
   if (length > 0) {
-    memcpy(p, &data[*offset], length);
+    memcpy(p, &data_[*offset], length);
     (*offset) += length;
   }
 }
@@ -1588,16 +1592,20 @@ void SerializationData::ReadMemory(void* p, int length, int* offset) const {
 void SerializationData::ReadArrayBufferContents(ArrayBuffer::Contents* contents,
                                                 int* offset) const {
   int index = Read<int>(offset);
-  DCHECK(index < array_buffer_contents.length());
-  *contents = array_buffer_contents[index];
+  DCHECK(index < array_buffer_contents_.length());
+  *contents = array_buffer_contents_[index];
+  // Ownership of this ArrayBuffer::Contents is passed to the caller. Neuter
+  // our copy so it won't be double-free'd when this SerializationData is
+  // destroyed.
+  array_buffer_contents_[index] = ArrayBuffer::Contents();
 }
 
 
 void SerializationData::ReadSharedArrayBufferContents(
     SharedArrayBuffer::Contents* contents, int* offset) const {
   int index = Read<int>(offset);
-  DCHECK(index < shared_array_buffer_contents.length());
-  *contents = shared_array_buffer_contents[index];
+  DCHECK(index < shared_array_buffer_contents_.length());
+  *contents = shared_array_buffer_contents_[index];
 }
 
 
@@ -2040,7 +2048,9 @@ bool Shell::SerializeValue(Isolate* isolate, Handle<Value> value,
         return false;
       }
 
-      ArrayBuffer::Contents contents = array_buffer->Externalize();
+      ArrayBuffer::Contents contents = array_buffer->IsExternal()
+                                           ? array_buffer->GetContents()
+                                           : array_buffer->Externalize();
       array_buffer->Neuter();
       out_data->WriteArrayBufferContents(contents);
     } else {
@@ -2069,9 +2079,14 @@ bool Shell::SerializeValue(Isolate* isolate, Handle<Value> value,
       return false;
     }
 
-    SharedArrayBuffer::Contents contents = sab->Externalize();
+    SharedArrayBuffer::Contents contents;
+    if (sab->IsExternal()) {
+      contents = sab->GetContents();
+    } else {
+      contents = sab->Externalize();
+      externalized_shared_contents_.Add(contents);
+    }
     out_data->WriteSharedArrayBufferContents(contents);
-    externalized_shared_contents_.Add(contents);
   } else if (value->IsObject()) {
     Handle<Object> object = Handle<Object>::Cast(value);
     if (FindInObjectList(object, *seen_objects)) {
@@ -2187,8 +2202,8 @@ MaybeLocal<Value> Shell::DeserializeValue(Isolate* isolate,
     case kSerializationTagTransferredArrayBuffer: {
       ArrayBuffer::Contents contents;
       data.ReadArrayBufferContents(&contents, offset);
-      result =
-          ArrayBuffer::New(isolate, contents.Data(), contents.ByteLength());
+      result = ArrayBuffer::New(isolate, contents.Data(), contents.ByteLength(),
+                                ArrayBufferCreationMode::kInternalized);
       break;
     }
     case kSerializationTagTransferredSharedArrayBuffer: {