Bug 1338280 - Use generation numbers to allow multiple Places restarts.
In non-test code, the database is a singleton initialized at startup,
and torn down on shutdown. This doesn't play well with tests that need
to restart the database multiple times. To make these tests work, we
use a "generation number": a counter that's incremented in lockstep
between the database and its two shutdown blockers.
The blockers maintain a corresponding counter that's incremented when
shutdown begins. At that point,
`Database::sGeneration < PlacesShutdownBlocker::sGeneration`, so we
know the current database must be shutting down. When the current
database is destroyed, we bump the database generation number for the
next restart.
During shutdown, `ConnectionShutdownBlocker` also fires two observer
notifications, which are exposed as `AsyncShutdown` phases. These
phases aren't designed to fire more than once, so we introduce a
per-database tag passed as the `data` parameter to the observer. In the
next patch, we'll change `AsyncShutdown` to support phases with tags.
MozReview-Commit-ID: IXPyrfe5qOf
--- a/toolkit/components/places/Database.cpp
+++ b/toolkit/components/places/Database.cpp
@@ -323,30 +323,49 @@ NS_IMPL_ISUPPORTS(Database
, nsIObserver
, nsISupportsWeakReference
)
Database::Database()
: mMainThreadStatements(mMainConn)
, mMainThreadAsyncStatements(mMainConn)
, mAsyncThreadStatements(mMainConn)
+ , mGeneration(sGeneration)
, mDBPageSize(0)
, mDatabaseStatus(nsINavHistoryService::DATABASE_STATUS_OK)
, mClosed(false)
, mClientsShutdown(new ClientsShutdownBlocker())
, mConnectionShutdown(new ConnectionShutdownBlocker(this))
, mMaxUrlLength(0)
{
MOZ_ASSERT(!XRE_IsContentProcess(),
"Cannot instantiate Places in the content process");
// Attempting to create two instances of the service?
MOZ_ASSERT(!gDatabase);
gDatabase = this;
}
+// In non-test code, the database is a singleton initialized at startup, and
+// torn down on shutdown. This doesn't play well with tests that need to
+// shut down and restart the database multiple times. To make these tests work,
+// we use a "generation number": an incrementing counter that's shared between
+// the database and its two shutdown blockers. The blockers maintain a
+// corresponding counter that's incremented when shutdown begins. At that point,
+// `Database::sGeneration < PlacesShutdownBlocker::sGeneration`, so we know the
+// current database must be shutting down. When the current database is
+// destroyed, we bump the database generation number for the next restart.
+Atomic<uint32_t> Database::sGeneration(0);
+
+void
+Database::Tag(nsAString& aTag)
+{
+ aTag.AssignLiteral("Places database");
+ aTag.AppendInt(mGeneration);
+}
+
already_AddRefed<nsIAsyncShutdownClient>
Database::GetProfileChangeTeardownPhase()
{
nsCOMPtr<nsIAsyncShutdownService> asyncShutdownSvc = services::GetAsyncShutdown();
MOZ_ASSERT(asyncShutdownSvc);
if (NS_WARN_IF(!asyncShutdownSvc)) {
return nullptr;
}
@@ -373,26 +392,27 @@ Database::GetProfileBeforeChangePhase()
DebugOnly<nsresult> rv = asyncShutdownSvc->
GetProfileBeforeChange(getter_AddRefs(shutdownPhase));
MOZ_ASSERT(NS_SUCCEEDED(rv));
return shutdownPhase.forget();
}
Database::~Database()
{
+ sGeneration++;
}
bool
Database::IsShutdownStarted() const
{
if (!mConnectionShutdown) {
// We have already broken the cycle between `this` and `mConnectionShutdown`.
return true;
}
- return mConnectionShutdown->IsStarted();
+ return mConnectionShutdown->IsStarted(mGeneration);
}
already_AddRefed<mozIStorageAsyncStatement>
Database::GetAsyncStatement(const nsACString& aQuery) const
{
if (IsShutdownStarted()) {
return nullptr;
}
@@ -418,17 +438,19 @@ Database::GetClientsShutdown()
MOZ_ASSERT(mClientsShutdown);
return mClientsShutdown->GetClient();
}
// static
already_AddRefed<Database>
Database::GetDatabase()
{
- if (PlacesShutdownBlocker::IsStarted()) {
+ // This function can be called from multiple threads; it's important that
+ // it not call any main thread-only functions.
+ if (PlacesShutdownBlocker::IsStarted(sGeneration)) {
return nullptr;
}
return GetSingleton();
}
nsresult
Database::Init()
{
--- a/toolkit/components/places/Database.h
+++ b/toolkit/components/places/Database.h
@@ -194,16 +194,18 @@ public:
* @return The cached statement.
* @note Always null check the result.
* @note AsyncStatements are automatically reset on execution.
*/
already_AddRefed<mozIStorageAsyncStatement> GetAsyncStatement(const nsACString& aQuery) const;
uint32_t MaxUrlLength();
+ void Tag(nsAString& aTag);
+
protected:
/**
* Finalizes the cached statements and closes the database connection.
* A TOPIC_PLACES_CONNECTION_CLOSED notification is fired when done.
*/
void Shutdown();
bool IsShutdownStarted() const;
@@ -292,23 +294,25 @@ private:
~Database();
/**
* Singleton getter, invoked by class instantiation.
*/
static already_AddRefed<Database> GetSingleton();
static Database* gDatabase;
+ static Atomic<uint32_t> sGeneration;
nsCOMPtr<mozIStorageConnection> mMainConn;
mutable StatementCache mMainThreadStatements;
mutable AsyncStatementCache mMainThreadAsyncStatements;
mutable StatementCache mAsyncThreadStatements;
+ uint32_t mGeneration;
int32_t mDBPageSize;
uint16_t mDatabaseStatus;
bool mClosed;
/**
* Phases for shutting down the Database.
* See Shutdown.h for further details about the shutdown procedure.
*/
--- a/toolkit/components/places/Shutdown.cpp
+++ b/toolkit/components/places/Shutdown.cpp
@@ -3,29 +3,28 @@
* file, You can obtain one at http://mozilla.org/MPL/2.0/. */
#include "Shutdown.h"
#include "mozilla/Unused.h"
namespace mozilla {
namespace places {
-uint16_t PlacesShutdownBlocker::sCounter = 0;
-Atomic<bool> PlacesShutdownBlocker::sIsStarted(false);
+Atomic<uint32_t> PlacesShutdownBlocker::sGeneration(0);
PlacesShutdownBlocker::PlacesShutdownBlocker(const nsString& aName)
: mName(aName)
, mState(NOT_STARTED)
- , mCounter(sCounter++)
+ , mGeneration(sGeneration)
{
MOZ_ASSERT(NS_IsMainThread());
// During tests, we can end up with the Database singleton being resurrected.
// Make sure that each instance of DatabaseShutdown has a unique name.
- if (mCounter > 1) {
- mName.AppendInt(mCounter);
+ if (mGeneration > 0) {
+ mName.AppendInt(mGeneration);
}
}
// nsIAsyncShutdownBlocker
NS_IMETHODIMP
PlacesShutdownBlocker::GetName(nsAString& aName)
{
aName = mName;
@@ -166,23 +165,25 @@ ConnectionShutdownBlocker::ConnectionShu
// nsIAsyncShutdownBlocker
NS_IMETHODIMP
ConnectionShutdownBlocker::BlockShutdown(nsIAsyncShutdownClient* aParentClient)
{
MOZ_ASSERT(NS_IsMainThread());
mParentClient = new nsMainThreadPtrHolder<nsIAsyncShutdownClient>(aParentClient);
mState = RECEIVED_BLOCK_SHUTDOWN;
// Annotate that Database shutdown started.
- sIsStarted = true;
+ sGeneration.compareExchange(mGeneration, mGeneration + 1);
// Fire internal database closing notification.
nsCOMPtr<nsIObserverService> os = services::GetObserverService();
MOZ_ASSERT(os);
if (os) {
- Unused << os->NotifyObservers(nullptr, TOPIC_PLACES_WILL_CLOSE_CONNECTION, nullptr);
+ nsAutoString tag;
+ mDatabase->Tag(tag);
+ Unused << os->NotifyObservers(nullptr, TOPIC_PLACES_WILL_CLOSE_CONNECTION, tag.get());
}
mState = NOTIFIED_OBSERVERS_PLACES_WILL_CLOSE_CONNECTION;
// At this stage, any use of this database is forbidden. Get rid of
// `gDatabase`. Note, however, that the database could be
// resurrected. This can happen in particular during tests.
MOZ_ASSERT(Database::gDatabase == nullptr || Database::gDatabase == mDatabase);
Database::gDatabase = nullptr;
@@ -195,27 +196,30 @@ ConnectionShutdownBlocker::BlockShutdown
// mozIStorageCompletionCallback
NS_IMETHODIMP
ConnectionShutdownBlocker::Complete(nsresult, nsISupports*)
{
MOZ_ASSERT(NS_IsMainThread());
mState = RECEIVED_STORAGESHUTDOWN_COMPLETE;
+ nsAutoString tag;
+ mDatabase->Tag(tag);
+
// The connection is closed, the Database has no more use, so we can break
// possible cycles.
mDatabase = nullptr;
// Notify the connection has gone.
nsCOMPtr<nsIObserverService> os = mozilla::services::GetObserverService();
MOZ_ASSERT(os);
if (os) {
MOZ_ALWAYS_SUCCEEDS(os->NotifyObservers(nullptr,
TOPIC_PLACES_CONNECTION_CLOSED,
- nullptr));
+ tag.get()));
}
mState = NOTIFIED_OBSERVERS_PLACES_CONNECTION_CLOSED;
// mParentClient is nullptr in tests
if (mParentClient) {
nsresult rv = mParentClient->RemoveBlocker(this);
if (NS_WARN_IF(NS_FAILED(rv))) return rv;
mParentClient = nullptr;
--- a/toolkit/components/places/Shutdown.h
+++ b/toolkit/components/places/Shutdown.h
@@ -64,18 +64,18 @@ public:
NS_DECL_NSIASYNCSHUTDOWNBLOCKER
explicit PlacesShutdownBlocker(const nsString& aName);
/**
* `true` if we have not started shutdown, i.e. if
* `BlockShutdown()` hasn't been called yet, false otherwise.
*/
- static bool IsStarted() {
- return sIsStarted;
+ static bool IsStarted(uint32_t aDBGeneration) {
+ return aDBGeneration < sGeneration;
}
// The current state, used internally and for forensics/debugging purposes.
// Not all the states make sense for all the derived classes.
enum States {
NOT_STARTED,
// Execution of `BlockShutdown` in progress.
RECEIVED_BLOCK_SHUTDOWN,
@@ -107,22 +107,22 @@ protected:
nsString mName;
// The current state, see States.
States mState;
// The barrier optionally used to wait for clients.
nsMainThreadPtrHandle<nsIAsyncShutdownBarrier> mBarrier;
// The parent object who registered this as a blocker.
nsMainThreadPtrHandle<nsIAsyncShutdownClient> mParentClient;
- // As tests may resurrect a dead `Database`, we use a counter to
- // give the instances of `PlacesShutdownBlocker` unique names.
- uint16_t mCounter;
- static uint16_t sCounter;
-
- static Atomic<bool> sIsStarted;
+ // As tests may resurrect a dead `Database`, we use the generation
+ // number to give the instances of `PlacesShutdownBlocker` unique names.
+ // We also use this to determine whether a database has started
+ // shutdown.
+ uint32_t mGeneration;
+ static Atomic<uint32_t> sGeneration;
virtual ~PlacesShutdownBlocker() {}
};
/**
* Blocker also used to wait for clients, through an owned barrier.
*/
class ClientsShutdownBlocker final : public PlacesShutdownBlocker
--- a/toolkit/components/places/nsNavHistory.cpp
+++ b/toolkit/components/places/nsNavHistory.cpp
@@ -2944,16 +2944,25 @@ nsNavHistory::GetDBConnection(mozIStorag
NS_ENSURE_STATE(DB);
RefPtr<mozIStorageConnection> connection = DB->MainConn();
connection.forget(_DBConnection);
return NS_OK;
}
NS_IMETHODIMP
+nsNavHistory::GetDBTag(nsAString& aTag)
+{
+ RefPtr<Database> DB = GetDatabase();
+ NS_ENSURE_STATE(DB);
+ DB->Tag(aTag);
+ return NS_OK;
+}
+
+NS_IMETHODIMP
nsNavHistory::GetShutdownClient(nsIAsyncShutdownClient **_shutdownClient)
{
NS_ENSURE_ARG_POINTER(_shutdownClient);
RefPtr<Database> DB = GetDatabase();
NS_ENSURE_STATE(DB);
RefPtr<nsIAsyncShutdownClient> client = DB->GetClientsShutdown();
MOZ_ASSERT(client);
client.forget(_shutdownClient);
--- a/toolkit/components/places/nsPIPlacesDatabase.idl
+++ b/toolkit/components/places/nsPIPlacesDatabase.idl
@@ -21,16 +21,18 @@ interface nsIAsyncShutdownClient;
[scriptable, uuid(366ee63e-a413-477d-9ad6-8d6863e89401)]
interface nsPIPlacesDatabase : nsISupports
{
/**
* The database connection used by Places.
*/
readonly attribute mozIStorageConnection DBConnection;
+ readonly attribute AString DBTag;
+
/**
* Asynchronously executes the statement created from queries.
*
* @see nsINavHistoryService::executeQueries
* @note THIS IS A TEMPORARY API. Don't rely on it, since it will be replaced
* in future versions by a real async querying API.
* @note Results obtained from this method differ from results obtained from
* executeQueries, because there is additional filtering and sorting