Strengthen WebAssemblyArrayBuffer.GrowMemory

Cellule · Cellule · commit 5156c9be1e9f · 2017-08-02T15:30:55.000-04:00
OS#12948195
diff --git a/lib/Common/Memory/Recycler.cpp b/lib/Common/Memory/Recycler.cpp
@@ -1101,7 +1101,7 @@ Recycler::AddExternalMemoryUsage(size_t size)
     CollectNow<CollectOnAllocation>();
 }
 
-BOOL Recycler::ReportExternalMemoryAllocation(size_t size)
+bool Recycler::RequestExternalMemoryAllocation(size_t size)
 {
     return recyclerPageAllocator.RequestAlloc(size);
 }
diff --git a/lib/Common/Memory/Recycler.h b/lib/Common/Memory/Recycler.h
@@ -1202,9 +1202,12 @@ class Recycler
 
     template <CollectionFlags flags>
     bool FinishDisposeObjectsNow();
-    BOOL ReportExternalMemoryAllocation(size_t size);
+    bool RequestExternalMemoryAllocation(size_t size);
     void ReportExternalMemoryFailure(size_t size);
     void ReportExternalMemoryFree(size_t size);
+    // ExternalAllocFunc returns true when allocation succeeds
+    template <typename ExternalAllocFunc>
+    bool DoExternalAllocation(size_t size, ExternalAllocFunc externalAllocFunc);
 
 #ifdef TRACE_OBJECT_LIFETIME
 #define DEFINE_RECYCLER_ALLOC_TRACE(AllocFunc, AllocWithAttributesFunc, attributes) \
@@ -2525,3 +2528,35 @@ extern bool IsLikelyRuntimeFalseReference(
 #else
 #define DECLARE_RECYCLER_VERIFY_MARK_FRIEND()
 #endif
+
+template <typename ExternalAllocFunc>
+bool Recycler::DoExternalAllocation(size_t size, ExternalAllocFunc externalAllocFunc)
+{
+    // Request external memory allocation
+    if (!RequestExternalMemoryAllocation(size))
+    {
+        // Attempt to free some memory then try again
+        CollectNow<CollectOnTypedArrayAllocation>();
+        if (!RequestExternalMemoryAllocation(size))
+        {
+            return false;
+        }
+    }
+    struct AutoExternalAllocation
+    {
+        bool allocationSucceeded = false;
+        Recycler* recycler;
+        size_t size;
+        AutoExternalAllocation(Recycler* recycler, size_t size): recycler(recycler), size(size) {}
+        // In case the externalAllocFunc throws or fails, the destructor will report the failure
+        ~AutoExternalAllocation() { if (!allocationSucceeded) recycler->ReportExternalMemoryFailure(size); }
+    };
+    AutoExternalAllocation externalAllocation(this, size);
+    if (externalAllocFunc())
+    {
+        this->AddExternalMemoryUsage(size);
+        externalAllocation.allocationSucceeded = true;
+        return true;
+    }
+    return false;
+}
diff --git a/lib/Runtime/Library/ArrayBuffer.cpp b/lib/Runtime/Library/ArrayBuffer.cpp
@@ -208,12 +208,10 @@ namespace Js
         }
     }
 
-    ArrayBufferDetachedStateBase* ArrayBuffer::DetachAndGetState()
+    void ArrayBuffer::Detach()
     {
         Assert(!this->isDetached);
 
-        AutoPtr<ArrayBufferDetachedStateBase> arrayBufferState(this->CreateDetachedState(this->buffer, this->bufferLength));
-
         this->buffer = nullptr;
         this->bufferLength = 0;
         this->isDetached = true;
@@ -235,7 +233,13 @@ namespace Js
                 this->DetachBufferFromParent(item->Get());
             });
         }
+    }
 
+    ArrayBufferDetachedStateBase* ArrayBuffer::DetachAndGetState()
+    {
+        // Save the state before detaching
+        AutoPtr<ArrayBufferDetachedStateBase> arrayBufferState(this->CreateDetachedState(this->buffer, this->bufferLength));
+        Detach();
         return arrayBufferState.Detach();
     }
 
@@ -594,7 +598,7 @@ namespace Js
         else if (length > 0)
         {
             Recycler* recycler = GetType()->GetLibrary()->GetRecycler();
-            if (recycler->ReportExternalMemoryAllocation(length))
+            if (recycler->RequestExternalMemoryAllocation(length))
             {
                 buffer = (BYTE*)allocator(length);
                 if (buffer == nullptr)
@@ -607,7 +611,7 @@ namespace Js
             {
                 recycler->CollectNow<CollectOnTypedArrayAllocation>();
 
-                if (recycler->ReportExternalMemoryAllocation(length))
+                if (recycler->RequestExternalMemoryAllocation(length))
                 {
                     buffer = (BYTE*)allocator(length);
                     if (buffer == nullptr)
@@ -909,10 +913,10 @@ namespace Js
             // Expanding buffer
             if (newBufferLength > this->bufferLength)
             {
-                if (!recycler->ReportExternalMemoryAllocation(newBufferLength - this->bufferLength))
+                if (!recycler->RequestExternalMemoryAllocation(newBufferLength - this->bufferLength))
                 {
                     recycler->CollectNow<CollectOnTypedArrayAllocation>();
-                    if (!recycler->ReportExternalMemoryAllocation(newBufferLength - this->bufferLength))
+                    if (!recycler->RequestExternalMemoryAllocation(newBufferLength - this->bufferLength))
                     {
                         reportFailureFn();
                     }
@@ -1001,7 +1005,7 @@ namespace Js
     WebAssemblyArrayBuffer* WebAssemblyArrayBuffer::Create(byte* buffer, uint32 length, DynamicType * type)
     {
         Recycler* recycler = type->GetScriptContext()->GetRecycler();
-        WebAssemblyArrayBuffer* result;
+        WebAssemblyArrayBuffer* result = nullptr;
         if (buffer)
         {
             result = RecyclerNewFinalized(recycler, WebAssemblyArrayBuffer, buffer, length, type);
@@ -1018,9 +1022,10 @@ namespace Js
             {
                 result = RecyclerNewFinalized(recycler, WebAssemblyArrayBuffer, length, type);
             }
+            // Only add external memory when we create a new internal buffer
+            recycler->AddExternalMemoryUsage(length);
         }
         Assert(result);
-        recycler->AddExternalMemoryUsage(length);
         return result;
     }
 
@@ -1042,68 +1047,85 @@ namespace Js
         }
         uint32 growSize = newBufferLength - this->bufferLength;
 
-        bool failedReport = false;
-        const auto reportFailedFn = [&failedReport] { failedReport = true; };
+        struct AutoFailFastOnBadState
+        {
+            bool isStateValid = true;
+            ~AutoFailFastOnBadState()
+            {
+                if (!isStateValid)
+                {
+                    // We ended up in an unrecoverable state, fail fast
+                    Js::Throw::FatalInternalError();
+                }
+            }
+        };
+        AutoFailFastOnBadState autoFailFastOnBadState;
 
         WebAssemblyArrayBuffer* newArrayBuffer = nullptr;
 #if ENABLE_FAST_ARRAYBUFFER
         if (CONFIG_FLAG(WasmFastArray))
         {
             AssertOrFailFast(this->buffer);
-            ReportDifferentialAllocation(newBufferLength, reportFailedFn);
-            if (failedReport)
-            {
-                return nullptr;
-            }
-
             if (growSize > 0)
             {
-                LPVOID newMem = VirtualAlloc(this->buffer + this->bufferLength, growSize, MEM_COMMIT, PAGE_READWRITE);
-                if (!newMem)
+                const auto virtualAllocFunc = [&]
+                {
+                    return !!VirtualAlloc(this->buffer + this->bufferLength, growSize, MEM_COMMIT, PAGE_READWRITE);
+                };
+                if (!this->GetRecycler()->DoExternalAllocation(growSize, virtualAllocFunc))
                 {
-                    Recycler* recycler = this->GetRecycler();
-                    recycler->ReportExternalMemoryFailure(newBufferLength);
                     return nullptr;
                 }
             }
-            newArrayBuffer = GetLibrary()->CreateWebAssemblyArrayBuffer(this->buffer, newBufferLength);
+            newArrayBuffer = this->GetLibrary()->CreateWebAssemblyArrayBuffer(this->buffer, newBufferLength);
         }
         else
 #endif
         if (this->GetByteLength() == 0)
         {
             if (growSize > 0)
             {
-                newArrayBuffer = GetLibrary()->CreateWebAssemblyArrayBuffer(newBufferLength);
+                // Creating a new buffer will do the external memory allocation report
+                newArrayBuffer = this->GetLibrary()->CreateWebAssemblyArrayBuffer(newBufferLength);
             }
             else
             {
-                newArrayBuffer = GetLibrary()->CreateWebAssemblyArrayBuffer(this->buffer, 0);
+                newArrayBuffer = this->GetLibrary()->CreateWebAssemblyArrayBuffer(this->buffer, 0);
             }
         }
         else
         {
-            ReportDifferentialAllocation(newBufferLength, reportFailedFn);
-            if (failedReport)
+            byte* newBuffer = nullptr;
+            const auto reallocFunc = [&]
             {
-                return nullptr;
-            }
-            byte* newBuffer = ReallocZero(this->buffer, this->bufferLength, newBufferLength);
-            if (!newBuffer)
+                newBuffer = ReallocZero(this->buffer, this->bufferLength, newBufferLength);
+                // Realloc freed this->buffer if newBuffer is not nullptr
+                // if anything goes wrong before we detach, we can't recover the state and should failfast
+                autoFailFastOnBadState.isStateValid = newBuffer == nullptr;
+                return !!newBuffer;
+            };
+
+            if (!this->GetRecycler()->DoExternalAllocation(growSize, reallocFunc))
             {
-                this->GetRecycler()->ReportExternalMemoryFailure(newBufferLength - this->bufferLength);
                 return nullptr;
             }
-            newArrayBuffer = GetLibrary()->CreateWebAssemblyArrayBuffer(newBuffer, newBufferLength);
+
+            this->buffer = nullptr;
+            newArrayBuffer = this->GetLibrary()->CreateWebAssemblyArrayBuffer(newBuffer, newBufferLength);
         }
 
         if (!newArrayBuffer || newArrayBuffer->GetByteLength() != newBufferLength)
         {
             return nullptr;
         }
 
-        AutoDiscardPTR<Js::ArrayBufferDetachedStateBase> state(DetachAndGetState());
-        state->MarkAsClaimed();
+        // Detach the buffer from this ArrayBuffer
+        this->Detach();
+
+        // The buffer has been freed or passed down to the new ArrayBuffer.
+        Assert(this->buffer == nullptr);
+        autoFailFastOnBadState.isStateValid = true;
+
         return newArrayBuffer;
     }
 
diff --git a/lib/Runtime/Library/ArrayBuffer.h b/lib/Runtime/Library/ArrayBuffer.h
@@ -159,7 +159,8 @@ namespace Js
         virtual BOOL GetDiagTypeString(StringBuilder<ArenaAllocator>* stringBuilder, ScriptContext* requestContext) override;
         virtual BOOL GetDiagValueString(StringBuilder<ArenaAllocator>* stringBuilder, ScriptContext* requestContext) override;
 
-        virtual ArrayBufferDetachedStateBase* DetachAndGetState();
+        void Detach();
+        ArrayBufferDetachedStateBase* DetachAndGetState();
         virtual uint32 GetByteLength() const override { return bufferLength; }
         virtual BYTE* GetBuffer() const override { return buffer; }
 
diff --git a/lib/Runtime/Library/SharedArrayBuffer.cpp b/lib/Runtime/Library/SharedArrayBuffer.cpp
@@ -270,7 +270,7 @@ namespace Js
             }
         };
 
-        if (recycler->ReportExternalMemoryAllocation(length))
+        if (recycler->RequestExternalMemoryAllocation(length))
         {
             buffer = alloc(length);
             if (buffer == nullptr)

Original file line number	Diff line number	Diff line change
`@@ -1101,7 +1101,7 @@ Recycler::AddExternalMemoryUsage(size_t size)`
`1101`	`1101`	`CollectNow<CollectOnAllocation>();`
`1102`	`1102`	`}`
`1103`	`1103`
`1104`		`-BOOL Recycler::ReportExternalMemoryAllocation(size_t size)`
	`1104`	`+bool Recycler::RequestExternalMemoryAllocation(size_t size)`
`1105`	`1105`	`{`
`1106`	`1106`	`return recyclerPageAllocator.RequestAlloc(size);`
`1107`	`1107`	`}`
Original file line number	Diff line number	Diff line change
`@@ -208,12 +208,10 @@ namespace Js`
`208`	`208`	`}`
`209`	`209`	`}`
`210`	`210`
`211`		`- ArrayBufferDetachedStateBase* ArrayBuffer::DetachAndGetState()`
	`211`	`+ void ArrayBuffer::Detach()`
`212`	`212`	`{`
`213`	`213`	`Assert(!this->isDetached);`
`214`	`214`
`215`		`- AutoPtr<ArrayBufferDetachedStateBase> arrayBufferState(this->CreateDetachedState(this->buffer, this->bufferLength));`
`216`		`-`
`217`	`215`	`this->buffer = nullptr;`
`218`	`216`	`this->bufferLength = 0;`
`219`	`217`	`this->isDetached = true;`
`@@ -235,7 +233,13 @@ namespace Js`
`235`	`233`	`this->DetachBufferFromParent(item->Get());`
`236`	`234`	`});`
`237`	`235`	`}`
	`236`	`+ }`
`238`	`237`
	`238`	`+ ArrayBufferDetachedStateBase* ArrayBuffer::DetachAndGetState()`
	`239`	`+ {`
	`240`	`+ // Save the state before detaching`
	`241`	`+ AutoPtr<ArrayBufferDetachedStateBase> arrayBufferState(this->CreateDetachedState(this->buffer, this->bufferLength));`
	`242`	`+ Detach();`
`239`	`243`	`return arrayBufferState.Detach();`
`240`	`244`	`}`
`241`	`245`
`@@ -594,7 +598,7 @@ namespace Js`
`594`	`598`	`else if (length > 0)`
`595`	`599`	`{`
`596`	`600`	`Recycler* recycler = GetType()->GetLibrary()->GetRecycler();`
`597`		`- if (recycler->ReportExternalMemoryAllocation(length))`
	`601`	`+ if (recycler->RequestExternalMemoryAllocation(length))`
`598`	`602`	`{`
`599`	`603`	`buffer = (BYTE*)allocator(length);`
`600`	`604`	`if (buffer == nullptr)`
`@@ -607,7 +611,7 @@ namespace Js`
`607`	`611`	`{`
`608`	`612`	`recycler->CollectNow<CollectOnTypedArrayAllocation>();`
`609`	`613`
`610`		`- if (recycler->ReportExternalMemoryAllocation(length))`
	`614`	`+ if (recycler->RequestExternalMemoryAllocation(length))`
`611`	`615`	`{`
`612`	`616`	`buffer = (BYTE*)allocator(length);`
`613`	`617`	`if (buffer == nullptr)`
`@@ -909,10 +913,10 @@ namespace Js`
`909`	`913`	`// Expanding buffer`
`910`	`914`	`if (newBufferLength > this->bufferLength)`
`911`	`915`	`{`
`912`		`- if (!recycler->ReportExternalMemoryAllocation(newBufferLength - this->bufferLength))`
	`916`	`+ if (!recycler->RequestExternalMemoryAllocation(newBufferLength - this->bufferLength))`
`913`	`917`	`{`
`914`	`918`	`recycler->CollectNow<CollectOnTypedArrayAllocation>();`
`915`		`- if (!recycler->ReportExternalMemoryAllocation(newBufferLength - this->bufferLength))`
	`919`	`+ if (!recycler->RequestExternalMemoryAllocation(newBufferLength - this->bufferLength))`
`916`	`920`	`{`
`917`	`921`	`reportFailureFn();`
`918`	`922`	`}`
`@@ -1001,7 +1005,7 @@ namespace Js`
`1001`	`1005`	`WebAssemblyArrayBuffer* WebAssemblyArrayBuffer::Create(byte* buffer, uint32 length, DynamicType * type)`
`1002`	`1006`	`{`
`1003`	`1007`	`Recycler* recycler = type->GetScriptContext()->GetRecycler();`
`1004`		`- WebAssemblyArrayBuffer* result;`
	`1008`	`+ WebAssemblyArrayBuffer* result = nullptr;`
`1005`	`1009`	`if (buffer)`
`1006`	`1010`	`{`
`1007`	`1011`	`result = RecyclerNewFinalized(recycler, WebAssemblyArrayBuffer, buffer, length, type);`
`@@ -1018,9 +1022,10 @@ namespace Js`
`1018`	`1022`	`{`
`1019`	`1023`	`result = RecyclerNewFinalized(recycler, WebAssemblyArrayBuffer, length, type);`
`1020`	`1024`	`}`
	`1025`	`+ // Only add external memory when we create a new internal buffer`
	`1026`	`+ recycler->AddExternalMemoryUsage(length);`
`1021`	`1027`	`}`
`1022`	`1028`	`Assert(result);`
`1023`		`- recycler->AddExternalMemoryUsage(length);`
`1024`	`1029`	`return result;`
`1025`	`1030`	`}`
`1026`	`1031`
`@@ -1042,68 +1047,85 @@ namespace Js`
`1042`	`1047`	`}`
`1043`	`1048`	`uint32 growSize = newBufferLength - this->bufferLength;`
`1044`	`1049`
`1045`		`- bool failedReport = false;`
`1046`		`- const auto reportFailedFn = [&failedReport] { failedReport = true; };`
	`1050`	`+ struct AutoFailFastOnBadState`
	`1051`	`+ {`
	`1052`	`+ bool isStateValid = true;`
	`1053`	`+ ~AutoFailFastOnBadState()`
	`1054`	`+ {`
	`1055`	`+ if (!isStateValid)`
	`1056`	`+ {`
	`1057`	`+ // We ended up in an unrecoverable state, fail fast`
	`1058`	`+ Js::Throw::FatalInternalError();`
	`1059`	`+ }`
	`1060`	`+ }`
	`1061`	`+ };`
	`1062`	`+ AutoFailFastOnBadState autoFailFastOnBadState;`
`1047`	`1063`
`1048`	`1064`	`WebAssemblyArrayBuffer* newArrayBuffer = nullptr;`
`1049`	`1065`	`#if ENABLE_FAST_ARRAYBUFFER`
`1050`	`1066`	`if (CONFIG_FLAG(WasmFastArray))`
`1051`	`1067`	`{`
`1052`	`1068`	`AssertOrFailFast(this->buffer);`
`1053`		`- ReportDifferentialAllocation(newBufferLength, reportFailedFn);`
`1054`		`- if (failedReport)`
`1055`		`- {`
`1056`		`- return nullptr;`
`1057`		`- }`
`1058`		`-`
`1059`	`1069`	`if (growSize > 0)`
`1060`	`1070`	`{`
`1061`		`- LPVOID newMem = VirtualAlloc(this->buffer + this->bufferLength, growSize, MEM_COMMIT, PAGE_READWRITE);`
`1062`		`- if (!newMem)`
	`1071`	`+ const auto virtualAllocFunc = [&]`
	`1072`	`+ {`
	`1073`	`+ return !!VirtualAlloc(this->buffer + this->bufferLength, growSize, MEM_COMMIT, PAGE_READWRITE);`
	`1074`	`+ };`
	`1075`	`+ if (!this->GetRecycler()->DoExternalAllocation(growSize, virtualAllocFunc))`
`1063`	`1076`	`{`
`1064`		`- Recycler* recycler = this->GetRecycler();`
`1065`		`- recycler->ReportExternalMemoryFailure(newBufferLength);`
`1066`	`1077`	`return nullptr;`
`1067`	`1078`	`}`
`1068`	`1079`	`}`
`1069`		`- newArrayBuffer = GetLibrary()->CreateWebAssemblyArrayBuffer(this->buffer, newBufferLength);`
	`1080`	`+ newArrayBuffer = this->GetLibrary()->CreateWebAssemblyArrayBuffer(this->buffer, newBufferLength);`
`1070`	`1081`	`}`
`1071`	`1082`	`else`
`1072`	`1083`	`#endif`
`1073`	`1084`	`if (this->GetByteLength() == 0)`
`1074`	`1085`	`{`
`1075`	`1086`	`if (growSize > 0)`
`1076`	`1087`	`{`
`1077`		`- newArrayBuffer = GetLibrary()->CreateWebAssemblyArrayBuffer(newBufferLength);`
	`1088`	`+ // Creating a new buffer will do the external memory allocation report`
	`1089`	`+ newArrayBuffer = this->GetLibrary()->CreateWebAssemblyArrayBuffer(newBufferLength);`
`1078`	`1090`	`}`
`1079`	`1091`	`else`
`1080`	`1092`	`{`
`1081`		`- newArrayBuffer = GetLibrary()->CreateWebAssemblyArrayBuffer(this->buffer, 0);`
	`1093`	`+ newArrayBuffer = this->GetLibrary()->CreateWebAssemblyArrayBuffer(this->buffer, 0);`
`1082`	`1094`	`}`
`1083`	`1095`	`}`
`1084`	`1096`	`else`
`1085`	`1097`	`{`
`1086`		`- ReportDifferentialAllocation(newBufferLength, reportFailedFn);`
`1087`		`- if (failedReport)`
	`1098`	`+ byte* newBuffer = nullptr;`
	`1099`	`+ const auto reallocFunc = [&]`
`1088`	`1100`	`{`
`1089`		`- return nullptr;`
`1090`		`- }`
`1091`		`- byte* newBuffer = ReallocZero(this->buffer, this->bufferLength, newBufferLength);`
`1092`		`- if (!newBuffer)`
	`1101`	`+ newBuffer = ReallocZero(this->buffer, this->bufferLength, newBufferLength);`
	`1102`	`+ // Realloc freed this->buffer if newBuffer is not nullptr`
	`1103`	`+ // if anything goes wrong before we detach, we can't recover the state and should failfast`
	`1104`	`+ autoFailFastOnBadState.isStateValid = newBuffer == nullptr;`
	`1105`	`+ return !!newBuffer;`
	`1106`	`+ };`
	`1107`	`+`
	`1108`	`+ if (!this->GetRecycler()->DoExternalAllocation(growSize, reallocFunc))`
`1093`	`1109`	`{`
`1094`		`- this->GetRecycler()->ReportExternalMemoryFailure(newBufferLength - this->bufferLength);`
`1095`	`1110`	`return nullptr;`
`1096`	`1111`	`}`
`1097`		`- newArrayBuffer = GetLibrary()->CreateWebAssemblyArrayBuffer(newBuffer, newBufferLength);`
	`1112`	`+`
	`1113`	`+ this->buffer = nullptr;`
	`1114`	`+ newArrayBuffer = this->GetLibrary()->CreateWebAssemblyArrayBuffer(newBuffer, newBufferLength);`
`1098`	`1115`	`}`
`1099`	`1116`
`1100`	`1117`	`if (!newArrayBuffer \|\| newArrayBuffer->GetByteLength() != newBufferLength)`
`1101`	`1118`	`{`
`1102`	`1119`	`return nullptr;`
`1103`	`1120`	`}`
`1104`	`1121`
`1105`		`- AutoDiscardPTR<Js::ArrayBufferDetachedStateBase> state(DetachAndGetState());`
`1106`		`- state->MarkAsClaimed();`
	`1122`	`+ // Detach the buffer from this ArrayBuffer`
	`1123`	`+ this->Detach();`
	`1124`	`+`
	`1125`	`+ // The buffer has been freed or passed down to the new ArrayBuffer.`
	`1126`	`+ Assert(this->buffer == nullptr);`
	`1127`	`+ autoFailFastOnBadState.isStateValid = true;`
	`1128`	`+`
`1107`	`1129`	`return newArrayBuffer;`
`1108`	`1130`	`}`
`1109`	`1131`
Original file line number	Diff line number	Diff line change
`@@ -270,7 +270,7 @@ namespace Js`
`270`	`270`	`}`
`271`	`271`	`};`
`272`	`272`
`273`		`- if (recycler->ReportExternalMemoryAllocation(length))`
	`273`	`+ if (recycler->RequestExternalMemoryAllocation(length))`
`274`	`274`	`{`
`275`	`275`	`buffer = alloc(length);`
`276`	`276`	`if (buffer == nullptr)`