Skip to content

Commit ab1d30d

Browse files
boingoingboingoing
committed
Move AutoCoTaskMemFreePtr into AutoPtr.h
Make sure SimpleDataCacheWrapper releases the output stream COM pointer if we fail trying to get it. Cleanups and some documentation
1 parent 0515855 commit ab1d30d

5 files changed

Lines changed: 57 additions & 26 deletions

File tree

‎lib/Common/Memory/AutoPtr.h‎

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -220,3 +220,24 @@ class AutoDiscardPTR : public BasePtr<T>
220220
}
221221
}
222222
};
223+
224+
template <typename T>
225+
class AutoCoTaskMemFreePtr : public BasePtr<T>
226+
{
227+
public:
228+
AutoCoTaskMemFreePtr(T* ptr) : BasePtr<T>(ptr) {}
229+
~AutoCoTaskMemFreePtr()
230+
{
231+
Clear();
232+
}
233+
234+
private:
235+
void Clear()
236+
{
237+
if (this->ptr != nullptr)
238+
{
239+
CoTaskMemFree(this->ptr);
240+
this->ptr = nullptr;
241+
}
242+
}
243+
};

‎lib/Runtime/Base/ScriptContext.cpp‎

Lines changed: 4 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -2159,17 +2159,6 @@ namespace Js
21592159
return hr;
21602160
}
21612161

2162-
template <class T>
2163-
class AutoCoTaskMemFreePtr
2164-
{
2165-
private:
2166-
T ptr;
2167-
2168-
public:
2169-
AutoCoTaskMemFreePtr(T ptr) : ptr(ptr) { }
2170-
~AutoCoTaskMemFreePtr() { CoTaskMemFree(this->ptr); this->ptr = nullptr; }
2171-
};
2172-
21732162
HRESULT ScriptContext::TrySerializeParserState(
21742163
_In_ LPCUTF8 pszSrc,
21752164
_In_ size_t cbLength,
@@ -2193,15 +2182,15 @@ namespace Js
21932182
&parserStateCacheSize, dwFlags);
21942183
END_TEMP_ALLOCATOR(tempAllocator, this);
21952184

2185+
// The parser state cache buffer was allocated by CoTaskMemAlloc.
2186+
// TODO: Keep this buffer around for the PLT1 scenario by deserializing it and storing a cache.
2187+
AutoCoTaskMemFreePtr<byte> autoFreeBytes(parserStateCacheBuffer);
2188+
21962189
if (FAILED(hr))
21972190
{
21982191
return hr;
21992192
}
22002193

2201-
// The parser state cache buffer was allocated by CoTaskMemAlloc.
2202-
// TODO: Keep this buffer around for the PLT1 scenario by deserializing it and storing a cache.
2203-
AutoCoTaskMemFreePtr<byte*> autoFreeBytes(parserStateCacheBuffer);
2204-
22052194
IFFAILRET(pDataCache->StartBlock(Js::SimpleDataCacheWrapper::BlockType_ParserState, parserStateCacheSize));
22062195
IFFAILRET(pDataCache->WriteArray(parserStateCacheBuffer, parserStateCacheSize));
22072196
#endif

‎lib/Runtime/ByteCode/ByteCodeSerializer.cpp‎

Lines changed: 8 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -2328,12 +2328,14 @@ class ByteCodeBufferBuilder
23282328

23292329
for (uint i = 0; i < stubsCount; i++)
23302330
{
2331-
PrependUInt32(builder, _u("Character Min"), deferredStubs[i].ichMin);
2332-
PrependUInt32(builder, _u("Function flags"), deferredStubs[i].fncFlags);
2333-
PrependStruct(builder, _u("Restore Point"), &deferredStubs[i].restorePoint);
2331+
DeferredFunctionStub* currentStub = &(deferredStubs[i]);
2332+
2333+
PrependUInt32(builder, _u("Character Min"), currentStub->ichMin);
2334+
PrependUInt32(builder, _u("Function flags"), currentStub->fncFlags);
2335+
PrependStruct(builder, _u("Restore Point"), &(currentStub->restorePoint));
23342336

23352337
// Add all the captured name ids
2336-
IdentPtrSet *capturedNames = deferredStubs[i].capturedNamePointers;
2338+
IdentPtrSet *capturedNames = currentStub->capturedNamePointers;
23372339

23382340
if (capturedNames != nullptr && capturedNames->Count() != 0)
23392341
{
@@ -2360,10 +2362,10 @@ class ByteCodeBufferBuilder
23602362
PrependUInt32(builder, _u("Captured Name Count"), 0);
23612363
}
23622364

2363-
PrependUInt32(builder, _u("Nested Count"), deferredStubs[i].nestedCount);
2365+
PrependUInt32(builder, _u("Nested Count"), currentStub->nestedCount);
23642366
if (recursive)
23652367
{
2366-
AddDeferredStubs(builder, deferredStubs[i].deferredStubs, deferredStubs[i].nestedCount, recursive);
2368+
AddDeferredStubs(builder, currentStub->deferredStubs, currentStub->nestedCount, recursive);
23672369
}
23682370
}
23692371

‎lib/Runtime/Language/SimpleDataCacheWrapper.cpp‎

Lines changed: 9 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -63,19 +63,23 @@ namespace Js
6363

6464
HRESULT SimpleDataCacheWrapper::OpenWriteStream()
6565
{
66-
HRESULT hr = E_FAIL;
67-
6866
#ifdef ENABLE_WININET_PROFILE_DATA_CACHE
67+
HRESULT hr = E_FAIL;
6968
Assert(!IsWriteStreamOpen());
7069
Assert(this->dataCache != nullptr);
7170
Assert(this->outStream == nullptr);
7271
hr = this->dataCache->GetWriteDataStream(&this->outStream);
73-
#endif
72+
7473
if (FAILED(hr))
7574
{
76-
this->outStream = nullptr;
75+
if (this->outStream != nullptr)
76+
{
77+
this->outStream->Release();
78+
this->outStream = nullptr;
79+
}
7780
return hr;
7881
}
82+
#endif
7983

8084
return WriteHeader();
8185
}
@@ -185,7 +189,6 @@ namespace Js
185189
BlockType currentBlockType = BlockType_Invalid;
186190
ULONG byteCount = 0;
187191

188-
// Reset above has moved the seek pointer to just after the header, we're at the first block.
189192
IFFAILRET(Read(&currentBlockType));
190193
IFFAILRET(Read(&byteCount));
191194

@@ -219,6 +222,7 @@ namespace Js
219222

220223
IFFAILRET(ResetReadStream());
221224

225+
// Reset above has moved the seek pointer to just after the header, we're at the first block.
222226
return SeekReadStreamToBlockHelper(blockType, bytesInBlock);
223227
}
224228
}

‎lib/Runtime/Language/SimpleDataCacheWrapper.h‎

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,21 @@ namespace Js
1111
// AddRef's the IActiveScriptDataCache passed to constructor and Release's it in the destructor.
1212
// We primarily need this class to support writing multiple blocks of data to the underlying
1313
// cache as it doesn't support opening multiple write streams.
14+
// The cache underlying this wrapper is segmented into blocks where each block has a small
15+
// header. The overall cache itself has a versioned header tied to the running version of
16+
// Chakra. A cache built with one version of Chakra may not be loaded by other versions.
17+
//
18+
// Cache layout:
19+
// /----------------\
20+
// | majorVersion | <- Stream header
21+
// | minorVersion |
22+
// ------------------
23+
// | BlockType | <- Block header
24+
// | blockByteCount |
25+
// ------------------
26+
// | <bytes> | <- Block contents
27+
// \----------------/
28+
//
1429
// To write a block of data, call StartBlock (to create a header) and then Write or WriteArray
1530
// as many times as necessary to write the data into the cache.
1631
// To read a block of data, call SeekReadStreamToBlock (optionally fetching the size of this

0 commit comments

Comments
 (0)