[1.6>master] [MERGE #3152 @leirocks] wait for agent thread before shutdown

Merge pull request #3152 from leirocks:$262fix

resloving #3149 and re-enable the tests

bin/ch/RuntimeThreadData.cpp

@@ -27,11 +27,6 @@ void RuntimeThreadLocalData::Initialize(RuntimeThreadData* threadData)
 
 void RuntimeThreadLocalData::Uninitialize()
 {
-    if (threadData)
-    {
-        delete threadData;
-        threadData = nullptr;
-    }
 }
 
 
@@ -62,6 +57,7 @@ RuntimeThreadData::~RuntimeThreadData()
     CloseHandle(this->hevntInitialScriptCompleted);
     CloseHandle(this->hevntReceivedBroadcast);
     CloseHandle(this->hevntShutdown);
+    CloseHandle(this->hThread);
     DeleteCriticalSection(&csReportQ);
 }
 
@@ -129,11 +125,6 @@ DWORD RuntimeThreadData::ThreadProc()
             ChakraRTInterface::JsSetCurrentContext(nullptr);
             ChakraRTInterface::JsDisposeRuntime(runtime);
 
-            if (this->parent->hSemaphore != INVALID_HANDLE_VALUE)
-            {
-                ReleaseSemaphore(this->parent->hSemaphore, 1, NULL);
-            }
-
             threadLocalData.Uninitialize();
             return 0;
         }

bin/ch/RuntimeThreadData.h

@@ -14,6 +14,8 @@ public:
     HANDLE hevntReceivedBroadcast;
     HANDLE hevntShutdown;
     HANDLE hSemaphore;
+    HANDLE hThread;
+
     JsSharedArrayBufferContentHandle sharedContent;
     JsValueRef receiveBroadcastCallbackFunc;
 

bin/ch/WScriptJsrt.cpp

@@ -3,6 +3,7 @@
 // Licensed under the MIT license. See LICENSE.txt file in the project root for full license information.
 //-------------------------------------------------------------------------------------------------------
 #include "stdafx.h"
+#include <vector>
 
 #if defined(_X86_) || defined(_M_IX86)
 #define CPU_ARCH_TEXT "x86"
@@ -476,26 +477,27 @@ JsValueRef WScriptJsrt::LoadScript(JsValueRef callee, LPCSTR fileName,
     }
     else if (strcmp(scriptInjectType, "crossthread") == 0)
     {
-        if (GetRuntimeThreadLocalData().threadData == nullptr) 
+        auto& threadData = GetRuntimeThreadLocalData().threadData;
+        if (threadData == nullptr) 
         {
-            GetRuntimeThreadLocalData().threadData = new RuntimeThreadData();
+            threadData = new RuntimeThreadData();
         }
 
         RuntimeThreadData* child = new RuntimeThreadData();
         child->initialSource = fileContent;
-        GetRuntimeThreadLocalData().threadData->children.push_back(child);
-        child->parent = GetRuntimeThreadLocalData().threadData;
+        threadData->children.push_back(child);
+        child->parent = threadData;
                 
         // TODO: need to add a switch in case we don't need to wait for 
         // child initial script completion
-        ResetEvent(GetRuntimeThreadLocalData().threadData->hevntInitialScriptCompleted);
+        ResetEvent(threadData->hevntInitialScriptCompleted);
 
-        ::CreateThread(NULL, NULL, [](void* param) -> DWORD
+        child->hThread = ::CreateThread(NULL, NULL, [](void* param) -> DWORD
         {
             return ((RuntimeThreadData*)param)->ThreadProc();
         }, (void*)child, NULL, NULL);
 
-        WaitForSingleObject(GetRuntimeThreadLocalData().threadData->hevntInitialScriptCompleted, INFINITE);
+        WaitForSingleObject(threadData->hevntInitialScriptCompleted, INFINITE);
     }
     else
     {
@@ -957,28 +959,29 @@ bool WScriptJsrt::Uninitialize()
     // to avoid worrying about global destructor order.
     moduleRecordMap.clear();
 
-    if (GetRuntimeThreadLocalData().threadData && !GetRuntimeThreadLocalData().threadData->children.empty())
+    auto& threadData = GetRuntimeThreadLocalData().threadData;
+    if (threadData && !threadData->children.empty())
     {
-        LONG count = (LONG)GetRuntimeThreadLocalData().threadData->children.size();
-        GetRuntimeThreadLocalData().threadData->hSemaphore = CreateSemaphore(NULL, 0, count, NULL);
+        LONG count = (LONG)threadData->children.size();
+        std::vector<HANDLE> childrenHandles;
         
         //Clang does not support "for each" yet
-        for(auto i = GetRuntimeThreadLocalData().threadData->children.begin(); i!= GetRuntimeThreadLocalData().threadData->children.end(); i++)
+        for(auto i = threadData->children.begin(); i!= threadData->children.end(); i++)
         {
             auto child = *i;
-            if (child->leaving)
-            {
-                ReleaseSemaphore(GetRuntimeThreadLocalData().threadData->hSemaphore, 1, NULL);
-            }
-            else
-            {
-                SetEvent(child->hevntShutdown);
-            }
+            childrenHandles.push_back(child->hThread);
+            SetEvent(child->hevntShutdown);
+        }
+
+        DWORD waitRet = WaitForMultipleObjects(count, &childrenHandles[0], TRUE, INFINITE);
+        Assert(waitRet == WAIT_OBJECT_0);
+
+        for (auto i = threadData->children.begin(); i != threadData->children.end(); i++)
+        {
+            delete *i;
         }
 
-        WaitForSingleObject(GetRuntimeThreadLocalData().threadData->hSemaphore, INFINITE);
-        CloseHandle(GetRuntimeThreadLocalData().threadData->hSemaphore);
-        GetRuntimeThreadLocalData().threadData->hSemaphore = INVALID_HANDLE_VALUE;
+        threadData->children.clear();
     }
 
     return true;
@@ -1120,25 +1123,33 @@ JsValueRef __stdcall WScriptJsrt::BroadcastCallback(JsValueRef callee, bool isCo
 
     if (argumentCount > 1)
     {
-        if (GetRuntimeThreadLocalData().threadData)
+        auto& threadData = GetRuntimeThreadLocalData().threadData;
+        if (threadData)
         {
-            ChakraRTInterface::JsGetSharedArrayBufferContent(arguments[1], &GetRuntimeThreadLocalData().threadData->sharedContent);
+            ChakraRTInterface::JsGetSharedArrayBufferContent(arguments[1], &threadData->sharedContent);
 
-            LONG count = (LONG)GetRuntimeThreadLocalData().threadData->children.size();
-            GetRuntimeThreadLocalData().threadData->hSemaphore = CreateSemaphore(NULL, 0, count, NULL);
-            //Clang does not support "for each" yet
-            for (auto i = GetRuntimeThreadLocalData().threadData->children.begin(); i != GetRuntimeThreadLocalData().threadData->children.end(); i++)
+            LONG count = (LONG)threadData->children.size();
+            threadData->hSemaphore = CreateSemaphore(NULL, 0, count, NULL);
+            if (threadData->hSemaphore)
             {
-                auto child = *i;
-                SetEvent(child->hevntReceivedBroadcast);
-            }
-
-            WaitForSingleObject(GetRuntimeThreadLocalData().threadData->hSemaphore, INFINITE);
-            CloseHandle(GetRuntimeThreadLocalData().threadData->hSemaphore);
-            GetRuntimeThreadLocalData().threadData->hSemaphore = INVALID_HANDLE_VALUE;
+                //Clang does not support "for each" yet
+                for (auto i = threadData->children.begin(); i != threadData->children.end(); i++)
+                {
+                    auto child = *i;
+                    SetEvent(child->hevntReceivedBroadcast);
+                }
 
+                WaitForSingleObject(threadData->hSemaphore, INFINITE);
+                CloseHandle(threadData->hSemaphore);
+                threadData->hSemaphore = INVALID_HANDLE_VALUE;
+            }
+            else
+            {
+                fwprintf(stderr, _u("Couldn't create semaphore.\n"));
+                fflush(stderr);
+            }
 
-            ChakraRTInterface::JsReleaseSharedArrayBufferContentHandle(GetRuntimeThreadLocalData().threadData->sharedContent);
+            ChakraRTInterface::JsReleaseSharedArrayBufferContentHandle(threadData->sharedContent);
         }
     }
 
@@ -1156,14 +1167,15 @@ JsValueRef __stdcall WScriptJsrt::ReceiveBroadcastCallback(JsValueRef callee, bo
 
     if (argumentCount > 1)
     {
-        if (GetRuntimeThreadLocalData().threadData)
+        auto& threadData = GetRuntimeThreadLocalData().threadData;
+        if (threadData)
         {
-            if (GetRuntimeThreadLocalData().threadData->receiveBroadcastCallbackFunc)
+            if (threadData->receiveBroadcastCallbackFunc)
             {
-                ChakraRTInterface::JsRelease(GetRuntimeThreadLocalData().threadData->receiveBroadcastCallbackFunc, nullptr);
+                ChakraRTInterface::JsRelease(threadData->receiveBroadcastCallbackFunc, nullptr);
             }
-            GetRuntimeThreadLocalData().threadData->receiveBroadcastCallbackFunc = arguments[1];
-            ChakraRTInterface::JsAddRef(GetRuntimeThreadLocalData().threadData->receiveBroadcastCallbackFunc, nullptr);
+            threadData->receiveBroadcastCallbackFunc = arguments[1];
+            ChakraRTInterface::JsAddRef(threadData->receiveBroadcastCallbackFunc, nullptr);
         }
     }
 
@@ -1189,12 +1201,13 @@ JsValueRef __stdcall WScriptJsrt::ReportCallback(JsValueRef callee, bool isConst
         if (autoStr.GetError() == JsNoError)
         {
             std::string str(autoStr.GetString());
+            auto& threadData = GetRuntimeThreadLocalData().threadData;
 
-            if (GetRuntimeThreadLocalData().threadData && GetRuntimeThreadLocalData().threadData->parent)
+            if (threadData && threadData->parent)
             {
-                EnterCriticalSection(&GetRuntimeThreadLocalData().threadData->parent->csReportQ);
-                GetRuntimeThreadLocalData().threadData->parent->reportQ.push_back(str);
-                LeaveCriticalSection(&GetRuntimeThreadLocalData().threadData->parent->csReportQ);
+                EnterCriticalSection(&threadData->parent->csReportQ);
+                threadData->parent->reportQ.push_back(str);
+                LeaveCriticalSection(&threadData->parent->csReportQ);
             }
         }
     }
@@ -1213,17 +1226,18 @@ JsValueRef __stdcall WScriptJsrt::GetReportCallback(JsValueRef callee, bool isCo
 
     if (argumentCount > 0)
     {
-        if (GetRuntimeThreadLocalData().threadData)
+        auto& threadData = GetRuntimeThreadLocalData().threadData;
+        if (threadData)
         {
-            EnterCriticalSection(&GetRuntimeThreadLocalData().threadData->csReportQ);
+            EnterCriticalSection(&threadData->csReportQ);
 
-            if (GetRuntimeThreadLocalData().threadData->reportQ.size() > 0)
+            if (threadData->reportQ.size() > 0)
             {
-                auto str = GetRuntimeThreadLocalData().threadData->reportQ.front();
-                GetRuntimeThreadLocalData().threadData->reportQ.pop_front();
+                auto str = threadData->reportQ.front();
+                threadData->reportQ.pop_front();
                 ChakraRTInterface::JsCreateString(str.c_str(), str.size(), &returnValue);
             }
-            LeaveCriticalSection(&GetRuntimeThreadLocalData().threadData->csReportQ);
+            LeaveCriticalSection(&threadData->csReportQ);
         }
     }
 
@@ -1241,9 +1255,10 @@ JsValueRef __stdcall WScriptJsrt::LeavingCallback(JsValueRef callee, bool isCons
 
     if (argumentCount > 0)
     {
-        if (GetRuntimeThreadLocalData().threadData)
+        auto& threadData = GetRuntimeThreadLocalData().threadData;
+        if (threadData)
         {
-            GetRuntimeThreadLocalData().threadData->leaving = true;
+            threadData->leaving = true;
         }
     }
 

test/$262/rlexe.xml

@@ -1,6 +1,5 @@
 <?xml version="1.0" encoding="utf-8"?>
 <regress-exe>
-<!-- issue #3149
   <test>
     <default>
       <compile-flags>-$262</compile-flags>
@@ -8,5 +7,4 @@
       <files>$262test.js</files>
     </default>
   </test>
--->
 </regress-exe>