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

lib/Common/Memory/AutoPtr.h

@@ -220,3 +220,24 @@ private:
         }
     }
 };
+
+template <typename T>
+class AutoCoTaskMemFreePtr : public BasePtr<T>
+{
+public:
+    AutoCoTaskMemFreePtr(T* ptr) : BasePtr<T>(ptr) {}
+    ~AutoCoTaskMemFreePtr()
+    {
+        Clear();
+    }
+
+private:
+    void Clear()
+    {
+        if (this->ptr != nullptr)
+        {
+            CoTaskMemFree(this->ptr);
+            this->ptr = nullptr;
+        }
+    }
+};

lib/Runtime/Base/ScriptContext.cpp

@@ -2159,17 +2159,6 @@ namespace Js
         return hr;
     }
 
-    template <class T>
-    class AutoCoTaskMemFreePtr
-    {
-    private:
-        T ptr;
-
-    public:
-        AutoCoTaskMemFreePtr(T ptr) : ptr(ptr) { }
-        ~AutoCoTaskMemFreePtr() { CoTaskMemFree(this->ptr); this->ptr = nullptr; }
-    };
-
     HRESULT ScriptContext::TrySerializeParserState(
         _In_ LPCUTF8 pszSrc,
         _In_ size_t cbLength,
@@ -2193,15 +2182,15 @@ namespace Js
             &parserStateCacheSize, dwFlags);
         END_TEMP_ALLOCATOR(tempAllocator, this);
 
+        // The parser state cache buffer was allocated by CoTaskMemAlloc.
+        // TODO: Keep this buffer around for the PLT1 scenario by deserializing it and storing a cache.
+        AutoCoTaskMemFreePtr<byte> autoFreeBytes(parserStateCacheBuffer);
+
         if (FAILED(hr))
         {
             return hr;
         }
 
-        // The parser state cache buffer was allocated by CoTaskMemAlloc.
-        // TODO: Keep this buffer around for the PLT1 scenario by deserializing it and storing a cache.
-        AutoCoTaskMemFreePtr<byte*> autoFreeBytes(parserStateCacheBuffer);
-
         IFFAILRET(pDataCache->StartBlock(Js::SimpleDataCacheWrapper::BlockType_ParserState, parserStateCacheSize));
         IFFAILRET(pDataCache->WriteArray(parserStateCacheBuffer, parserStateCacheSize));
 #endif

lib/Runtime/ByteCode/ByteCodeSerializer.cpp

@@ -2328,12 +2328,14 @@ public:
 
         for (uint i = 0; i < stubsCount; i++)
         {
-            PrependUInt32(builder, _u("Character Min"), deferredStubs[i].ichMin);
-            PrependUInt32(builder, _u("Function flags"), deferredStubs[i].fncFlags);
-            PrependStruct(builder, _u("Restore Point"), &deferredStubs[i].restorePoint);
+            DeferredFunctionStub* currentStub = &(deferredStubs[i]);
+
+            PrependUInt32(builder, _u("Character Min"), currentStub->ichMin);
+            PrependUInt32(builder, _u("Function flags"), currentStub->fncFlags);
+            PrependStruct(builder, _u("Restore Point"), &(currentStub->restorePoint));
 
             // Add all the captured name ids
-            IdentPtrSet *capturedNames = deferredStubs[i].capturedNamePointers;
+            IdentPtrSet *capturedNames = currentStub->capturedNamePointers;
 
             if (capturedNames != nullptr && capturedNames->Count() != 0)
             {
@@ -2360,10 +2362,10 @@ public:
                 PrependUInt32(builder, _u("Captured Name Count"), 0);
             }
 
-            PrependUInt32(builder, _u("Nested Count"), deferredStubs[i].nestedCount);
+            PrependUInt32(builder, _u("Nested Count"), currentStub->nestedCount);
             if (recursive)
             {
-                AddDeferredStubs(builder, deferredStubs[i].deferredStubs, deferredStubs[i].nestedCount, recursive);
+                AddDeferredStubs(builder, currentStub->deferredStubs, currentStub->nestedCount, recursive);
             }
         }
 

lib/Runtime/Language/SimpleDataCacheWrapper.cpp

@@ -63,19 +63,23 @@ namespace Js
 
     HRESULT SimpleDataCacheWrapper::OpenWriteStream()
     {
-        HRESULT hr = E_FAIL;
-
 #ifdef ENABLE_WININET_PROFILE_DATA_CACHE
+        HRESULT hr = E_FAIL;
         Assert(!IsWriteStreamOpen());
         Assert(this->dataCache != nullptr);
         Assert(this->outStream == nullptr);
         hr = this->dataCache->GetWriteDataStream(&this->outStream);
-#endif
+
         if (FAILED(hr))
         {
-            this->outStream = nullptr;
+            if (this->outStream != nullptr)
+            {
+                this->outStream->Release();
+                this->outStream = nullptr;
+            }
             return hr;
         }
+#endif
 
         return WriteHeader();
     }
@@ -185,7 +189,6 @@ namespace Js
         BlockType currentBlockType = BlockType_Invalid;
         ULONG byteCount = 0;
 
-        // Reset above has moved the seek pointer to just after the header, we're at the first block.
         IFFAILRET(Read(&currentBlockType));
         IFFAILRET(Read(&byteCount));
 
@@ -219,6 +222,7 @@ namespace Js
 
         IFFAILRET(ResetReadStream());
 
+        // Reset above has moved the seek pointer to just after the header, we're at the first block.
         return SeekReadStreamToBlockHelper(blockType, bytesInBlock);
     }
 }

lib/Runtime/Language/SimpleDataCacheWrapper.h

@@ -11,6 +11,21 @@ namespace Js
     // AddRef's the IActiveScriptDataCache passed to constructor and Release's it in the destructor.
     // We primarily need this class to support writing multiple blocks of data to the underlying
     // cache as it doesn't support opening multiple write streams.
+    // The cache underlying this wrapper is segmented into blocks where each block has a small
+    // header. The overall cache itself has a versioned header tied to the running version of
+    // Chakra. A cache built with one version of Chakra may not be loaded by other versions.
+    //
+    // Cache layout:
+    //      /----------------\
+    //      | majorVersion   |  <- Stream header
+    //      | minorVersion   |
+    //      ------------------
+    //      | BlockType      |  <- Block header
+    //      | blockByteCount |
+    //      ------------------
+    //      | <bytes>        |  <- Block contents
+    //      \----------------/
+    //
     // To write a block of data, call StartBlock (to create a header) and then Write or WriteArray
     // as many times as necessary to write the data into the cache.
     // To read a block of data, call SeekReadStreamToBlock (optionally fetching the size of this