Skip to content

Commit 47df46c

Browse files
committed
[MERGE #5481 @leirocks] seperate lockdown and closing states
Merge pull request #5481 from leirocks:counters Compact counters on function body is locked when JIT started and then we should never change it afterward (unless redefer and unlock first) however when function body cleanup we do cleanup the counters, at that time we unlock and then lock the counters, in between this, there might be JIT thread accessing the counters hence we Assert that counters should be in locked state. seperating lockdown and closing state to avoid this racing condition
2 parents 99ea9f4 + c361394 commit 47df46c

File tree

3 files changed

+11
-8
lines changed

3 files changed

+11
-8
lines changed

lib/Runtime/Base/CompactCounters.h

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -32,8 +32,8 @@ namespace Js
3232
private:
3333
FieldWithBarrier(uint8) fieldSize;
3434
#if DBG
35-
36-
mutable FieldWithBarrier(bool) isLockedDown;
35+
mutable FieldWithBarrier(bool) isLockedDown:1;
36+
mutable FieldWithBarrier(bool) isClosing:1;
3737
#endif
3838
typename FieldWithBarrier(Fields*) fields;
3939

@@ -42,6 +42,7 @@ namespace Js
4242
:fieldSize(0)
4343
#if DBG
4444
, isLockedDown(false)
45+
, isClosing(false)
4546
#endif
4647
{
4748
AllocCounters<uint8>(host);

lib/Runtime/Base/FunctionBody.cpp

Lines changed: 7 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -233,19 +233,22 @@ namespace Js
233233
uint32 FunctionBody::GetCountField(FunctionBody::CounterFields fieldEnum) const
234234
{
235235
#if DBG
236-
Assert(ThreadContext::GetContextForCurrentThread() || counters.isLockedDown
236+
bool isCountersLockedDown = counters.isLockedDown;
237+
Assert(ThreadContext::GetContextForCurrentThread() || isCountersLockedDown
237238
|| (ThreadContext::GetCriticalSection()->IsLocked() && this->m_scriptContext->GetThreadContext()->GetFunctionBodyLock()->IsLocked())); // etw rundown
238239
#endif
239240
return counters.Get(fieldEnum);
240241
}
241242
uint32 FunctionBody::SetCountField(FunctionBody::CounterFields fieldEnum, uint32 val)
242243
{
243-
Assert(!counters.isLockedDown);
244+
DebugOnly(bool isCountersLockedDown = counters.isLockedDown);
245+
Assert(!isCountersLockedDown || counters.isClosing);
244246
return counters.Set(fieldEnum, val, this);
245247
}
246248
uint32 FunctionBody::IncreaseCountField(FunctionBody::CounterFields fieldEnum)
247249
{
248-
Assert(!counters.isLockedDown);
250+
DebugOnly(bool isCountersLockedDown = counters.isLockedDown);
251+
Assert(!isCountersLockedDown || counters.isClosing);
249252
return counters.Increase(fieldEnum, this);
250253
}
251254

@@ -7513,7 +7516,7 @@ namespace Js
75137516
return;
75147517
}
75157518

7516-
DebugOnly(this->UnlockCounters());
7519+
DebugOnly(this->SetIsClosing());
75177520

75187521
CleanupRecyclerData(isScriptContextClosing, false /* capture entry point cleanup stack trace */);
75197522
CleanUpForInCache(isScriptContextClosing);
@@ -7552,8 +7555,6 @@ namespace Js
75527555
#endif
75537556

75547557
this->cleanedUp = true;
7555-
7556-
DebugOnly(this->LockDownCounters());
75577558
}
75587559

75597560

lib/Runtime/Base/FunctionBody.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1891,6 +1891,7 @@ namespace Js
18911891
#if DBG
18921892
void LockDownCounters() { counters.isLockedDown = true; };
18931893
void UnlockCounters() { counters.isLockedDown = false; };
1894+
void SetIsClosing() { counters.isClosing = true; };
18941895
#endif
18951896

18961897
struct StatementMap

0 commit comments

Comments
 (0)