Reimplemented zone memory tracking to avoid race conditions that were causing crashes in the previo…

runtime/vm/zone.cc

Index: runtime/vm/zone.cc
diff --git a/runtime/vm/zone.cc b/runtime/vm/zone.cc
index 8b3cdac905faf210905786168a660c3c15e65329..83dbb2bc0eb3e3978dacdd602bea3038f7bb954b 100644
--- a/runtime/vm/zone.cc
+++ b/runtime/vm/zone.cc
@@ -30,6 +30,9 @@ class Zone::Segment {
   static void DeleteSegmentList(Segment* segment);
 
  private:
+  static void IncrementMemoryCapacity(uintptr_t size);
+  static void DecrementMemoryCapacity(uintptr_t size);

[siva, 2017/03/22 23:36:06]

The two versions of IncrementMemoryCapacity/DecrementMemoryCapacity here and in
class Zone could be shared.

+
   Segment* next_;
   intptr_t size_;
 
@@ -42,16 +45,28 @@ class Zone::Segment {
 };
 
 
+Zone::Segment* Zone::Segment::New(intptr_t size, Zone::Segment* next) {
+  ASSERT(size >= 0);
+  Segment* result = reinterpret_cast<Segment*>(malloc(size));
+  if (result == NULL) {
+    OUT_OF_MEMORY();
+  }
+  ASSERT(Utils::IsAligned(result->start(), Zone::kAlignment));
+#ifdef DEBUG
+  // Zap the entire allocated segment (including the header).
+  memset(result, kZapUninitializedByte, size);
+#endif
+  result->next_ = next;
+  result->size_ = size;
+  IncrementMemoryCapacity(size);
+  return result;
+}
+
+
 void Zone::Segment::DeleteSegmentList(Segment* head) {
   Segment* current = head;
-  Thread* current_thread = Thread::Current();
   while (current != NULL) {
-    if (current_thread != NULL) {
-      current_thread->DecrementMemoryUsage(current->size());
-    } else if (ApiNativeScope::Current() != NULL) {
-      // If there is no current thread, we might be inside of a native scope.
-      ApiNativeScope::DecrementNativeScopeMemoryUsage(current->size());
-    }
+    DecrementMemoryCapacity(current->size());
     Segment* next = current->next();
 #ifdef DEBUG
     // Zap the entire current segment (including the header).
@@ -63,29 +78,28 @@ void Zone::Segment::DeleteSegmentList(Segment* head) {
 }
 
 
-Zone::Segment* Zone::Segment::New(intptr_t size, Zone::Segment* next) {
-  ASSERT(size >= 0);
-  Segment* result = reinterpret_cast<Segment*>(malloc(size));
-  if (result == NULL) {
-    OUT_OF_MEMORY();
+void Zone::Segment::IncrementMemoryCapacity(uintptr_t size) {
+  Thread* current_thread = Thread::Current();
+  if (current_thread != NULL) {
+    current_thread->IncrementMemoryCapacity(size);
+  } else if (ApiNativeScope::Current() != NULL) {
+    // If there is no current thread, we might be inside of a native scope.
+    ApiNativeScope::IncrementNativeScopeMemoryCapacity(size);
   }
-  ASSERT(Utils::IsAligned(result->start(), Zone::kAlignment));
-#ifdef DEBUG
-  // Zap the entire allocated segment (including the header).
-  memset(result, kZapUninitializedByte, size);
-#endif
-  result->next_ = next;
-  result->size_ = size;
-  Thread* current = Thread::Current();
-  if (current != NULL) {
-    current->IncrementMemoryUsage(size);
+}
+
+
+void Zone::Segment::DecrementMemoryCapacity(uintptr_t size) {
+  Thread* current_thread = Thread::Current();
+  if (current_thread != NULL) {
+    current_thread->DecrementMemoryCapacity(size);
   } else if (ApiNativeScope::Current() != NULL) {
     // If there is no current thread, we might be inside of a native scope.
-    ApiNativeScope::IncrementNativeScopeMemoryUsage(size);
+    ApiNativeScope::DecrementNativeScopeMemoryCapacity(size);
   }
-  return result;
 }
 
+
 // TODO(bkonyi): We need to account for the initial chunk size when a new zone
 // is created within a new thread or ApiNativeScope when calculating high
 // watermarks or memory consumption.
@@ -98,10 +112,7 @@ Zone::Zone()
       handles_(),
       previous_(NULL) {
   ASSERT(Utils::IsAligned(position_, kAlignment));
-  Thread* current = Thread::Current();
-  if (current != NULL) {
-    current->IncrementMemoryUsage(kInitialChunkSize);
-  }
+  IncrementMemoryCapacity(kInitialChunkSize);
 #ifdef DEBUG
   // Zap the entire initial buffer.
   memset(initial_buffer_.pointer(), kZapUninitializedByte,
@@ -114,11 +125,8 @@ Zone::~Zone() {
   if (FLAG_trace_zones) {
     DumpZoneSizes();
   }
-  Thread* current = Thread::Current();
-  if (current != NULL) {
-    current->DecrementMemoryUsage(kInitialChunkSize);
-  }
   DeleteAll();
+  DecrementMemoryCapacity(kInitialChunkSize);
 }
 
 
@@ -144,8 +152,8 @@ void Zone::DeleteAll() {
 }
 
 
-intptr_t Zone::SizeInBytes() const {
-  intptr_t size = 0;
+uintptr_t Zone::SizeInBytes() const {
+  uintptr_t size = 0;
   for (Segment* s = large_segments_; s != NULL; s = s->next()) {
     size += s->size();
   }
@@ -160,8 +168,8 @@ intptr_t Zone::SizeInBytes() const {
 }
 
 
-intptr_t Zone::CapacityInBytes() const {
-  intptr_t size = 0;
+uintptr_t Zone::CapacityInBytes() const {
+  uintptr_t size = 0;
   for (Segment* s = large_segments_; s != NULL; s = s->next()) {
     size += s->size();
   }
@@ -176,6 +184,28 @@ intptr_t Zone::CapacityInBytes() const {
 }
 
 
+void Zone::IncrementMemoryCapacity(uintptr_t size) {
+  Thread* current_thread = Thread::Current();
+  if (current_thread != NULL) {
+    current_thread->IncrementMemoryCapacity(size);
+  } else if (ApiNativeScope::Current() != NULL) {
+    // If there is no current thread, we might be inside of a native scope.
+    ApiNativeScope::IncrementNativeScopeMemoryCapacity(size);
+  }
+}
+
+
+void Zone::DecrementMemoryCapacity(uintptr_t size) {
+  Thread* current_thread = Thread::Current();
+  if (current_thread != NULL) {
+    current_thread->DecrementMemoryCapacity(size);
+  } else if (ApiNativeScope::Current() != NULL) {
+    // If there is no current thread, we might be inside of a native scope.
+    ApiNativeScope::DecrementNativeScopeMemoryCapacity(size);
+  }
+}
+
+
 uword Zone::AllocateExpand(intptr_t size) {
   ASSERT(size >= 0);
   if (FLAG_trace_zones) {
@@ -300,19 +330,6 @@ char* Zone::VPrint(const char* format, va_list args) {
 }
 
 
-#ifndef PRODUCT
-// TODO(bkonyi): Currently dead code. See issue #28885.
-void Zone::PrintJSON(JSONStream* stream) const {
-  JSONObject jsobj(stream);
-  intptr_t capacity = CapacityInBytes();
-  intptr_t used_size = SizeInBytes();
-  jsobj.AddProperty("type", "_Zone");
-  jsobj.AddProperty("capacity", capacity);
-  jsobj.AddProperty("used", used_size);
-}
-#endif
-
-
 StackZone::StackZone(Thread* thread) : StackResource(thread), zone_() {
   if (FLAG_trace_zones) {
     OS::PrintErr("*** Starting a new Stack zone 0x%" Px "(0x%" Px ")\n",