[1.10>master] [MERGE #5519 @akroshg] OS 18333466 : Recursion in the g…

…enerator next call. Merge pull request #5519 from akroshg:generator Moving exception throw out from the catch handler. Otherwise we will throw hard stack overflow.
chakra-core · Jul 25, 2018 · d28ff47 · d28ff47
2 parents 6906f95 + 55c9960
commit d28ff47
Show file tree

Hide file tree

Showing 3 changed files with 33 additions and 7 deletions.
diff --git a/lib/Runtime/Language/JavascriptOperators.inl b/lib/Runtime/Language/JavascriptOperators.inl
@@ -62,6 +62,7 @@ namespace Js
     {
         Var nextItem = nullptr;
         bool shouldCallReturn = false;
+        JavascriptExceptionObject *exception = nullptr;
         try
         {
             while (JavascriptOperators::IteratorStepAndValue(iterator, scriptContext, &nextItem))
@@ -73,13 +74,17 @@ namespace Js
         }
         catch (const JavascriptException& err)
         {
-            JavascriptExceptionObject * exceptionObj = err.GetAndClear();
+            exception = err.GetAndClear();
+        }
+
+        if (exception != nullptr)
+        {
             if (shouldCallReturn)
             {
                 // Closing the iterator
                 JavascriptOperators::IteratorClose(iterator, scriptContext);
             }
-            JavascriptExceptionOperators::DoThrow(exceptionObj, scriptContext);
+            JavascriptExceptionOperators::DoThrowCheckClone(exception, scriptContext);
         }
     }
 

diff --git a/lib/Runtime/Library/JavascriptGenerator.cpp b/lib/Runtime/Library/JavascriptGenerator.cpp
@@ -126,6 +126,8 @@ namespace Js
             Var thunkArgs[] = { this, yieldData };
             Arguments arguments(_countof(thunkArgs), thunkArgs);
 
+            JavascriptExceptionObject *exception = nullptr;
+
             try
             {
                 BEGIN_SAFE_REENTRANT_CALL(scriptContext->GetThreadContext())
@@ -137,12 +139,16 @@ namespace Js
             }
             catch (const JavascriptException& err)
             {
-                Js::JavascriptExceptionObject* exceptionObj = err.GetAndClear();
-                if (!exceptionObj->IsGeneratorReturnException())
+                exception = err.GetAndClear();
+            }
+
+            if (exception != nullptr)
+            {
+                if (!exception->IsGeneratorReturnException())
                 {
-                    JavascriptExceptionOperators::DoThrow(exceptionObj, scriptContext);
+                    JavascriptExceptionOperators::DoThrowCheckClone(exception, scriptContext);
                 }
-                result = exceptionObj->GetThrownObject(nullptr);
+                result = exception->GetThrownObject(nullptr);
             }
         }
 

diff --git a/test/Bugs/misc_bugs.js b/test/Bugs/misc_bugs.js
@@ -174,7 +174,22 @@ var tests = [
         assert.throws(() => {"use strict"; let p1 = new Proxy({}, { deleteProperty() {return false;}}); delete p1.foo;}, TypeError, "deleteProperty handler is returning false which will throw type error",  "Proxy deleteProperty handler returned false");
       }
   },
-
+  {
+    name: "Generator : testing recursion",
+    body: function () {
+        // This will throw out of stack error
+        assert.throws(() => {
+            function foo() {
+                function *f() {
+                    yield foo();
+                }
+                f().next();
+            }
+            foo();
+        });
+      }
+  },
+
 ];
 
 testRunner.runTests(tests, { verbose: WScript.Arguments[0] != "summary" });