Skip to content

Commit e3f9a51

Browse files
leirocksleirocks
committed
Fix assertion for background access on function body counter fields
- in case of -prejit -forceserialized combination, we assumed since background jit started, these counters should not be modified any more. however with prejit and forceserialized, it updates serializationId and hit the assertion. Looks we don't use the serializeID and functionTable fields at all. just remove them - When Etw rundown in background thread, the foreground thread can be updating the counters and can lead to expansion. we need a lock for the expansion.
1 parent 3d68d30 commit e3f9a51

11 files changed

Lines changed: 9859 additions & 9984 deletions

File tree

‎lib/Common/Core/CriticalSection.h‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@ class CriticalSection
1818
void Leave() { ::LeaveCriticalSection(&cs); }
1919
#if DBG
2020
bool IsLocked() const { return cs.OwningThread == (HANDLE)::GetCurrentThreadId(); }
21+
bool IsLockedByAnyThread() const { return (InterlockedExchangeAdd((volatile LONG*)&cs.LockCount, 0L) & 1/*CS_LOCK_BIT*/) == 0; }
2122
#endif
2223
private:
2324
CRITICAL_SECTION cs;

‎lib/Jsrt/Jsrt.cpp‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2562,7 +2562,7 @@ JsErrorCode JsSerializeScriptCore(const wchar_t *script, BYTE *functionTable, in
25622562
#endif
25632563

25642564
BEGIN_TEMP_ALLOCATOR(tempAllocator, scriptContext, L"ByteCodeSerializer");
2565-
HRESULT hr = Js::ByteCodeSerializer::SerializeToBuffer(scriptContext, tempAllocator, static_cast<DWORD>(cSourceCodeLength), utf8Code, 0, nullptr, functionBody, functionBody->GetHostSrcInfo(), false, &buffer, bufferSize, dwFlags);
2565+
HRESULT hr = Js::ByteCodeSerializer::SerializeToBuffer(scriptContext, tempAllocator, static_cast<DWORD>(cSourceCodeLength), utf8Code, functionBody, functionBody->GetHostSrcInfo(), false, &buffer, bufferSize, dwFlags);
25662566
END_TEMP_ALLOCATOR(tempAllocator, scriptContext);
25672567

25682568
if (SUCCEEDED(hr))

‎lib/Runtime/Base/CompactCounters.h‎

Lines changed: 16 additions & 90 deletions
Original file line numberDiff line numberDiff line change
@@ -73,40 +73,10 @@ namespace Js
7373
return value;
7474
}
7575

76-
int32 GetSigned(CountT typeEnum) const
77-
{
78-
#if DBG
79-
if (!bgThreadCallStarted && ThreadContext::GetContextForCurrentThread() == nullptr)
80-
{
81-
bgThreadCallStarted = true;
82-
}
83-
#endif
84-
85-
uint8 type = static_cast<uint8>(typeEnum);
86-
uint8 localFieldSize = fieldSize;
87-
int32 value = 0;
88-
if (localFieldSize == 1)
89-
{
90-
value = this->fields->i8Fields[type];
91-
}
92-
else if (localFieldSize == 2)
93-
{
94-
value = this->fields->i16Fields[type];
95-
}
96-
else if (localFieldSize == 4)
97-
{
98-
value = this->fields->i32Fields[type];
99-
}
100-
else
101-
{
102-
Assert(localFieldSize == 0 && this->isCleaningUp && this->fields == nullptr); // OOM when initial allocation failed
103-
}
104-
return value;
105-
}
106-
10776
uint32 Set(CountT typeEnum, uint32 val, T* host)
10877
{
109-
Assert(bgThreadCallStarted == false || isCleaningUp == true);
78+
Assert(bgThreadCallStarted == false || isCleaningUp == true
79+
|| host->GetScriptContext()->GetThreadContext()->GetEtwRundownCriticalSection()->IsLockedByAnyThread());
11080

11181
uint8 type = static_cast<uint8>(typeEnum);
11282
if (fieldSize == 1)
@@ -144,46 +114,6 @@ namespace Js
144114
return val;
145115
}
146116

147-
int32 SetSigned(CountT typeEnum, int32 val, T* host)
148-
{
149-
Assert(bgThreadCallStarted == false || isCleaningUp == true);
150-
151-
uint8 type = static_cast<uint8>(typeEnum);
152-
if (fieldSize == 1)
153-
{
154-
if (val <= INT8_MAX && val >= INT8_MIN)
155-
{
156-
return this->fields->i8Fields[type] = static_cast<uint8>(val);
157-
}
158-
else
159-
{
160-
(val <= INT16_MAX && val >= INT16_MIN)? AllocCounters<uint16>(host): AllocCounters<uint32>(host);
161-
return host->counters.SetSigned(typeEnum, val, host);
162-
}
163-
}
164-
165-
if (fieldSize == 2)
166-
{
167-
if (val <= INT16_MAX && val >= INT16_MIN)
168-
{
169-
return this->fields->i16Fields[type] = static_cast<uint16>(val);
170-
}
171-
else
172-
{
173-
AllocCounters<uint32>(host);
174-
return host->counters.SetSigned(typeEnum, val, host);
175-
}
176-
}
177-
178-
if (fieldSize == 4)
179-
{
180-
return this->fields->i32Fields[type] = val;
181-
}
182-
183-
Assert(fieldSize == 0 && this->isCleaningUp && this->fields == nullptr && val == 0); // OOM when allocating the counters structure
184-
return val;
185-
}
186-
187117
uint32 Increase(CountT typeEnum, T* host)
188118
{
189119
Assert(bgThreadCallStarted == false);
@@ -225,7 +155,6 @@ namespace Js
225155
Assert(ThreadContext::GetContextForCurrentThread() || ThreadContext::GetCriticalSection()->IsLocked());
226156
Assert(host->GetRecycler() != nullptr);
227157

228-
const uint8 signedStart = static_cast<uint8>(CountT::SignedFieldsStart);
229158
const uint8 max = static_cast<uint8>(CountT::Max);
230159
typedef CompactCounters<T, CountT> CounterT;
231160
CounterT::Fields* fieldsArray = (CounterT::Fields*)RecyclerNewArrayLeafZ(host->GetRecycler(), FieldT, sizeof(FieldT)*max);
@@ -235,45 +164,42 @@ namespace Js
235164
{
236165
if (sizeof(FieldT) == 2)
237166
{
238-
for (; i < signedStart; i++)
239-
{
240-
fieldsArray->u16Fields[i] = oldFieldsArray->u8Fields[i];
241-
}
242167
for (; i < max; i++)
243168
{
244-
fieldsArray->i16Fields[i] = oldFieldsArray->i8Fields[i];
169+
fieldsArray->u16Fields[i] = oldFieldsArray->u8Fields[i];
245170
}
246171
}
247172
else if (sizeof(FieldT) == 4)
248173
{
249-
for (; i < signedStart; i++)
250-
{
251-
fieldsArray->u32Fields[i] = oldFieldsArray->u8Fields[i];
252-
}
253174
for (; i < max; i++)
254175
{
255-
fieldsArray->i32Fields[i] = oldFieldsArray->i8Fields[i];
176+
fieldsArray->u32Fields[i] = oldFieldsArray->u8Fields[i];
256177
}
257178
}
258179
}
259180
else if (this->fieldSize == 2)
260181
{
261-
for (; i < signedStart; i++)
262-
{
263-
fieldsArray->u32Fields[i] = oldFieldsArray->u16Fields[i];
264-
}
265182
for (; i < max; i++)
266183
{
267-
fieldsArray->i32Fields[i] = oldFieldsArray->i16Fields[i];
184+
fieldsArray->u32Fields[i] = oldFieldsArray->u16Fields[i];
268185
}
269186
}
270187
else
271188
{
272189
Assert(this->fieldSize==0);
273190
}
274191

275-
this->fieldSize = sizeof(FieldT);
276-
this->fields = fieldsArray;
192+
if (this->fieldSize == 0)
193+
{
194+
this->fieldSize = sizeof(FieldT);
195+
this->fields = fieldsArray;
196+
}
197+
else
198+
{
199+
AutoCriticalSection autoCS(host->GetScriptContext()->GetThreadContext()->GetEtwRundownCriticalSection());
200+
this->fieldSize = sizeof(FieldT);
201+
this->fields = fieldsArray;
202+
}
277203
}
278204

279205
};

‎lib/Runtime/Base/FunctionBody.cpp‎

Lines changed: 3 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -509,6 +509,9 @@ namespace Js
509509
, m_isFromNativeCodeModule(false)
510510
, hasHotLoop(false)
511511
, m_isPartialDeserializedFunction(false)
512+
#if DBG
513+
, m_isSerialized(false)
514+
#endif
512515
#ifdef PERF_COUNTERS
513516
, m_isDeserializedFunction(isDeserializedFunction)
514517
#endif
@@ -527,7 +530,6 @@ namespace Js
527530
#endif
528531
{
529532
SetCountField(CounterFields::ConstantCount, 1);
530-
SetCountFieldSigned(CounterFields::SerializationIndex, -1);
531533

532534
this->SetDefaultFunctionEntryPointInfo((FunctionEntryPointInfo*) this->GetDefaultEntryPointInfo(), DefaultEntryThunk);
533535
this->m_hasBeenParsed = true;
@@ -582,16 +584,6 @@ namespace Js
582584
}
583585
}
584586

585-
const int
586-
FunctionBody::GetSerializationIndex() const
587-
{
588-
return GetCountFieldSigned(CounterFields::SerializationIndex);
589-
}
590-
void FunctionBody::SetSerializationIndex(int index)
591-
{
592-
SetCountFieldSigned(CounterFields::SerializationIndex, index);
593-
}
594-
595587
const char16* ParseableFunctionInfo::GetExternalDisplayName() const
596588
{
597589
return GetExternalDisplayName(this);
@@ -3551,7 +3543,6 @@ namespace Js
35513543
}
35523544
}
35533545

3554-
newFunctionBody->SetSerializationIndex(this->GetSerializationIndex());
35553546
newFunctionBody->m_isFromNativeCodeModule = this->m_isFromNativeCodeModule;
35563547
}
35573548

‎lib/Runtime/Base/FunctionBody.h‎

Lines changed: 8 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -1595,7 +1595,7 @@ namespace Js
15951595
bool m_isAsmjsMode : 1;
15961596
bool m_isAsmJsFunction : 1;
15971597
bool m_isGlobalFunc : 1;
1598-
bool m_doBackendArgumentsOptimization :1;
1598+
bool m_doBackendArgumentsOptimization : 1;
15991599
bool m_isEval : 1; // Source code is in 'eval'
16001600
bool m_isDynamicFunction : 1; // Source code is in 'Function'
16011601
bool m_hasImplicitArgIns : 1;
@@ -1711,10 +1711,6 @@ namespace Js
17111711
FuncExprScopeRegister = 22,
17121712
FirstTmpRegister = 23,
17131713

1714-
// Signed integers need keep the sign when promoting
1715-
SignedFieldsStart = 24,
1716-
SerializationIndex = 24,
1717-
17181714
Max
17191715
};
17201716

@@ -1733,14 +1729,6 @@ namespace Js
17331729
{
17341730
return counters.Increase(fieldEnum, this);
17351731
}
1736-
int32 GetCountFieldSigned(FunctionBody::CounterFields fieldEnum) const
1737-
{
1738-
return counters.GetSigned(fieldEnum);
1739-
}
1740-
int32 SetCountFieldSigned(FunctionBody::CounterFields fieldEnum, int32 val)
1741-
{
1742-
return counters.SetSigned(fieldEnum, val, this);
1743-
}
17441732

17451733
struct StatementMap
17461734
{
@@ -1969,6 +1957,9 @@ namespace Js
19691957
bool m_hasFirstInnerScopeRegister : 1;
19701958
bool m_hasFuncExprScopeRegister : 1;
19711959
bool m_hasFirstTmpRegister : 1;
1960+
#if DBG
1961+
bool m_isSerialized : 1;
1962+
#endif
19721963
#ifdef PERF_COUNTERS
19731964
bool m_isDeserializedFunction : 1;
19741965
#endif
@@ -2095,8 +2086,10 @@ namespace Js
20952086
this->byteCodeCache = byteCodeCache;
20962087
}
20972088
}
2098-
void SetSerializationIndex(int index);
2099-
const int GetSerializationIndex() const;
2089+
#if DBG
2090+
void SetIsSerialized(bool serialized) { m_isSerialized = serialized; }
2091+
bool GetIsSerialized()const { return m_isSerialized; }
2092+
#endif
21002093
uint GetByteCodeCount() const { return GetCountField(CounterFields::ByteCodeCount); }
21012094
void SetByteCodeCount(uint count) { SetCountField(CounterFields::ByteCodeCount, count); }
21022095
uint GetByteCodeWithoutLDACount() const { return GetCountField(CounterFields::ByteCodeWithoutLDACount); }

0 commit comments

Comments
 (0)