Prevent clearing of closure environment from live eval function object. We clear the environment from cached eval functions to avoid pinning captured variables. But if the same function is fetched from the cache while another instance of the same eval is executing, then when the newer invocation returns we clear the environment out from under the instance that is still running. Fix this by making sure we get use a different function object to execute a direct eval if there is already a running instance, indicated by a non-null environment on the function.

lib/Runtime/Library/GlobalObject.cpp

@@ -596,7 +596,8 @@ namespace Js
         char16 const * sourceString = argString->GetSz();
         charcount_t sourceLen = argString->GetLength();
         FastEvalMapString key(sourceString, sourceLen, moduleID, strictMode, isLibraryCode);
-        if (!scriptContext->IsInEvalMap(key, isIndirect, &pfuncScript))
+        bool found = scriptContext->IsInEvalMap(key, isIndirect, &pfuncScript);
+        if (!found || (!isIndirect && pfuncScript->GetEnvironment() != &NullFrameDisplay))
         {
             uint32 grfscr = additionalGrfscr | fscrReturnExpression | fscrEval | fscrEvalCode | fscrGlobalCode;
 
@@ -608,7 +609,10 @@ namespace Js
             pfuncScript = library->GetGlobalObject()->EvalHelper(scriptContext, argString->GetSz(), argString->GetLength(), moduleID,
                 grfscr, Constants::EvalCode, doRegisterDocument, isIndirect, strictMode);
 
-            scriptContext->AddToEvalMap(key, isIndirect, pfuncScript);
+            if (!found)
+            {
+                scriptContext->AddToEvalMap(key, isIndirect, pfuncScript);
+            }
         }
 
 #ifdef ENABLE_DEBUG_CONFIG_OPTIONS
@@ -743,7 +747,7 @@ namespace Js
             pfuncScript->InvalidateCachedScopeChain();
         }
         Var varResult = CALL_FUNCTION(scriptContext->GetThreadContext(), pfuncScript, CallInfo(CallFlags_Eval, 1), varThis);
-        pfuncScript->SetEnvironment(nullptr);
+        pfuncScript->SetEnvironment((FrameDisplay*)&NullFrameDisplay);
         return varResult;
     }
 

test/GlobalFunctions/eval1.js

@@ -110,6 +110,20 @@ alias();
 // bug 1147044
 eval("with ({}) (function fibonacci() {})();"); 
 
+// Test recursive evals to make sure closure environments remain intact
+var flg = 0;
+function TestDirect() {
+  var func = "if(flg == 0) { flg = 1; eval(func); (function(a){(function(){if (a !== undefined) throw 0;})()})(); WScript.Echo('pass direct')}";
+  eval(func);
+}
+TestDirect();
+
+var func = "if(flg == 1) { flg = 2; this.eval(func); (function(a){(function(){if (a !== undefined) throw 0;})()})(); WScript.Echo('pass indirect');}";
+function TestIndirect() {
+  this.eval(func);
+}
+TestIndirect();
+
 // 8. Set up a custom eval that indirectly calls built-in eval, evoke it, and verify the effect.
 var q = eval;
 var eval = function(s) {

test/GlobalFunctions/eval1_v3.baseline

@@ -21,6 +21,8 @@ inside eval: this.name = global object
 42
 42
 [object Object]
+pass direct
+pass indirect
 Custom eval:
 arg 0 = 'x[i][0] = 2;'
 2