Index: Source/platform/heap/Heap.cpp |
diff --git a/Source/platform/heap/Heap.cpp b/Source/platform/heap/Heap.cpp |
index 430e1121cd22fee3d4651aea19f3667277737c7f..7cb6d50bef15522dade2d2563aaf52e62ae13857 100644 |
--- a/Source/platform/heap/Heap.cpp |
+++ b/Source/platform/heap/Heap.cpp |
@@ -109,11 +109,30 @@ static String classOf(const void* object) |
} |
#endif |
+class GCForbiddenScope { |
+public: |
+ explicit GCForbiddenScope(ThreadState* state) |
+ : m_state(state) |
+ { |
+ // Prevent nested collectGarbage() invocations. |
+ m_state->enterGCForbiddenScope(); |
+ } |
+ |
+ ~GCForbiddenScope() |
+ { |
+ m_state->leaveGCForbiddenScope(); |
+ } |
+ |
+private: |
+ ThreadState* m_state; |
+}; |
+ |
class GCScope { |
public: |
- explicit GCScope(ThreadState::StackState stackState) |
- : m_state(ThreadState::current()) |
- , m_safePointScope(stackState) |
+ GCScope(ThreadState* state, ThreadState::StackState stackState) |
+ : m_state(state) |
+ , m_gcForbiddenScope(state) |
+ , m_safePointScope(stackState, state) |
, m_parkedAllThreads(false) |
{ |
TRACE_EVENT0("blink_gc", "Heap::GCScope"); |
@@ -126,9 +145,9 @@ public: |
// FIXME: in an unlikely coincidence that two threads decide |
// to collect garbage at the same time, avoid doing two GCs in |
// a row. |
- if (LIKELY(ThreadState::stopThreads())) { |
+ if (LIKELY(ThreadState::stopThreads())) |
m_parkedAllThreads = true; |
- } |
+ |
if (m_state->isMainThread()) |
TRACE_EVENT_SET_NONCONST_SAMPLING_STATE(samplingState); |
} |
@@ -139,13 +158,13 @@ public: |
{ |
// Only cleanup if we parked all threads in which case the GC happened |
// and we need to resume the other threads. |
- if (LIKELY(m_parkedAllThreads)) { |
+ if (LIKELY(m_parkedAllThreads)) |
ThreadState::resumeThreads(); |
- } |
} |
private: |
ThreadState* m_state; |
+ GCForbiddenScope m_gcForbiddenScope; |
SafePointScope m_safePointScope; |
haraken
2015/05/28 15:00:56
Add a comment and mention that the order between G
sof
2015/05/28 15:08:50
Will add a comment re: ordering.
sof
2015/05/29 18:52:18
Done.
|
bool m_parkedAllThreads; // False if we fail to park all threads |
}; |
@@ -1872,20 +1891,14 @@ const char* Heap::gcReasonString(GCReason reason) |
void Heap::collectGarbage(ThreadState::StackState stackState, ThreadState::GCType gcType, GCReason reason) |
{ |
ThreadState* state = ThreadState::current(); |
- RELEASE_ASSERT(!state->isInGC()); |
+ RELEASE_ASSERT(!state->isGCForbidden()); |
haraken
2015/05/28 15:00:55
I'd move this check to GCForbiddenScope's construc
sof
2015/05/28 15:27:57
I don't understand the motivation behind the sugge
haraken
2015/05/28 15:28:46
ah, that makes sense!
sof
2015/05/29 18:52:18
Added a comment next to it.
|
state->completeSweep(); |
- ThreadState::GCState originalGCState = state->gcState(); |
- state->setGCState(ThreadState::StoppingOtherThreads); |
- GCScope gcScope(stackState); |
+ GCScope gcScope(state, stackState); |
// Check if we successfully parked the other threads. If not we bail out of |
// the GC. |
- if (!gcScope.allThreadsParked()) { |
- // Restore the original GCState. |
- if (LIKELY(state->gcState() == ThreadState::StoppingOtherThreads)) |
- state->setGCState(originalGCState); |
+ if (!gcScope.allThreadsParked()) |
return; |
- } |
if (state->isMainThread()) |
ScriptForbiddenScope::enter(); |