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..7d980691685b41a8e8f9d9b5854ae2c123f550c7 100644
--- a/runtime/vm/zone.cc
+++ b/runtime/vm/zone.cc
@@ -47,10 +47,10 @@ void Zone::Segment::DeleteSegmentList(Segment* head) {
   Thread* current_thread = Thread::Current();
   while (current != NULL) {
     if (current_thread != NULL) {
-      current_thread->DecrementMemoryUsage(current->size());
+      current_thread->DecrementMemoryCapacity(current->size());

[Cutch, 2017/03/22 17:21:49]

ditto here:

IncrementThreadMemoryCapacity / DecrementThreadMemoryCapacity

Fold all the current_thread != NULL checks into those two functions and call
those functions instead of littering the code with the checks.

[bkonyi, 2017/03/22 18:01:48]

Done.

     } else if (ApiNativeScope::Current() != NULL) {
       // If there is no current thread, we might be inside of a native scope.
-      ApiNativeScope::DecrementNativeScopeMemoryUsage(current->size());
+      ApiNativeScope::DecrementNativeScopeMemoryCapacity(current->size());
     }
     Segment* next = current->next();
 #ifdef DEBUG
@@ -76,12 +76,12 @@ Zone::Segment* Zone::Segment::New(intptr_t size, Zone::Segment* next) {
 #endif
   result->next_ = next;
   result->size_ = size;
-  Thread* current = Thread::Current();
-  if (current != NULL) {
-    current->IncrementMemoryUsage(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::IncrementNativeScopeMemoryUsage(size);
+    ApiNativeScope::IncrementNativeScopeMemoryCapacity(size);
   }
   return result;
 }
@@ -98,9 +98,9 @@ Zone::Zone()
       handles_(),
       previous_(NULL) {
   ASSERT(Utils::IsAligned(position_, kAlignment));
-  Thread* current = Thread::Current();
-  if (current != NULL) {
-    current->IncrementMemoryUsage(kInitialChunkSize);
+  Thread* current_thread = Thread::Current();
+  if (current_thread != NULL) {
+    current_thread->IncrementMemoryCapacity(kInitialChunkSize);
   }
 #ifdef DEBUG
   // Zap the entire initial buffer.
@@ -114,15 +114,20 @@ Zone::~Zone() {
   if (FLAG_trace_zones) {
     DumpZoneSizes();
   }
-  Thread* current = Thread::Current();
-  if (current != NULL) {
-    current->DecrementMemoryUsage(kInitialChunkSize);
-  }
   DeleteAll();
+
+  Thread* current_thread = Thread::Current();
+  if (current_thread != NULL) {
+    current_thread->DecrementMemoryCapacity(kInitialChunkSize);
+  }
 }
 
 
 void Zone::DeleteAll() {
+  Thread* current_thread = Thread::Current();
+  if (current_thread != NULL) {
+    current_thread->DecrementMemoryUsage(SizeInBytes());
+  }
   // Traverse the chained list of segments, zapping (in debug mode)
   // and freeing every zone segment.
   if (head_ != NULL) {
@@ -144,8 +149,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();
   }
@@ -176,6 +181,15 @@ intptr_t Zone::CapacityInBytes() const {
 }
 
 
+intptr_t Zone::FreeCapacityInBytes() const {
+  if (head_ == NULL) {
+    return initial_buffer_.start() + initial_buffer_.size() - position_;
+  } else {
+    return head_->start() + head_->size() - position_;
+  }
+}
+
+
 uword Zone::AllocateExpand(intptr_t size) {
   ASSERT(size >= 0);
   if (FLAG_trace_zones) {
@@ -197,9 +211,16 @@ uword Zone::AllocateExpand(intptr_t size) {
     return AllocateLargeSegment(size);
   }
 
+  intptr_t remaining_capacity = FreeCapacityInBytes();
+
   // Allocate another segment and chain it up.
   head_ = Segment::New(kSegmentSize, head_);
 
+  Thread* current_thread = Thread::Current();
+  if (current_thread != NULL) {
+    current_thread->IncrementMemoryUsage(remaining_capacity + size);
+  }
+
   // Recompute 'position' and 'limit' based on the new head segment.
   uword result = Utils::RoundUp(head_->start(), kAlignment);
   position_ = result + size;
@@ -222,6 +243,11 @@ uword Zone::AllocateLargeSegment(intptr_t size) {
   size += sizeof(Segment);  // Account for book keeping fields in size.
   large_segments_ = Segment::New(size, large_segments_);
 
+  Thread* current_thread = Thread::Current();
+  if (current_thread != NULL) {
+    current_thread->IncrementMemoryUsage(size);
+  }
+
   uword result = Utils::RoundUp(large_segments_->start(), kAlignment);
   return result;
 }
@@ -300,19 +326,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",