Commit acfa38b8 authored by Daniel Murphy's avatar Daniel Murphy

[IndexedDB] Prevent reentry during context destruction

The IndexedDBOriginState map is iterated on context destruction, and
each state is ForceClose()'ed. However, calling this:
* Requires that a handle is held, so ForceClose doesn't operate on a
  destructed state, and
* Can cause the OriginState to destruct itself.

To avoid this, all of the RemoveOriginState callbacks are bound with a
specific weak factory that can be invalidated in order to call
ForceClose on all of the OriginState, which are then manually deleted.

TBR: pwnall@chromium.org
Bug: 966366,966267
Change-Id: I2ba2b9e6e10d34172ce38a934cafb0e9e341c05f
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1627268Reviewed-by: default avatarDaniel Murphy <dmurph@chromium.org>
Commit-Queue: Daniel Murphy <dmurph@chromium.org>
Cr-Commit-Position: refs/heads/master@{#662653}
parent f917fb22
...@@ -426,6 +426,11 @@ void IndexedDBFactoryImpl::ContextDestroyed() { ...@@ -426,6 +426,11 @@ void IndexedDBFactoryImpl::ContextDestroyed() {
// Set |context_| to nullptr first to ensure no re-entry into the |cotext_| // Set |context_| to nullptr first to ensure no re-entry into the |cotext_|
// object during shutdown. This can happen in methods like BlobFilesCleaned. // object during shutdown. This can happen in methods like BlobFilesCleaned.
context_ = nullptr; context_ = nullptr;
// Invalidate the weak factory that is used by the IndexedDBOriginStates to
// destruct themselves. This prevents modification of the
// |factories_per_origin_| map while it is iterated below, and allows us to
// avoid holding a handle to call ForceClose();
origin_state_destruction_weak_factory_.InvalidateWeakPtrs();
for (const auto& pair : factories_per_origin_) { for (const auto& pair : factories_per_origin_) {
pair.second->ForceClose(); pair.second->ForceClose();
} }
...@@ -583,12 +588,14 @@ IndexedDBFactoryImpl::GetOrOpenOriginFactory( ...@@ -583,12 +588,14 @@ IndexedDBFactoryImpl::GetOrOpenOriginFactory(
data_loss_info}; data_loss_info};
it = factories_per_origin_ it = factories_per_origin_
.emplace(origin, std::make_unique<IndexedDBOriginState>( .emplace(origin,
is_in_memory, clock_, &earliest_sweep_, std::make_unique<IndexedDBOriginState>(
base::BindOnce( is_in_memory, clock_, &earliest_sweep_,
&IndexedDBFactoryImpl::RemoveOriginFactory, base::BindOnce(
base::Unretained(this), origin), &IndexedDBFactoryImpl::RemoveOriginState,
std::move(backing_store))) origin_state_destruction_weak_factory_.GetWeakPtr(),
origin),
std::move(backing_store)))
.first; .first;
context_->FactoryOpened(origin); context_->FactoryOpened(origin);
return {it->second->CreateHandle(), s, IndexedDBDatabaseError(), return {it->second->CreateHandle(), s, IndexedDBDatabaseError(),
...@@ -605,7 +612,7 @@ std::unique_ptr<IndexedDBBackingStore> IndexedDBFactoryImpl::CreateBackingStore( ...@@ -605,7 +612,7 @@ std::unique_ptr<IndexedDBBackingStore> IndexedDBFactoryImpl::CreateBackingStore(
backing_store_mode, this, origin, blob_path, std::move(db), task_runner); backing_store_mode, this, origin, blob_path, std::move(db), task_runner);
} }
void IndexedDBFactoryImpl::RemoveOriginFactory(const url::Origin& origin) { void IndexedDBFactoryImpl::RemoveOriginState(const url::Origin& origin) {
factories_per_origin_.erase(origin); factories_per_origin_.erase(origin);
} }
......
...@@ -159,7 +159,7 @@ class CONTENT_EXPORT IndexedDBFactoryImpl : public IndexedDBFactory { ...@@ -159,7 +159,7 @@ class CONTENT_EXPORT IndexedDBFactoryImpl : public IndexedDBFactory {
FRIEND_TEST_ALL_PREFIXES(IndexedDBTest, FRIEND_TEST_ALL_PREFIXES(IndexedDBTest,
ForceCloseOpenDatabasesOnCommitFailure); ForceCloseOpenDatabasesOnCommitFailure);
void RemoveOriginFactory(const url::Origin& origin); void RemoveOriginState(const url::Origin& origin);
void OnDatabaseError(const url::Origin& origin, void OnDatabaseError(const url::Origin& origin,
leveldb::Status s, leveldb::Status s,
...@@ -186,6 +186,12 @@ class CONTENT_EXPORT IndexedDBFactoryImpl : public IndexedDBFactory { ...@@ -186,6 +186,12 @@ class CONTENT_EXPORT IndexedDBFactoryImpl : public IndexedDBFactory {
std::set<url::Origin> backends_opened_since_startup_; std::set<url::Origin> backends_opened_since_startup_;
// Weak pointers from this factory are used to bind the RemoveOriginState()
// function, which deletes the IndexedDBOriginState object. This allows those
// weak pointers to be invalidated during force close & shutdown to prevent
// re-entry (see ContextDestroyed()).
base::WeakPtrFactory<IndexedDBFactoryImpl>
origin_state_destruction_weak_factory_{this};
base::WeakPtrFactory<IndexedDBFactoryImpl> weak_factory_{this}; base::WeakPtrFactory<IndexedDBFactoryImpl> weak_factory_{this};
DISALLOW_COPY_AND_ASSIGN(IndexedDBFactoryImpl); DISALLOW_COPY_AND_ASSIGN(IndexedDBFactoryImpl);
}; };
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment