Fixed memory leak associated with TLS.

We used to allocate thread-local memory on each compile.
If the compile did not happen on the same thread as ShInitialize,
we leaked the thread-local memory.

It turns out that there is no need to allocate any thread-local
memory. This patch cleans up all the unnecessary junk around TLS.

BUG=crbug.com/181691
R=kbr@chromium.org

Review URL: https://codereview.appspot.com/11679046

16 files changed, 56 insertions, 250 deletions

diff --git a/src/compiler/Common.h b/src/compiler/Common.h
index 532486a9..46f9440f 100644
--- a/src/compiler/Common.h
+++ b/src/compiler/Common.h
@@ -24,14 +24,14 @@ struct TSourceLoc {
 //
 // Put POOL_ALLOCATOR_NEW_DELETE in base classes to make them use this scheme.
 //
-#define POOL_ALLOCATOR_NEW_DELETE(A)                                  \
-    void* operator new(size_t s) { return (A).allocate(s); }          \
-    void* operator new(size_t, void *_Where) { return (_Where);	}     \
-    void operator delete(void*) { }                                   \
-    void operator delete(void *, void *) { }                          \
-    void* operator new[](size_t s) { return (A).allocate(s); }        \
-    void* operator new[](size_t, void *_Where) { return (_Where);	} \
-    void operator delete[](void*) { }                                 \
+#define POOL_ALLOCATOR_NEW_DELETE()                                                  \
+    void* operator new(size_t s) { return GetGlobalPoolAllocator()->allocate(s); }   \
+    void* operator new(size_t, void *_Where) { return (_Where); }                    \
+    void operator delete(void*) { }                                                  \
+    void operator delete(void *, void *) { }                                         \
+    void* operator new[](size_t s) { return GetGlobalPoolAllocator()->allocate(s); } \
+    void* operator new[](size_t, void *_Where) { return (_Where); }                  \
+    void operator delete[](void*) { }                                                \
     void operator delete[](void *, void *) { }
 
 //
@@ -42,7 +42,7 @@ typedef std::basic_string <char, std::char_traits<char>, TStringAllocator> TStri
 typedef std::basic_ostringstream<char, std::char_traits<char>, TStringAllocator> TStringStream;
 inline TString* NewPoolTString(const char* s)
 {
-	void* memory = GlobalPoolAllocator.allocate(sizeof(TString));
+	void* memory = GetGlobalPoolAllocator()->allocate(sizeof(TString));
 	return new(memory) TString(s);
 }
 
diff --git a/src/compiler/Compiler.cpp b/src/compiler/Compiler.cpp
index 263ec770..3e80d96a 100644
--- a/src/compiler/Compiler.cpp
+++ b/src/compiler/Compiler.cpp
@@ -125,7 +125,7 @@ bool TCompiler::compile(const char* const shaderStrings[],
                                shaderType, shaderSpec, compileOptions, true,
                                sourcePath, infoSink);
     parseContext.fragmentPrecisionHigh = fragmentPrecisionHigh;
-    GlobalParseContext = &parseContext;
+    SetGlobalParseContext(&parseContext);
 
     // We preserve symbols at the built-in level from compile-to-compile.
     // Start pushing the user-defined symbols at global level.
diff --git a/src/compiler/ConstantUnion.h b/src/compiler/ConstantUnion.h
index 32af4d38..b1e37885 100644
--- a/src/compiler/ConstantUnion.h
+++ b/src/compiler/ConstantUnion.h
@@ -11,13 +11,13 @@
 
 class ConstantUnion {
 public:
+    POOL_ALLOCATOR_NEW_DELETE();
     ConstantUnion()
     {
         iConst = 0;
         type = EbtVoid;
     }
 
-    POOL_ALLOCATOR_NEW_DELETE(GlobalPoolAllocator)        
     void setIConst(int i) {iConst = i; type = EbtInt; }
     void setFConst(float f) {fConst = f; type = EbtFloat; }
     void setBConst(bool b) {bConst = b; type = EbtBool; }
diff --git a/src/compiler/InitializeDll.cpp b/src/compiler/InitializeDll.cpp
index 8763cfee..6c7f27fc 100644
--- a/src/compiler/InitializeDll.cpp
+++ b/src/compiler/InitializeDll.cpp
@@ -10,25 +10,8 @@
 #include "compiler/InitializeParseContext.h"
 #include "compiler/osinclude.h"
 
-OS_TLSIndex ThreadInitializeIndex = OS_INVALID_TLS_INDEX;
-
 bool InitProcess()
 {
-    if (ThreadInitializeIndex != OS_INVALID_TLS_INDEX) {
-        //
-        // Function is re-entrant.
-        //
-        return true;
-    }
-
-    ThreadInitializeIndex = OS_AllocTLSIndex();
-
-    if (ThreadInitializeIndex == OS_INVALID_TLS_INDEX) {
-        assert(0 && "InitProcess(): Failed to allocate TLS area for init flag");
-        return false;
-    }
-
-
     if (!InitializePoolIndex()) {
         assert(0 && "InitProcess(): Failed to initalize global pool");
         return false;
@@ -39,77 +22,11 @@ bool InitProcess()
         return false;
     }
 
-    return InitThread();
-}
-
-bool DetachProcess()
-{
-    bool success = true;
-
-    if (ThreadInitializeIndex == OS_INVALID_TLS_INDEX)
-        return true;
-
-    success = DetachThread();
-
-    if (!FreeParseContextIndex())
-        success = false;
-
-    FreePoolIndex();
-
-    OS_FreeTLSIndex(ThreadInitializeIndex);
-    ThreadInitializeIndex = OS_INVALID_TLS_INDEX;
-
-    return success;
-}
-
-bool InitThread()
-{
-    //
-    // This function is re-entrant
-    //
-    if (ThreadInitializeIndex == OS_INVALID_TLS_INDEX) {
-        assert(0 && "InitThread(): Process hasn't been initalised.");
-        return false;
-    }
-
-    if (OS_GetTLSValue(ThreadInitializeIndex) != 0)
-        return true;
-
-    InitializeGlobalPools();
-
-    if (!InitializeGlobalParseContext())
-        return false;
-
-    if (!OS_SetTLSValue(ThreadInitializeIndex, (void *)1)) {
-        assert(0 && "InitThread(): Unable to set init flag.");
-        return false;
-    }
-
     return true;
 }
 
-bool DetachThread()
+void DetachProcess()
 {
-    bool success = true;
-
-    if (ThreadInitializeIndex == OS_INVALID_TLS_INDEX)
-        return true;
-
-    //
-    // Function is re-entrant and this thread may not have been initalised.
-    //
-    if (OS_GetTLSValue(ThreadInitializeIndex) != 0) {
-        if (!OS_SetTLSValue(ThreadInitializeIndex, (void *)0)) {
-            assert(0 && "DetachThread(): Unable to clear init flag.");
-            success = false;
-        }
-
-        if (!FreeParseContext())
-            success = false;
-
-        FreeGlobalPools();
-    }
-
-    return success;
+    FreeParseContextIndex();
+    FreePoolIndex();
 }
-
diff --git a/src/compiler/InitializeDll.h b/src/compiler/InitializeDll.h
index 857238ee..43070cc3 100644
--- a/src/compiler/InitializeDll.h
+++ b/src/compiler/InitializeDll.h
@@ -7,10 +7,7 @@
 #define __INITIALIZEDLL_H
 
 bool InitProcess();
-bool DetachProcess();
-
-bool InitThread();
-bool DetachThread();
+void DetachProcess();
 
 #endif // __INITIALIZEDLL_H
 
diff --git a/src/compiler/InitializeGlobals.h b/src/compiler/InitializeGlobals.h
index 842a4528..07159414 100644
--- a/src/compiler/InitializeGlobals.h
+++ b/src/compiler/InitializeGlobals.h
@@ -7,8 +7,6 @@
 #ifndef __INITIALIZE_GLOBALS_INCLUDED_
 #define __INITIALIZE_GLOBALS_INCLUDED_
 
-void InitializeGlobalPools();
-void FreeGlobalPools();
 bool InitializePoolIndex();
 void FreePoolIndex();
 
diff --git a/src/compiler/InitializeParseContext.cpp b/src/compiler/InitializeParseContext.cpp
index 1f40cf58..dfab0273 100644
--- a/src/compiler/InitializeParseContext.cpp
+++ b/src/compiler/InitializeParseContext.cpp
@@ -12,85 +12,29 @@ OS_TLSIndex GlobalParseContextIndex = OS_INVALID_TLS_INDEX;
 
 bool InitializeParseContextIndex()
 {
-    if (GlobalParseContextIndex != OS_INVALID_TLS_INDEX) {
-        assert(0 && "InitializeParseContextIndex(): Parse Context already initalized");
-        return false;
-    }
+    assert(GlobalParseContextIndex == OS_INVALID_TLS_INDEX);
 
-    //
-    // Allocate a TLS index.
-    //
     GlobalParseContextIndex = OS_AllocTLSIndex();
-    
-    if (GlobalParseContextIndex == OS_INVALID_TLS_INDEX) {
-        assert(0 && "InitializeParseContextIndex(): Parse Context already initalized");
-        return false;
-    }
-
-    return true;
+    return GlobalParseContextIndex != OS_INVALID_TLS_INDEX;
 }
 
-bool FreeParseContextIndex()
+void FreeParseContextIndex()
 {
-    OS_TLSIndex tlsiIndex = GlobalParseContextIndex;
-
-    if (GlobalParseContextIndex == OS_INVALID_TLS_INDEX) {
-        assert(0 && "FreeParseContextIndex(): Parse Context index not initalized");
-        return false;
-    }
+    assert(GlobalParseContextIndex != OS_INVALID_TLS_INDEX);
 
+    OS_FreeTLSIndex(GlobalParseContextIndex);
     GlobalParseContextIndex = OS_INVALID_TLS_INDEX;
-
-    return OS_FreeTLSIndex(tlsiIndex);
-}
-
-bool InitializeGlobalParseContext()
-{
-    if (GlobalParseContextIndex == OS_INVALID_TLS_INDEX) {
-        assert(0 && "InitializeGlobalParseContext(): Parse Context index not initalized");
-        return false;
-    }
-
-    TThreadParseContext *lpParseContext = static_cast<TThreadParseContext *>(OS_GetTLSValue(GlobalParseContextIndex));
-    if (lpParseContext != 0) {
-        assert(0 && "InitializeParseContextIndex(): Parse Context already initalized");
-        return false;
-    }
-
-    TThreadParseContext *lpThreadData = new TThreadParseContext();
-    if (lpThreadData == 0) {
-        assert(0 && "InitializeGlobalParseContext(): Unable to create thread parse context");
-        return false;
-    }
-
-    lpThreadData->lpGlobalParseContext = 0;
-    OS_SetTLSValue(GlobalParseContextIndex, lpThreadData);
-
-    return true;
 }
 
-bool FreeParseContext()
+void SetGlobalParseContext(TParseContext* context)
 {
-    if (GlobalParseContextIndex == OS_INVALID_TLS_INDEX) {
-        assert(0 && "FreeParseContext(): Parse Context index not initalized");
-        return false;
-    }
-
-    TThreadParseContext *lpParseContext = static_cast<TThreadParseContext *>(OS_GetTLSValue(GlobalParseContextIndex));
-    if (lpParseContext)
-        delete lpParseContext;
-
-    return true;
+    assert(GlobalParseContextIndex != OS_INVALID_TLS_INDEX);
+    OS_SetTLSValue(GlobalParseContextIndex, context);
 }
 
-TParseContextPointer& GetGlobalParseContext()
+TParseContext* GetGlobalParseContext()
 {
-    //
-    // Minimal error checking for speed
-    //
-
-    TThreadParseContext *lpParseContext = static_cast<TThreadParseContext *>(OS_GetTLSValue(GlobalParseContextIndex));
-
-    return lpParseContext->lpGlobalParseContext;
+    assert(GlobalParseContextIndex != OS_INVALID_TLS_INDEX);
+    return static_cast<TParseContext*>(OS_GetTLSValue(GlobalParseContextIndex));
 }
 
diff --git a/src/compiler/InitializeParseContext.h b/src/compiler/InitializeParseContext.h
index aa53b735..bffbab87 100644
--- a/src/compiler/InitializeParseContext.h
+++ b/src/compiler/InitializeParseContext.h
@@ -8,19 +8,10 @@
 #define __INITIALIZE_PARSE_CONTEXT_INCLUDED_
 
 bool InitializeParseContextIndex();
-bool FreeParseContextIndex();
-
-bool InitializeGlobalParseContext();
-bool FreeParseContext();
+void FreeParseContextIndex();
 
 struct TParseContext;
-typedef TParseContext* TParseContextPointer;
-extern TParseContextPointer& GetGlobalParseContext();
-#define GlobalParseContext GetGlobalParseContext()
-
-typedef struct TThreadParseContextRec
-{
-    TParseContext *lpGlobalParseContext;
-} TThreadParseContext;
+extern void SetGlobalParseContext(TParseContext* context);
+extern TParseContext* GetGlobalParseContext();
 
 #endif // __INITIALIZE_PARSE_CONTEXT_INCLUDED_
diff --git a/src/compiler/PoolAlloc.cpp b/src/compiler/PoolAlloc.cpp
index 3f0f5a63..eb993567 100644
--- a/src/compiler/PoolAlloc.cpp
+++ b/src/compiler/PoolAlloc.cpp
@@ -17,55 +17,32 @@
 
 OS_TLSIndex PoolIndex = OS_INVALID_TLS_INDEX;
 
-void InitializeGlobalPools()
-{
-    TThreadGlobalPools* globalPools= static_cast<TThreadGlobalPools*>(OS_GetTLSValue(PoolIndex));    
-    if (globalPools)
-        return;
-
-    TThreadGlobalPools* threadData = new TThreadGlobalPools();
-    threadData->globalPoolAllocator = 0;
-
-    OS_SetTLSValue(PoolIndex, threadData);
-}
-
-void FreeGlobalPools()
-{
-    // Release the allocated memory for this thread.
-    TThreadGlobalPools* globalPools= static_cast<TThreadGlobalPools*>(OS_GetTLSValue(PoolIndex));    
-    if (!globalPools)
-        return;
- 
-    delete globalPools;
-}
-
 bool InitializePoolIndex()
 {
-    // Allocate a TLS index.
-    if ((PoolIndex = OS_AllocTLSIndex()) == OS_INVALID_TLS_INDEX)
-        return false;
+    assert(PoolIndex == OS_INVALID_TLS_INDEX);
 
-    return true;
+    PoolIndex = OS_AllocTLSIndex();
+    return PoolIndex != OS_INVALID_TLS_INDEX;
 }
 
 void FreePoolIndex()
 {
-    // Release the TLS index.
+    assert(PoolIndex != OS_INVALID_TLS_INDEX);
+
     OS_FreeTLSIndex(PoolIndex);
+    PoolIndex = OS_INVALID_TLS_INDEX;
 }
 
-TPoolAllocator& GetGlobalPoolAllocator()
+TPoolAllocator* GetGlobalPoolAllocator()
 {
-    TThreadGlobalPools* threadData = static_cast<TThreadGlobalPools*>(OS_GetTLSValue(PoolIndex));
-
-    return *threadData->globalPoolAllocator;
+    assert(PoolIndex != OS_INVALID_TLS_INDEX);
+    return static_cast<TPoolAllocator*>(OS_GetTLSValue(PoolIndex));
 }
 
 void SetGlobalPoolAllocator(TPoolAllocator* poolAllocator)
 {
-    TThreadGlobalPools* threadData = static_cast<TThreadGlobalPools*>(OS_GetTLSValue(PoolIndex));
-
-    threadData->globalPoolAllocator = poolAllocator;
+    assert(PoolIndex != OS_INVALID_TLS_INDEX);
+    OS_SetTLSValue(PoolIndex, poolAllocator);
 }
 
 //
diff --git a/src/compiler/PoolAlloc.h b/src/compiler/PoolAlloc.h
index a8a59c69..edd249c4 100644
--- a/src/compiler/PoolAlloc.h
+++ b/src/compiler/PoolAlloc.h
@@ -219,14 +219,8 @@ private:
 // different times.  But a simple use is to have a global pop
 // with everyone using the same global allocator.
 //
-extern TPoolAllocator& GetGlobalPoolAllocator();
+extern TPoolAllocator* GetGlobalPoolAllocator();
 extern void SetGlobalPoolAllocator(TPoolAllocator* poolAllocator);
-#define GlobalPoolAllocator GetGlobalPoolAllocator()
-
-struct TThreadGlobalPools
-{
-    TPoolAllocator* globalPoolAllocator;
-};
 
 //
 // This STL compatible allocator is intended to be used as the allocator
@@ -253,7 +247,7 @@ public:
     pointer address(reference x) const { return &x; }
     const_pointer address(const_reference x) const { return &x; }
 
-    pool_allocator() : allocator(&GlobalPoolAllocator) { }
+    pool_allocator() : allocator(GetGlobalPoolAllocator()) { }
     pool_allocator(TPoolAllocator& a) : allocator(&a) { }
     pool_allocator(const pool_allocator<T>& p) : allocator(p.allocator) { }
 
diff --git a/src/compiler/ShaderLang.cpp b/src/compiler/ShaderLang.cpp
index 3f637038..5e4535e5 100644
--- a/src/compiler/ShaderLang.cpp
+++ b/src/compiler/ShaderLang.cpp
@@ -90,10 +90,7 @@ static void getVariableInfo(ShShaderInfo varType,
 //
 int ShInitialize()
 {
-    if (!InitProcess())
-        return 0;
-
-    return 1;
+    return InitProcess() ? 1 : 0;
 }
 
 //
@@ -101,9 +98,7 @@ int ShInitialize()
 //
 int ShFinalize()
 {
-    if (!DetachProcess())
-        return 0;
-
+    DetachProcess();
     return 1;
 }
 
@@ -145,9 +140,6 @@ ShHandle ShConstructCompiler(ShShaderType type, ShShaderSpec spec,
                              ShShaderOutput output,
                              const ShBuiltInResources* resources)
 {
-    if (!InitThread())
-        return 0;
-
     TShHandleBase* base = static_cast<TShHandleBase*>(ConstructCompiler(type, spec, output));
     TCompiler* compiler = base->getAsCompiler();
     if (compiler == 0)
@@ -186,9 +178,6 @@ int ShCompile(
     size_t numStrings,
     int compileOptions)
 {
-    if (!InitThread())
-        return 0;
-
     if (handle == 0)
         return 0;
 
diff --git a/src/compiler/SymbolTable.h b/src/compiler/SymbolTable.h
index 7ea6cd15..f9a7948a 100644
--- a/src/compiler/SymbolTable.h
+++ b/src/compiler/SymbolTable.h
@@ -41,9 +41,10 @@
 //
 class TSymbol {    
 public:
-    POOL_ALLOCATOR_NEW_DELETE(GlobalPoolAllocator)
+    POOL_ALLOCATOR_NEW_DELETE();
     TSymbol(const TString *n) :  name(n) { }
     virtual ~TSymbol() { /* don't delete name, it's from the pool */ }
+
     const TString& getName() const { return *name; }
     virtual const TString& getMangledName() const { return getName(); }
     virtual bool isFunction() const { return false; }
@@ -186,7 +187,7 @@ public:
     typedef const tLevel::value_type tLevelPair;
     typedef std::pair<tLevel::iterator, bool> tInsertResult;
 
-    POOL_ALLOCATOR_NEW_DELETE(GlobalPoolAllocator)
+    POOL_ALLOCATOR_NEW_DELETE();
     TSymbolTableLevel() { }
     ~TSymbolTableLevel();
 
diff --git a/src/compiler/Types.h b/src/compiler/Types.h
index 5946af04..505fa8e3 100644
--- a/src/compiler/Types.h
+++ b/src/compiler/Types.h
@@ -19,7 +19,7 @@ class TType;
 class TField
 {
 public:
-    POOL_ALLOCATOR_NEW_DELETE(GlobalPoolAllocator);
+    POOL_ALLOCATOR_NEW_DELETE();
     TField(TType* type, TString* name) : mType(type), mName(name) {}
 
     // TODO(alokp): We should only return const type.
@@ -38,14 +38,14 @@ private:
 typedef TVector<TField*> TFieldList;
 inline TFieldList* NewPoolTFieldList()
 {
-    void* memory = GlobalPoolAllocator.allocate(sizeof(TFieldList));
+    void* memory = GetGlobalPoolAllocator()->allocate(sizeof(TFieldList));
     return new(memory) TFieldList;
 }
 
 class TStructure
 {
 public:
-    POOL_ALLOCATOR_NEW_DELETE(GlobalPoolAllocator);
+    POOL_ALLOCATOR_NEW_DELETE();
     TStructure(TString* name, TFieldList* fields)
         : mName(name),
           mFields(fields),
@@ -93,7 +93,7 @@ private:
 class TType
 {
 public:
-    POOL_ALLOCATOR_NEW_DELETE(GlobalPoolAllocator)
+    POOL_ALLOCATOR_NEW_DELETE();
     TType() {}
     TType(TBasicType t, TPrecision p, TQualifier q = EvqTemporary, int s = 1, bool m = false, bool a = false) :
             type(t), precision(p), qualifier(q), size(s), matrix(m), array(a), arraySize(0), structure(0)
diff --git a/src/compiler/ValidateLimitations.cpp b/src/compiler/ValidateLimitations.cpp
index a5562d09..736ceeae 100644
--- a/src/compiler/ValidateLimitations.cpp
+++ b/src/compiler/ValidateLimitations.cpp
@@ -435,7 +435,7 @@ bool ValidateLimitations::validateFunctionCall(TIntermAggregate* node)
         return true;
 
     bool valid = true;
-    TSymbolTable& symbolTable = GlobalParseContext->symbolTable;
+    TSymbolTable& symbolTable = GetGlobalParseContext()->symbolTable;
     TSymbol* symbol = symbolTable.find(node->getName());
     ASSERT(symbol && symbol->isFunction());
     TFunction* function = static_cast<TFunction*>(symbol);
diff --git a/src/compiler/intermediate.h b/src/compiler/intermediate.h
index b1d20f70..738621fe 100644
--- a/src/compiler/intermediate.h
+++ b/src/compiler/intermediate.h
@@ -205,8 +205,7 @@ class TInfoSink;
 //
 class TIntermNode {
 public:
-    POOL_ALLOCATOR_NEW_DELETE(GlobalPoolAllocator)
-
+    POOL_ALLOCATOR_NEW_DELETE();
     TIntermNode() {
         // TODO: Move this to TSourceLoc constructor
         // after getting rid of TPublicType.
@@ -540,8 +539,7 @@ enum Visit
 class TIntermTraverser
 {
 public:
-    POOL_ALLOCATOR_NEW_DELETE(GlobalPoolAllocator)
-
+    POOL_ALLOCATOR_NEW_DELETE();
     TIntermTraverser(bool preVisit = true, bool inVisit = false, bool postVisit = false, bool rightToLeft = false) : 
             preVisit(preVisit),
             inVisit(inVisit),
diff --git a/src/compiler/localintermediate.h b/src/compiler/localintermediate.h
index b6734070..ccbc4017 100644
--- a/src/compiler/localintermediate.h
+++ b/src/compiler/localintermediate.h
@@ -22,9 +22,9 @@ struct TVectorFields {
 class TInfoSink;
 class TIntermediate {
 public:    
-    POOL_ALLOCATOR_NEW_DELETE(GlobalPoolAllocator)
-
+    POOL_ALLOCATOR_NEW_DELETE();
     TIntermediate(TInfoSink& i) : infoSink(i) { }
+
     TIntermSymbol* addSymbol(int Id, const TString&, const TType&, const TSourceLoc&);
     TIntermTyped* addConversion(TOperator, const TType&, TIntermTyped*);
     TIntermTyped* addBinaryMath(TOperator op, TIntermTyped* left, TIntermTyped* right, const TSourceLoc&, TSymbolTable&);