Bug 19968: Slow Script at www.huffingtonpost.com

<https://bugs.webkit.org/show_bug.cgi?id=19968>

Reviewed by Maciej Stachowiak

Finally found the cause of this accursed issue.  It is triggered
by synchronous creation of a new global object from JS.  The new
global object resets the timer state in this execution group's
Machine, taking timerCheckCount to 0.  Then when JS returns the
timerCheckCount is decremented making it non-zero.  The next time
we execute JS we will start the timeout counter, however the non-zero
timeoutCheckCount means we don't reset the timer information. This
means that the timeout check is now checking the cumulative time
since the creation of the global object rather than the time since
JS was last entered.  At this point the slow script dialog is guaranteed
to eventually be displayed incorrectly unless a page is loaded
asynchronously (which will reset everything into a sane state).

The fix for this is rather trivial -- the JSGlobalObject constructor
should not be resetting the machine timer state.

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@36843 268f45cc-cd09-0410-ab3c-d52691b4dbfc

diff --git a/JavaScriptCore/ChangeLog b/JavaScriptCore/ChangeLog
index 2db0e17..d2640a3 100644 (file)
--- a/JavaScriptCore/ChangeLog
+++ b/JavaScriptCore/ChangeLog
@@ -1,3 +1,39 @@
+2008-09-23  Oliver Hunt  <oliver@apple.com>
+
+        Reviewed by Maciej Stachowiak.
+
+        Bug 19968: Slow Script at www.huffingtonpost.com
+        <https://bugs.webkit.org/show_bug.cgi?id=19968>
+
+        Finally found the cause of this accursed issue.  It is triggered
+        by synchronous creation of a new global object from JS.  The new
+        global object resets the timer state in this execution group's
+        Machine, taking timerCheckCount to 0.  Then when JS returns the
+        timerCheckCount is decremented making it non-zero.  The next time
+        we execute JS we will start the timeout counter, however the non-zero
+        timeoutCheckCount means we don't reset the timer information. This
+        means that the timeout check is now checking the cumulative time
+        since the creation of the global object rather than the time since
+        JS was last entered.  At this point the slow script dialog is guaranteed
+        to eventually be displayed incorrectly unless a page is loaded
+        asynchronously (which will reset everything into a sane state).
+
+        The fix for this is rather trivial -- the JSGlobalObject constructor
+        should not be resetting the machine timer state.
+
+        * VM/Machine.cpp:
+        (JSC::Machine::Machine):
+          Now that we can't rely on the GlobalObject initialising the timeout
+          state, we do it in the Machine constructor.
+
+        * VM/Machine.h:
+        (JSC::Machine::stopTimeoutCheck):
+          Add assertions to guard against this happening.
+
+        * kjs/JSGlobalObject.cpp:
+        (JSC::JSGlobalObject::init):
+          Don't reset the timeout state.
+
 2008-09-23  Geoffrey Garen  <ggaren@apple.com>
 
         Reviewed by Oliver Hunt.

diff --git a/JavaScriptCore/VM/Machine.cpp b/JavaScriptCore/VM/Machine.cpp
index e6bd699..8c7b624 100644 (file)
--- a/JavaScriptCore/VM/Machine.cpp
+++ b/JavaScriptCore/VM/Machine.cpp
@@ -635,6 +635,7 @@ Machine::Machine()
     , m_timeoutCheckCount(0)
     , m_ticksUntilNextTimeoutCheck(initialTickCountThreshold)
 {
+    initTimeout();
     privateExecute(InitializeAndReturn);
     
     // Bizarrely, calling fastMalloc here is faster than allocating space on the stack.

diff --git a/JavaScriptCore/VM/Machine.h b/JavaScriptCore/VM/Machine.h
index 5bb4d64..458955f 100644 (file)
--- a/JavaScriptCore/VM/Machine.h
+++ b/JavaScriptCore/VM/Machine.h
@@ -109,11 +109,13 @@ namespace JSC {
         
         void stopTimeoutCheck()
         {
+            ASSERT(m_timeoutCheckCount);
             --m_timeoutCheckCount;
         }
 
         inline void initTimeout()
         {
+            ASSERT(!m_timeoutCheckCount);
             resetTimeoutCheck();
             m_timeoutTime = 0;
             m_timeoutCheckCount = 0;

diff --git a/JavaScriptCore/kjs/JSGlobalObject.cpp b/JavaScriptCore/kjs/JSGlobalObject.cpp
index d9f3ad5..d902580 100644 (file)
--- a/JavaScriptCore/kjs/JSGlobalObject.cpp
+++ b/JavaScriptCore/kjs/JSGlobalObject.cpp
@@ -138,7 +138,6 @@ void JSGlobalObject::init(JSObject* thisValue)
 
     d()->recursion = 0;
     d()->debugger = 0;
-    globalData()->machine->initTimeout();
 
     d()->globalExec.set(new ExecState(this, thisValue, d()->globalScopeChain.node()));