Commit 8b884c80 authored by Kris Maglione's avatar Kris Maglione
Browse files

Bug 1399646: Part 2 - Use the async shutdown service for ServiceWorkerRegistrar. r=baku 

The current shutdown handling code is susceptible to deadlocks, since it spins
the event loop while it holds mMonitor, and other main thread methods which
try to acquire mMonitor can be called from code that runs while the event loop
is spinning.

My initial solution was just to release mMonitor before spinning the event
loop, but at this point I think it makes more sense to switch to the
standardized AsyncShutdown routines, which provide better diagnostics and
allow us to avoid one more nested event loop during shutdown.

MozReview-Commit-ID: 1RtFN585IR7

--HG--
extra : rebase_source : 978f3bf7cef73e56b3e1c1c838c2bc6efcefb0c0
extra : amend_source : 2b7c9422004b58ad1d38d7dd705ad446bc47cb23
extra : histedit_source : 7a4e80de7d5aa48e074ea03821bb78e5e287842e%2C92c1119a131adaa33f5691c0e534bb243115817b
parent 2d3eb6ce

browser/base/content/test/performance/browser_startup.js

+0 −3
Original line number Diff line number Diff line
@@ -49,9 +49,6 @@ const startupPhases = { 


  // We are at this phase after creating the first browser window (ie. after final-ui-startup).

  "before opening first browser window": {blacklist: {

    components: new Set([

      "nsAsyncShutdown.js",

    ]),

    modules: new Set([

      "resource://gre/modules/PlacesBackups.jsm",

      "resource://gre/modules/PlacesUtils.jsm",

dom/workers/ServiceWorkerRegistrar.cpp

+44 −23
Original line number Diff line number Diff line
@@ -54,7 +54,8 @@ StaticRefPtr<ServiceWorkerRegistrar> gServiceWorkerRegistrar; 
} // namespace



NS_IMPL_ISUPPORTS(ServiceWorkerRegistrar,

                  nsIObserver)

                  nsIObserver,

                  nsIAsyncShutdownBlocker)



void

ServiceWorkerRegistrar::Initialize()
@@ -73,10 +74,6 @@ ServiceWorkerRegistrar::Initialize() 
    DebugOnly<nsresult> rv = obs->AddObserver(gServiceWorkerRegistrar,

                                              "profile-after-change", false);

    MOZ_ASSERT(NS_SUCCEEDED(rv));



    rv = obs->AddObserver(gServiceWorkerRegistrar, "profile-before-change",

                          false);

    MOZ_ASSERT(NS_SUCCEEDED(rv));

  }

}
@@ -94,7 +91,6 @@ ServiceWorkerRegistrar::ServiceWorkerRegistrar() 
  : mMonitor("ServiceWorkerRegistrar.mMonitor")

  , mDataLoaded(false)

  , mShuttingDown(false)

  , mShutdownCompleteFlag(nullptr)

  , mRunnableCounter(0)

{

  MOZ_ASSERT(NS_IsMainThread());
@@ -829,8 +825,8 @@ ServiceWorkerRegistrar::ShutdownCompleted() 
{

  MOZ_ASSERT(NS_IsMainThread());



  MOZ_ASSERT(mShutdownCompleteFlag && !*mShutdownCompleteFlag);

  *mShutdownCompleteFlag = true;

  DebugOnly<nsresult> rv = GetShutdownPhase()->RemoveBlocker(this);

  MOZ_ASSERT(NS_SUCCEEDED(rv));

}



void
@@ -1027,6 +1023,13 @@ ServiceWorkerRegistrar::ProfileStarted() 
    return;

  }



  rv = GetShutdownPhase()->AddBlocker(

    this, NS_LITERAL_STRING(__FILE__), __LINE__,

    NS_LITERAL_STRING("ServiceWorkerRegistrar: Flushing data"));

  if (NS_WARN_IF(NS_FAILED(rv))) {

    return;

  }



  nsCOMPtr<nsIEventTarget> target =

    do_GetService(NS_STREAMTRANSPORTSERVICE_CONTRACTID);

  MOZ_ASSERT(target, "Must have stream transport service");
@@ -1061,12 +1064,41 @@ ServiceWorkerRegistrar::ProfileStopped() 
    return;

  }



  bool completed = false;

  mShutdownCompleteFlag = &completed;



  child->SendShutdownServiceWorkerRegistrar();

}



// Async shutdown blocker methods



NS_IMETHODIMP

ServiceWorkerRegistrar::BlockShutdown(nsIAsyncShutdownClient* aClient)

{

  ProfileStopped();

  return NS_OK;

}



NS_IMETHODIMP

ServiceWorkerRegistrar::GetName(nsAString& aName)

{

  aName = NS_LITERAL_STRING("ServiceWorkerRegistrar: Flushing data");

  return NS_OK;

}



NS_IMETHODIMP

ServiceWorkerRegistrar::GetState(nsIPropertyBag**)

{

  return NS_OK;

}



nsCOMPtr<nsIAsyncShutdownClient>

ServiceWorkerRegistrar::GetShutdownPhase() const

{

  nsCOMPtr<nsIAsyncShutdownService> svc = services::GetAsyncShutdown();

  MOZ_RELEASE_ASSERT(svc);



  MOZ_ALWAYS_TRUE(SpinEventLoopUntil([&]() { return completed; }));

  nsCOMPtr<nsIAsyncShutdownClient> client;

  Unused << svc->GetProfileBeforeChange(getter_AddRefs(client));

  MOZ_RELEASE_ASSERT(client);

  return Move(client);

}



void
@@ -1097,17 +1129,6 @@ ServiceWorkerRegistrar::Observe(nsISupports* aSubject, const char* aTopic, 
    return NS_OK;

  }



  if (!strcmp(aTopic, "profile-before-change")) {

    nsCOMPtr<nsIObserverService> observerService =

      services::GetObserverService();

    observerService->RemoveObserver(this, "profile-before-change");



    // Shutting down, let's sync the data.

    ProfileStopped();



    return NS_OK;

  }



  MOZ_ASSERT(false, "ServiceWorkerRegistrar got unexpected topic!");

  return NS_ERROR_UNEXPECTED;

}

dom/workers/ServiceWorkerRegistrar.h

+5 −1
Original line number Diff line number Diff line
@@ -10,6 +10,7 @@ 
#include "mozilla/Monitor.h"

#include "mozilla/Telemetry.h"

#include "nsClassHashtable.h"

#include "nsIAsyncShutdown.h"

#include "nsIObserver.h"

#include "nsCOMPtr.h"

#include "nsString.h"
@@ -34,12 +35,14 @@ namespace dom { 
class ServiceWorkerRegistrationData;



class ServiceWorkerRegistrar : public nsIObserver

                             , public nsIAsyncShutdownBlocker

{

  friend class ServiceWorkerRegistrarSaveDataRunnable;



public:

  NS_DECL_THREADSAFE_ISUPPORTS

  NS_DECL_NSIOBSERVER

  NS_DECL_NSIASYNCSHUTDOWNBLOCKER



  static void Initialize();
@@ -79,6 +82,8 @@ private: 
  void ShutdownCompleted();

  void MaybeScheduleShutdownCompleted();



  nsCOMPtr<nsIAsyncShutdownClient> GetShutdownPhase() const;



  bool IsSupportedVersion(const nsACString& aVersion) const;



  mozilla::Monitor mMonitor;
@@ -91,7 +96,6 @@ protected: 


  // PBackground thread only

  bool mShuttingDown;

  bool* mShutdownCompleteFlag;

  uint32_t mRunnableCounter;

};