Skip to content

Commit

Permalink
Fix recursive issues for large Regions GC heap enumeration (#3333)
Browse files Browse the repository at this point in the history 
* Fix recursive issues for large Regions GC heap enumeration

* Ensure check interrupt in other calls of GC algorithms
  • Loading branch information
@hoyosjs
hoyosjs authored Aug 31, 2022
1 parent b5bca68 commit 896728d
Show file tree
Hide file tree
Showing 3 changed files with 61 additions and 28 deletions.
36 changes: 28 additions & 8 deletions src/SOS/Strike/sos.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -586,11 +586,13 @@ namespace sos
mCurrObj = mStart < TO_TADDR(mSegment.mem) ? TO_TADDR(mSegment.mem) : mStart;
mSegmentEnd = TO_TADDR(mSegment.highAllocMark);

CheckSegmentRange();
TryAlignToObjectInRange();
}

bool ObjectIterator::NextSegment()
bool ObjectIterator::TryMoveNextSegment()
{
CheckInterrupt();

if (mCurrHeap >= mNumHeaps)
{
return false;
Expand Down Expand Up @@ -648,16 +650,30 @@ namespace sos
mLastObj = 0;
mCurrObj = mStart < TO_TADDR(mSegment.mem) ? TO_TADDR(mSegment.mem) : mStart;
mSegmentEnd = TO_TADDR(mSegment.highAllocMark);
return CheckSegmentRange();
return true;
}

bool ObjectIterator::CheckSegmentRange()
bool ObjectIterator::TryMoveToObjectInNextSegmentInRange()
{
CheckInterrupt();
if (TryMoveNextSegment())
{
return TryAlignToObjectInRange();
}

return false;
}

bool ObjectIterator::TryAlignToObjectInRange()
{
CheckInterrupt();
while (!MemOverlap(mStart, mEnd, TO_TADDR(mSegment.mem), mSegmentEnd))
if (!NextSegment())
{
CheckInterrupt();
if (!TryMoveNextSegment())
{
return false;
}
}

// At this point we know that the current segment contains objects in
// the correct range. However, there's no telling if the user gave us
Expand Down Expand Up @@ -724,7 +740,7 @@ namespace sos
}
catch(const sos::Exception &)
{
NextSegment();
TryMoveToObjectInNextSegmentInRange();
}
}

Expand All @@ -742,6 +758,8 @@ namespace sos

void ObjectIterator::MoveToNextObject()
{
CheckInterrupt();

// Object::GetSize can be unaligned, so we must align it ourselves.
size_t size = (bLarge || bPinned) ? AlignLarge(mCurrObj.GetSize()) : Align(mCurrObj.GetSize());

Expand Down Expand Up @@ -773,7 +791,9 @@ namespace sos
}

if (mCurrObj > mEnd || mCurrObj >= mSegmentEnd)
NextSegment();
{
TryMoveToObjectInNextSegmentInRange();
}
}

SyncBlkIterator::SyncBlkIterator()
Expand Down
23 changes: 21 additions & 2 deletions src/SOS/Strike/sos.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -627,8 +627,27 @@ namespace sos
void BuildError(__out_ecount(count) char *out, size_t count, const char *format, ...) const;

void AssertSanity() const;
bool NextSegment();
bool CheckSegmentRange();

/*
This function moves to the next segment/region without checking any restrictions
on the range. Returns true if it was able to move to a new segment/region.
*/
bool TryMoveNextSegment();

/*
Aligns the iterator to the object that falls in the requested range, moving to
the next segment/region as necessary. The iterator state doesn't change if the
current object already lies in the requested range. Returns true if aligning
to such an object was possible.
*/
bool TryAlignToObjectInRange();

/*
Moves to the next segment/region that contains an object in the requested
range and align it to such object. This operation always moves the iterator.
Returns false if no such move was possible.
*/
bool TryMoveToObjectInNextSegmentInRange();
void MoveToNextObject();

private:
Expand Down
30 changes: 12 additions & 18 deletions src/SOS/Strike/strike.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -11045,6 +11045,7 @@ DECLARE_API(GCWhere)
}
}

DWORD heapIdx = 0;
if (!IsServerBuild())
{
DacpGcHeapDetails heapDetails;
Expand All @@ -11054,13 +11055,7 @@ DECLARE_API(GCWhere)
return Status;
}

if (GCObjInHeap(taddrObj, heapDetails, trngSeg, gen, allocCtx, bLarge))
{
ExtOut("Address " WIN64_8SPACES " Gen Heap segment " WIN64_8SPACES " begin " WIN64_8SPACES " allocated " WIN64_8SPACES " size\n");
ExtOut("%p %d %2d %p %p %p 0x%x(%d)\n",
SOS_PTR(taddrObj), gen, 0, SOS_PTR(trngSeg.segAddr), SOS_PTR(trngSeg.start), SOS_PTR(trngSeg.end), size, size);
bFound = TRUE;
}
bFound = GCObjInHeap(taddrObj, heapDetails, trngSeg, gen, allocCtx, bLarge);
}
else
{
Expand All @@ -11086,31 +11081,30 @@ DECLARE_API(GCWhere)
return Status;
}

for (DWORD n = 0; n < dwNHeaps; n ++)
for (heapIdx = 0; heapIdx < dwNHeaps && !bFound; heapIdx++)
{
DacpGcHeapDetails dacHeapDetails;
if (dacHeapDetails.Request(g_sos, heapAddrs[n]) != S_OK)
if (dacHeapDetails.Request(g_sos, heapAddrs[heapIdx]) != S_OK)
{
ExtOut("Error requesting details\n");
return Status;
}

GCHeapDetails heapDetails(dacHeapDetails, heapAddrs[n]);
if (GCObjInHeap(taddrObj, heapDetails, trngSeg, gen, allocCtx, bLarge))
{
ExtOut("Address " WIN64_8SPACES " Gen Heap segment " WIN64_8SPACES " begin " WIN64_8SPACES " allocated" WIN64_8SPACES " size\n");
ExtOut("%p %d %2d %p %p %p 0x%x(%d)\n",
SOS_PTR(taddrObj), gen, n, SOS_PTR(trngSeg.segAddr), SOS_PTR(trngSeg.start), SOS_PTR(trngSeg.end), size, size);
bFound = TRUE;
break;
}
GCHeapDetails heapDetails(dacHeapDetails, heapAddrs[heapIdx]);
bFound = GCObjInHeap(taddrObj, heapDetails, trngSeg, gen, allocCtx, bLarge);
}
}

if (!bFound)
{
ExtOut("Address %#p not found in the managed heap.\n", SOS_PTR(taddrObj));
}
else
{
ExtOut("Address " WIN64_8SPACES " Gen Heap segment " WIN64_8SPACES " begin " WIN64_8SPACES " allocated" WIN64_8SPACES " size\n");
ExtOut("%p %d %2d %p %p %p 0x%x(%d)\n",
SOS_PTR(taddrObj), gen, heapIdx, SOS_PTR(trngSeg.segAddr), SOS_PTR(trngSeg.start), SOS_PTR(trngSeg.end), size, size);
}

return Status;
}
Expand Down

0 comments on commit 896728d

Please sign in to comment.