Skip to content

Commit 38ffa70

Browse files
committed
PR feedback: we should not specialcase projection buffers on Detach.
1 parent 0015707 commit 38ffa70

File tree

3 files changed

+30
-14
lines changed

3 files changed

+30
-14
lines changed

lib/Common/Memory/Recycler.h

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2648,7 +2648,6 @@ bool Recycler::DoExternalAllocation(size_t size, ExternalAllocFunc externalAlloc
26482648
AutoExternalAllocation externalAllocation(this, size);
26492649
if (externalAllocFunc())
26502650
{
2651-
this->AddExternalMemoryUsage(size);
26522651
externalAllocation.allocationSucceeded = true;
26532652
return true;
26542653
}

lib/Runtime/Library/ArrayBuffer.cpp

Lines changed: 28 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -218,11 +218,10 @@ namespace Js
218218
{
219219
Assert(!this->isDetached);
220220

221-
if (this->IsProjectionArrayBuffer())
222-
{
223-
Recycler* recycler = GetType()->GetLibrary()->GetRecycler();
224-
recycler->ReportExternalMemoryFree(bufferLength);
225-
}
221+
// we are about to lose track of the buffer to another owner
222+
// report that we no longer own the memory
223+
Recycler* recycler = GetType()->GetLibrary()->GetRecycler();
224+
recycler->ReportExternalMemoryFree(bufferLength);
226225

227226
this->buffer = nullptr;
228227
this->bufferLength = 0;
@@ -702,6 +701,13 @@ namespace Js
702701
Recycler* recycler = type->GetScriptContext()->GetRecycler();
703702
JavascriptArrayBuffer* result = RecyclerNewFinalized(recycler, JavascriptArrayBuffer, buffer, length, type);
704703
Assert(result);
704+
705+
// we take the ownership of the buffer and will have to free it so charge it to our quota.
706+
if (!recycler->RequestExternalMemoryAllocation(length))
707+
{
708+
JavascriptError::ThrowOutOfMemoryError(result->GetScriptContext());
709+
}
710+
705711
recycler->AddExternalMemoryUsage(length);
706712
return result;
707713
}
@@ -713,12 +719,6 @@ namespace Js
713719
{
714720
freeFunction(this->buffer);
715721
this->buffer = nullptr;
716-
717-
// for projection array buffers we have already reported freeing on detach
718-
if (this->allocationType != ArrayBufferAllocationType::CoTask)
719-
{
720-
this->recycler->ReportExternalMemoryFree(this->bufferLength);
721-
}
722722
}
723723
this->bufferLength = 0;
724724
}
@@ -890,6 +890,12 @@ namespace Js
890890
if (buffer)
891891
{
892892
result = RecyclerNewFinalized(recycler, WebAssemblyArrayBuffer, buffer, length, type);
893+
894+
// we take the ownership of the buffer and will have to free it so charge it to our quota.
895+
if (!recycler->RequestExternalMemoryAllocation(length))
896+
{
897+
JavascriptError::ThrowOutOfMemoryError(result->GetScriptContext());
898+
}
893899
}
894900
else
895901
{
@@ -903,9 +909,9 @@ namespace Js
903909
{
904910
result = RecyclerNewFinalized(recycler, WebAssemblyArrayBuffer, length, type);
905911
}
906-
// Only add external memory when we create a new internal buffer
907-
recycler->AddExternalMemoryUsage(length);
908912
}
913+
914+
recycler->AddExternalMemoryUsage(length);
909915
Assert(result);
910916
return result;
911917
}
@@ -960,6 +966,11 @@ namespace Js
960966
{
961967
return nullptr;
962968
}
969+
970+
// We are transferring the buffer to the new owner.
971+
// To avoid double-charge to the allocation quota we will free the "diff" amount here.
972+
this->GetRecycler()->ReportExternalMemoryFree(growSize);
973+
963974
return finalizeGrowMemory(this->GetLibrary()->CreateWebAssemblyArrayBuffer(this->buffer, newBufferLength));
964975
}
965976
#endif
@@ -995,6 +1006,10 @@ namespace Js
9951006
return nullptr;
9961007
}
9971008

1009+
// We are transferring the buffer to the new owner.
1010+
// To avoid double-charge to the allocation quota we will free the "diff" amount here.
1011+
this->GetRecycler()->ReportExternalMemoryFree(growSize);
1012+
9981013
WebAssemblyArrayBuffer* newArrayBuffer = finalizeGrowMemory(this->GetLibrary()->CreateWebAssemblyArrayBuffer(newBuffer, newBufferLength));
9991014
// We've successfully Detached this buffer and created a new WebAssemblyArrayBuffer
10001015
autoDisableInterrupt.Completed();

lib/Runtime/Library/SharedArrayBuffer.cpp

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -617,6 +617,8 @@ namespace Js
617617
{
618618
return false;
619619
}
620+
621+
this->GetRecycler()->AddExternalMemoryUsage(growSize);
620622
}
621623
else
622624
{

0 commit comments

Comments
 (0)