Bug 364315 - Fix threadsafety assertions and crashes by only releasing documents on the main thread and not starting two speculative parsers for the same nsParser. r+sr=jst
authorBlake Kaplan <mrbkap@gmail.com>
Wed, 01 Oct 2008 17:09:21 -0700
changeset 20068 4ed163dabf935b9fca474c8de37c901929655ccb
parent 20067 d07aa0d0712a93c8c5d988fd99dd1fc8d7c5839b
child 20070 a1c4e010b83f60661de8d80d525770809cedec6f
push idunknown
push userunknown
push dateunknown
bugs364315
milestone1.9.1b1pre
Bug 364315 - Fix threadsafety assertions and crashes by only releasing documents on the main thread and not starting two speculative parsers for the same nsParser. r+sr=jst
parser/htmlparser/src/nsParser.cpp
--- a/parser/htmlparser/src/nsParser.cpp
+++ b/parser/htmlparser/src/nsParser.cpp
@@ -205,16 +205,18 @@ public:
       mKeepParsing(0),
       mCurrentlyParsing(0),
       mNumURIs(0),
       mNumConsumed(0),
       mTerminated(PR_FALSE) {
   }
 
   ~nsSpeculativeScriptThread() {
+    NS_ASSERTION(NS_IsMainThread() || !mDocument,
+                 "Destroying the document on the wrong thread");
   }
 
   NS_DECL_ISUPPORTS
   NS_DECL_NSIRUNNABLE
 
   nsresult StartParsing(nsParser *aParser);
   void StopParsing(PRBool aFromDocWrite);
 
@@ -505,19 +507,26 @@ nsSpeculativeScriptThread::StopParsing(P
 
     mKeepParsing = 0;
     if (mCurrentlyParsing) {
       PR_WaitCondVar(mCVar.get(), PR_INTERVAL_NO_TIMEOUT);
       NS_ASSERTION(!mCurrentlyParsing, "Didn't actually stop parsing?");
     }
   }
 
-  // The thread is now idle. It is now safe to touch mContext on the main
-  // thread.
-  if (!mTerminated && mNumURIs) {
+  // The thread is now idle.
+  if (mTerminated) {
+    // If we're terminated, then we need to ensure that we release our document
+    // and tokenizer here on the main thread so that our last reference to them
+    // isn't our alter-ego rescheduled on another thread.
+    mDocument = nsnull;
+    mTokenizer = nsnull;
+    mScanner = nsnull;
+  } else if (mNumURIs) {
+    // Note: Don't do this if we're terminated.
     nsPreloadURIs::PreloadURIs(mURIs, this);
     mNumURIs = 0;
     mURIs.Clear();
   }
 
   // Note: Currently, we pop the tokens off (see the comment in Run) so this
   // isn't a problem. If and when we actually use the tokens created
   // off-thread, we'll need to use aFromDocWrite for real.
@@ -2168,20 +2177,20 @@ nsParser::ResumeParse(PRBool allowIterat
 {
   nsresult result = NS_OK;
 
   if ((mFlags & NS_PARSER_FLAG_PARSER_ENABLED) &&
       mInternalState != NS_ERROR_HTMLPARSER_STOPPARSING) {
     MOZ_TIMER_DEBUGLOG(("Start: Parse Time: nsParser::ResumeParse(), this=%p\n", this));
     MOZ_TIMER_START(mParseTime);
 
-    if (mSpeculativeScriptThread) {
-      mSpeculativeScriptThread->StopParsing(PR_FALSE);
-    }
-
+    NS_ASSERTION(!mSpeculativeScriptThread || !mSpeculativeScriptThread->Parsing(),
+                 "Bad races happening, expect to crash!");
+
+    CParserContext *originalContext = mParserContext;
     result = WillBuildModel(mParserContext->mScanner->GetFilename());
     if (NS_FAILED(result)) {
       mFlags &= ~NS_PARSER_FLAG_CAN_TOKENIZE;
       return result;
     }
 
     if (mParserContext->mDTD) {
       mParserContext->mDTD->WillResumeParse(mSink);
@@ -2221,17 +2230,24 @@ nsParser::ResumeParse(PRBool allowIterat
         // If we're told to block the parser, we disable all further parsing
         // (and cache any data coming in) until the parser is re-enabled.
         if (NS_ERROR_HTMLPARSER_BLOCK == result) {
           if (mParserContext->mDTD) {
             mParserContext->mDTD->WillInterruptParse(mSink);
           }
 
           BlockParser();
-          SpeculativelyParse();
+
+          // If our context has changed, then someone did a document.write of
+          // an asynchronous script that blocked a sub context. Since *that*
+          // block already might have started a speculative parse, we don't
+          // have to.
+          if (mParserContext == originalContext) {
+            SpeculativelyParse();
+          }
           return NS_OK;
         }
         if (NS_ERROR_HTMLPARSER_STOPPARSING == result) {
           // Note: Parser Terminate() calls DidBuildModel.
           if (mInternalState != NS_ERROR_HTMLPARSER_STOPPARSING) {
             DidBuildModel(mStreamStatus);
             mInternalState = result;
           }