Bug 1333099 - Fix crash when incorrectly access EventSourceImpl::mEventSource. r=baku, a=lizzard
authorStone Shih <sshih@mozilla.com>
Tue, 24 Jan 2017 10:26:30 +0800
changeset 376571 329cd3f42a3355d27a83cf83ea54834f26e81e8d
parent 376570 f4263d463641a484ed26d0202d198500fdb464fb
child 376572 1122e037842cb908323ffa63c321b8dc6a72b111
push id6996
push userjlorenzo@mozilla.com
push dateMon, 06 Mar 2017 20:48:21 +0000
treeherdermozilla-beta@d89512dab048 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersbaku, lizzard
bugs1333099
milestone53.0a2
Bug 1333099 - Fix crash when incorrectly access EventSourceImpl::mEventSource. r=baku, a=lizzard MozReview-Commit-ID: K8nepp9G1Ho
dom/base/EventSource.cpp
--- a/dom/base/EventSource.cpp
+++ b/dom/base/EventSource.cpp
@@ -154,29 +154,30 @@ public:
 
   bool IsTargetThread() const
   {
     return NS_IsMainThread() == mIsMainThread;
   }
 
   uint16_t ReadyState()
   {
+    MutexAutoLock lock(mMutex);
     if (mEventSource) {
-      MutexAutoLock lock(mMutex);
       return mEventSource->mReadyState;
     }
     // EventSourceImpl keeps EventSource alive. If mEventSource is null, it
     // means that the EventSource has been closed.
     return CLOSED;
   }
 
   void SetReadyState(uint16_t aReadyState)
   {
+    MutexAutoLock lock(mMutex);
     MOZ_ASSERT(mEventSource);
-    MutexAutoLock lock(mMutex);
+    MOZ_ASSERT(!mIsShutDown);
     mEventSource->mReadyState = aReadyState;
   }
 
   bool IsFrozen()
   {
     MutexAutoLock lock(mMutex);
     return mFrozen;
   }
@@ -187,16 +188,29 @@ public:
     mFrozen = aFrozen;
   }
 
   bool IsClosed()
   {
     return ReadyState() == CLOSED;
   }
 
+  void ShutDown()
+  {
+    MutexAutoLock lock(mMutex);
+    MOZ_ASSERT(!mIsShutDown);
+    mIsShutDown = true;
+  }
+
+  bool IsShutDown()
+  {
+    MutexAutoLock lock(mMutex);
+    return mIsShutDown;
+  }
+
   RefPtr<EventSource> mEventSource;
 
   /**
    * A simple state machine used to manage the event-source's line buffer
    *
    * PARSE_STATE_OFF              -> PARSE_STATE_BEGIN_OF_STREAM
    *
    * PARSE_STATE_BEGIN_OF_STREAM  -> PARSE_STATE_BOM_WAS_READ |
@@ -288,17 +302,17 @@ public:
   // Whether the window is frozen. May be set on main thread and read on target
   // thread. Use mMutex to protect it before accessing it.
   bool mFrozen;
   // There are some messages are going to be dispatched when thaw.
   bool mGoingToDispatchAllMessages;
   // Whether the EventSource is run on main thread.
   bool mIsMainThread;
   // Whether the EventSourceImpl is going to be destroyed.
-  bool mIsClosing;
+  bool mIsShutDown;
 
   // Event Source owner information:
   // - the script file name
   // - source code line number and column number where the Event Source object
   //   was constructed.
   // - the ID of the inner window where the script lives. Note that this may not
   //   be the same as the Event Source owner window.
   // These attributes are used for error reporting. Should only be accessed on
@@ -337,17 +351,17 @@ EventSourceImpl::EventSourceImpl(EventSo
   : mEventSource(aEventSource)
   , mReconnectionTime(0)
   , mStatus(PARSE_STATE_OFF)
   , mLastConvertionResult(NS_OK)
   , mMutex("EventSourceImpl::mMutex")
   , mFrozen(false)
   , mGoingToDispatchAllMessages(false)
   , mIsMainThread(NS_IsMainThread())
-  , mIsClosing(false)
+  , mIsShutDown(false)
   , mScriptLine(0)
   , mScriptColumn(0)
   , mInnerWindowID(0)
 {
   MOZ_ASSERT(mEventSource);
   if (!mIsMainThread) {
     mWorkerPrivate = GetCurrentThreadWorkerPrivate();
     MOZ_ASSERT(mWorkerPrivate);
@@ -393,47 +407,53 @@ EventSourceImpl::Close()
   MOZ_ASSERT(NS_SUCCEEDED(rv));
 }
 
 void
 EventSourceImpl::CloseInternal()
 {
   AssertIsOnTargetThread();
   MOZ_ASSERT(IsClosed());
-  if (mIsClosing) {
+  if (IsShutDown()) {
     return;
   }
-  mIsClosing = true;
-  while (mMessagesToDispatch.GetSize() != 0) {
-    delete static_cast<Message*>(mMessagesToDispatch.PopFront());
-  }
 
+  // Invoke CleanupOnMainThread before cleaning any members. It will call
+  // ShutDown, which is supposed to be called before cleaning any members.
   if (NS_IsMainThread()) {
     CleanupOnMainThread();
   } else {
     ErrorResult rv;
     // run CleanupOnMainThread synchronously on main thread since it touches
     // observers and members only can be accessed on main thread.
     RefPtr<CleanupRunnable> runnable = new CleanupRunnable(this);
     runnable->Dispatch(Killing, rv);
     MOZ_ASSERT(!rv.Failed());
     UnregisterWorkerHolder();
   }
 
+  while (mMessagesToDispatch.GetSize() != 0) {
+    delete static_cast<Message*>(mMessagesToDispatch.PopFront());
+  }
   SetFrozen(false);
   ResetDecoder();
   mUnicodeDecoder = nullptr;
   // UpdateDontKeepAlive() can release the object. Don't access to any members
   // after it.
   mEventSource->UpdateDontKeepAlive();
 }
 
 void EventSourceImpl::CleanupOnMainThread()
 {
   AssertIsOnMainThread();
+  MOZ_ASSERT(IsClosed());
+
+  // Call ShutDown before cleaning any members.
+  ShutDown();
+
   if (mIsMainThread) {
     RemoveWindowObservers();
   }
 
   if (mTimer) {
     mTimer->Cancel();
     mTimer = nullptr;
   }
@@ -485,16 +505,17 @@ protected:
   const nsAString& mURL;
   nsresult mRv;
 };
 
 nsresult
 EventSourceImpl::ParseURL(const nsAString& aURL)
 {
   AssertIsOnMainThread();
+  MOZ_ASSERT(!IsShutDown());
   // get the src
   nsCOMPtr<nsIURI> baseURI;
   nsresult rv = GetBaseURI(getter_AddRefs(baseURI));
   NS_ENSURE_SUCCESS(rv, rv);
 
   nsCOMPtr<nsIURI> srcURI;
   rv = NS_NewURI(getter_AddRefs(srcURI), aURL, nullptr, baseURI);
   NS_ENSURE_SUCCESS(rv, NS_ERROR_DOM_SYNTAX_ERR);
@@ -513,16 +534,17 @@ EventSourceImpl::ParseURL(const nsAStrin
   return NS_OK;
 }
 
 nsresult
 EventSourceImpl::AddWindowObservers()
 {
   AssertIsOnMainThread();
   MOZ_ASSERT(mIsMainThread);
+  MOZ_ASSERT(!IsShutDown());
   nsCOMPtr<nsIObserverService> os = mozilla::services::GetObserverService();
   NS_ENSURE_STATE(os);
 
   nsresult rv = os->AddObserver(this, DOM_WINDOW_DESTROYED_TOPIC, true);
   NS_ENSURE_SUCCESS(rv, rv);
   rv = os->AddObserver(this, DOM_WINDOW_FROZEN_TOPIC, true);
   NS_ENSURE_SUCCESS(rv, rv);
   rv = os->AddObserver(this, DOM_WINDOW_THAWED_TOPIC, true);
@@ -530,34 +552,33 @@ EventSourceImpl::AddWindowObservers()
   return NS_OK;
 }
 
 void
 EventSourceImpl::RemoveWindowObservers()
 {
   AssertIsOnMainThread();
   MOZ_ASSERT(mIsMainThread);
+  MOZ_ASSERT(IsClosed());
   nsCOMPtr<nsIObserverService> os = mozilla::services::GetObserverService();
   if (os) {
     os->RemoveObserver(this, DOM_WINDOW_DESTROYED_TOPIC);
     os->RemoveObserver(this, DOM_WINDOW_FROZEN_TOPIC);
     os->RemoveObserver(this, DOM_WINDOW_THAWED_TOPIC);
   }
 }
 
 void
 EventSourceImpl::Init(nsIPrincipal* aPrincipal,
                       const nsAString& aURL,
                       ErrorResult& aRv)
 {
   AssertIsOnMainThread();
   MOZ_ASSERT(aPrincipal);
-  if (IsClosed()) {
-    return;
-  }
+  MOZ_ASSERT(ReadyState() == CONNECTING);
   mPrincipal = aPrincipal;
   aRv = ParseURL(aURL);
   if (NS_WARN_IF(aRv.Failed())) {
     return;
   }
   // The conditional here is historical and not necessarily sane.
   if (JSContext* cx = nsContentUtils::GetCurrentJSContext()) {
     nsJSUtils::GetCallingLocation(cx, mScriptFile, &mScriptLine,
@@ -689,16 +710,17 @@ EventSourceImpl::StreamReaderFunc(nsIInp
                                   uint32_t* aWriteCount)
 {
   EventSourceImpl* thisObject = static_cast<EventSourceImpl*>(aClosure);
   thisObject->AssertIsOnTargetThread();
   if (!thisObject || !aWriteCount) {
     NS_WARNING("EventSource cannot read from stream: no aClosure or aWriteCount");
     return NS_ERROR_FAILURE;
   }
+  MOZ_ASSERT(!thisObject->IsShutDown());
   thisObject->ParseSegment((const char*)aFromRawSegment, aCount);
   *aWriteCount = aCount;
   return NS_OK;
 }
 
 void
 EventSourceImpl::ParseSegment(const char* aBuffer, uint32_t aLength)
 {
@@ -841,16 +863,19 @@ EventSourceImpl::OnStopRequest(nsIReques
 
 NS_IMETHODIMP
 EventSourceImpl::AsyncOnChannelRedirect(nsIChannel* aOldChannel,
                                         nsIChannel* aNewChannel,
                                         uint32_t aFlags,
                                         nsIAsyncVerifyRedirectCallback* aCallback)
 {
   AssertIsOnMainThread();
+  if (IsClosed()) {
+    return NS_ERROR_ABORT;
+  }
   nsCOMPtr<nsIRequest> aOldRequest = do_QueryInterface(aOldChannel);
   NS_PRECONDITION(aOldRequest, "Redirect from a null request?");
 
   nsresult rv = CheckHealthOfRequestCallback(aOldRequest);
   NS_ENSURE_SUCCESS(rv, rv);
 
   NS_PRECONDITION(aNewChannel, "Redirect without a channel?");
 
@@ -890,16 +915,21 @@ EventSourceImpl::AsyncOnChannelRedirect(
 //-----------------------------------------------------------------------------
 // EventSourceImpl::nsIInterfaceRequestor
 //-----------------------------------------------------------------------------
 
 NS_IMETHODIMP
 EventSourceImpl::GetInterface(const nsIID& aIID, void** aResult)
 {
   AssertIsOnMainThread();
+
+  if (IsClosed()) {
+    return NS_ERROR_FAILURE;
+  }
+
   if (aIID.Equals(NS_GET_IID(nsIChannelEventSink))) {
     *aResult = static_cast<nsIChannelEventSink*>(this);
     NS_ADDREF_THIS();
     return NS_OK;
   }
 
   if (aIID.Equals(NS_GET_IID(nsIAuthPrompt)) ||
       aIID.Equals(NS_GET_IID(nsIAuthPrompt2))) {
@@ -930,16 +960,17 @@ EventSourceImpl::IsOnCurrentThread(bool*
   *aResult = IsTargetThread();
   return NS_OK;
 }
 
 nsresult
 EventSourceImpl::GetBaseURI(nsIURI** aBaseURI)
 {
   AssertIsOnMainThread();
+  MOZ_ASSERT(!IsShutDown());
   NS_ENSURE_ARG_POINTER(aBaseURI);
 
   *aBaseURI = nullptr;
 
   nsCOMPtr<nsIURI> baseURI;
 
   // first we try from document->GetBaseURI()
   nsCOMPtr<nsIDocument> doc = mEventSource->GetDocumentIfCurrent();
@@ -958,16 +989,17 @@ EventSourceImpl::GetBaseURI(nsIURI** aBa
   baseURI.forget(aBaseURI);
   return NS_OK;
 }
 
 void
 EventSourceImpl::SetupHttpChannel()
 {
   AssertIsOnMainThread();
+  MOZ_ASSERT(!IsShutDown());
   mHttpChannel->SetRequestMethod(NS_LITERAL_CSTRING("GET"));
 
   /* set the http request headers */
 
   mHttpChannel->SetRequestHeader(NS_LITERAL_CSTRING("Accept"),
     NS_LITERAL_CSTRING(TEXT_EVENT_STREAM), false);
 
   // LOAD_BYPASS_CACHE already adds the Cache-Control: no-cache header
@@ -977,16 +1009,17 @@ EventSourceImpl::SetupHttpChannel()
       NS_ConvertUTF16toUTF8(mLastEventID), false);
   }
 }
 
 nsresult
 EventSourceImpl::SetupReferrerPolicy()
 {
   AssertIsOnMainThread();
+  MOZ_ASSERT(!IsShutDown());
   nsCOMPtr<nsIDocument> doc = mEventSource->GetDocumentIfCurrent();
   if (doc) {
     nsresult rv = mHttpChannel->SetReferrerWithPolicy(doc->GetDocumentURI(),
                                                       doc->GetReferrerPolicy());
     NS_ENSURE_SUCCESS(rv, rv);
   }
 
   return NS_OK;
@@ -1147,16 +1180,19 @@ protected:
   // Raw pointer because this runnable is sync.
   EventSourceImpl* mImpl;
 };
 
 nsresult
 EventSourceImpl::RestartConnection()
 {
   AssertIsOnMainThread();
+  if (IsClosed()) {
+    return NS_ERROR_ABORT;
+  }
   nsresult rv = ResetConnection();
   NS_ENSURE_SUCCESS(rv, rv);
   rv = SetReconnectionTimeout();
   NS_ENSURE_SUCCESS(rv, rv);
   return NS_OK;
 }
 
 void
@@ -1219,16 +1255,17 @@ EventSourceImpl::SetReconnectionTimeout(
 
 nsresult
 EventSourceImpl::PrintErrorOnConsole(const char* aBundleURI,
                                      const char16_t* aError,
                                      const char16_t** aFormatStrings,
                                      uint32_t aFormatStringsLen)
 {
   AssertIsOnMainThread();
+  MOZ_ASSERT(!IsShutDown());
   nsCOMPtr<nsIStringBundleService> bundleService =
     mozilla::services::GetStringBundleService();
   NS_ENSURE_STATE(bundleService);
 
   nsCOMPtr<nsIStringBundle> strBundle;
   nsresult rv =
     bundleService->CreateBundle(aBundleURI, getter_AddRefs(strBundle));
   NS_ENSURE_SUCCESS(rv, rv);
@@ -1266,16 +1303,17 @@ EventSourceImpl::PrintErrorOnConsole(con
 
   return NS_OK;
 }
 
 nsresult
 EventSourceImpl::ConsoleError()
 {
   AssertIsOnMainThread();
+  MOZ_ASSERT(!IsShutDown());
   nsAutoCString targetSpec;
   nsresult rv = mSrc->GetSpec(targetSpec);
   NS_ENSURE_SUCCESS(rv, rv);
 
   NS_ConvertUTF8toUTF16 specUTF16(targetSpec);
   const char16_t* formatStrings[] = { specUTF16.get() };
 
   if (ReadyState() == CONNECTING) {
@@ -1396,16 +1434,17 @@ EventSourceImpl::Freeze()
   SetFrozen(true);
   return NS_OK;
 }
 
 nsresult
 EventSourceImpl::DispatchCurrentMessageEvent()
 {
   AssertIsOnTargetThread();
+  MOZ_ASSERT(!IsShutDown());
   nsAutoPtr<Message> message(new Message());
   *message = mCurrentMessage;
 
   ClearFields();
 
   if (message->mData.IsEmpty()) {
     return NS_OK;
   }
@@ -1524,16 +1563,17 @@ EventSourceImpl::ClearFields()
   mLastFieldValue.Truncate();
 
   return NS_OK;
 }
 
 nsresult
 EventSourceImpl::SetFieldAndClear()
 {
+  MOZ_ASSERT(!IsShutDown());
   AssertIsOnTargetThread();
   if (mLastFieldName.IsEmpty()) {
     mLastFieldValue.Truncate();
     return NS_OK;
   }
 
   char16_t first_char;
   first_char = mLastFieldName.CharAt(0);
@@ -1851,16 +1891,17 @@ public:
 private:
   nsCOMPtr<nsIRunnable> mEvent;
 };
 
 } // namespace
 
 bool EventSourceImpl::RegisterWorkerHolder()
 {
+  MOZ_ASSERT(!IsShutDown());
   MOZ_ASSERT(mWorkerPrivate);
   mWorkerPrivate->AssertIsOnWorkerThread();
   MOZ_ASSERT(!mWorkerHolder);
   mWorkerHolder = new EventSourceWorkerHolder(this);
   if (NS_WARN_IF(!mWorkerHolder->HoldWorker(mWorkerPrivate, Canceling))) {
     mWorkerHolder = nullptr;
     return false;
   }
@@ -1891,16 +1932,20 @@ EventSourceImpl::DispatchFromScript(nsIR
 
 NS_IMETHODIMP
 EventSourceImpl::Dispatch(already_AddRefed<nsIRunnable> aEvent, uint32_t aFlags)
 {
   nsCOMPtr<nsIRunnable> event_ref(aEvent);
   if (mIsMainThread) {
     return NS_DispatchToMainThread(event_ref.forget());
   }
+
+  if (IsShutDown()) {
+    return NS_OK;
+  }
   MOZ_ASSERT(mWorkerPrivate);
   // If the target is a worker, we have to use a custom WorkerRunnableDispatcher
   // runnable.
   RefPtr<WorkerRunnableDispatcher> event =
     new WorkerRunnableDispatcher(this, mWorkerPrivate, event_ref.forget());
 
   if (!event->Dispatch()) {
     return NS_ERROR_FAILURE;