From bb907f56a6bf307c2256c1cb7d1284e15bba0afa Mon Sep 17 00:00:00 2001
From: ClearScript <ClearScript@microsoft.com>
Date: Tue, 9 Jul 2013 12:02:52 -0400
Subject: [PATCH] Fixed script interruption crash in V8ScriptEngine.

---
 .../V8/ClearScriptV8/V8ContextImpl.cpp        | 13 ++++--
 ClearScriptTest/BugFixTest.cs                 | 46 ++++++++++++++++++-
 2 files changed, 54 insertions(+), 5 deletions(-)

diff --git a/ClearScript/V8/ClearScriptV8/V8ContextImpl.cpp b/ClearScript/V8/ClearScriptV8/V8ContextImpl.cpp
index bcd02a6f..b2c5a7ea 100644
--- a/ClearScript/V8/ClearScriptV8/V8ContextImpl.cpp
+++ b/ClearScript/V8/ClearScriptV8/V8ContextImpl.cpp
@@ -1333,11 +1333,16 @@ Handle<Value> V8ContextImpl::ImportValue(const V8Value& value)
                 return CreateLocal(::ObjectHandleFromPtr(pvV8Object));
             }
 
-            auto hObject = m_hHostObjectTemplate->InstanceTemplate()->NewInstance();
-            ::SetHostObjectHolder(hObject, pHolder->Clone());
+            // WARNING: Instantiation may fail during script interruption. Check NewInstance()
+            // result to avoid access violations and V8 fatal errors in ::SetObjectHolder().
 
-            pvV8Object = ::PtrFromObjectHandle(MakeWeak(CreatePersistent(hObject), HostObjectHelpers::AddRef(m_pvV8ObjectCache), DisposeWeakHandle));
-            HostObjectHelpers::CacheV8Object(m_pvV8ObjectCache, pHolder->GetObject(), pvV8Object);
+            auto hObject = m_hHostObjectTemplate->InstanceTemplate()->NewInstance();
+            if (!hObject.IsEmpty())
+            {
+                ::SetHostObjectHolder(hObject, pHolder->Clone());
+                pvV8Object = ::PtrFromObjectHandle(MakeWeak(CreatePersistent(hObject), HostObjectHelpers::AddRef(m_pvV8ObjectCache), DisposeWeakHandle));
+                HostObjectHelpers::CacheV8Object(m_pvV8ObjectCache, pHolder->GetObject(), pvV8Object);
+            }
 
             return hObject;
         }
diff --git a/ClearScriptTest/BugFixTest.cs b/ClearScriptTest/BugFixTest.cs
index b2362bd9..f5c67819 100644
--- a/ClearScriptTest/BugFixTest.cs
+++ b/ClearScriptTest/BugFixTest.cs
@@ -61,6 +61,7 @@
 
 using System;
 using System.Diagnostics.CodeAnalysis;
+using System.Threading;
 using Microsoft.CSharp.RuntimeBinder;
 using Microsoft.ClearScript.V8;
 using Microsoft.ClearScript.Windows;
@@ -131,7 +132,7 @@ namespace Microsoft.ClearScript.Test
         }
 
         [TestMethod, TestCategory("BugFix")]
-        public void BugFix_JScriptCaseInsensitivity()
+        public void BugFix_JScript_CaseInsensitivity()
         {
             engine.Dispose();
             engine = new JScriptEngine();
@@ -142,6 +143,49 @@ namespace Microsoft.ClearScript.Test
             Assert.AreEqual(4, engine.Script.FOO());
         }
 
+        [TestMethod, TestCategory("BugFix")]
+        public void BugFix_V8_ScriptInterruptCrash()
+        {
+            // A V8 fatal error on a background thread may not kill the process, so a single run is
+            // inconclusive. It will kill the V8 runtime, however, causing subsequent runs to fail.
+
+            for (var iteration = 0; iteration < 16; iteration++)
+            {
+                var context = new PropertyBag();
+                engine.AddHostObject("context", context);
+
+                var startEvent = new ManualResetEventSlim(false);
+                var thread = new Thread(() =>
+                {
+                    context["startEvent"] = startEvent;
+                    context["counter"] = 0;
+
+                    try
+                    {
+                        engine.Execute(
+                        @"
+                            for (var i = 0; i < 10000000; i++ )
+                            {
+                                context.counter++;
+                                context.startEvent.Set();
+                            }
+                        ");
+                    }
+                    catch (ScriptInterruptedException)
+                    {
+                    }
+                });
+
+                thread.Start();
+                startEvent.Wait();
+                engine.Interrupt();
+                thread.Join();
+
+                var counter = (int)context["counter"];
+                Assert.IsTrue((counter > 0) && (counter < 10000000));
+            }
+        }
+
         // ReSharper restore InconsistentNaming
 
         #endregion