Skip to content

Commit 96f86fa

Browse files
amabluea-maurice
authored andcommitted
Clear the SafeRef earlier during database shutdown, so that scheduled actions
do not have the opportunity to use it while it is being torn down. Additionally, there were scheduled call backs being kicked off in repo.cc that were not wrapped in a safe ref lock, so any of them could crash if the repo was being shut down while they were running. Those functions are all now wrapped in safe ref locks. PiperOrigin-RevId: 255608792
1 parent 16bbe32 commit 96f86fa

File tree

2 files changed

+20
-10
lines changed

2 files changed

+20
-10
lines changed

database/src/desktop/core/repo.cc

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -105,8 +105,8 @@ Repo::Repo(App* app, DatabaseInternal* database, const char* url)
105105
Repo::~Repo() {
106106
// Terminate the connection immediately to prevent messages from arriving
107107
// while the SyncTree is being torn down.
108-
connection_.reset(nullptr);
109108
safe_this_.ClearReference();
109+
connection_.reset(nullptr);
110110
}
111111

112112
void Repo::AddEventCallback(UniquePtr<EventRegistration> event_registration) {

database/src/desktop/query_desktop.cc

Lines changed: 19 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -196,19 +196,25 @@ void QueryInternal::RemoveAllValueListeners() {
196196
void QueryInternal::AddEventRegistration(
197197
UniquePtr<EventRegistration> registration) {
198198
Repo::scheduler().Schedule(NewCallback(
199-
[](Repo* repo, UniquePtr<EventRegistration> registration) {
200-
repo->AddEventCallback(Move(registration));
199+
[](Repo::ThisRef ref, UniquePtr<EventRegistration> registration) {
200+
Repo::ThisRefLock lock(&ref);
201+
if (lock.GetReference() != nullptr) {
202+
lock.GetReference()->AddEventCallback(Move(registration));
203+
}
201204
},
202-
database_->repo(), Move(registration)));
205+
database_->repo()->this_ref(), Move(registration)));
203206
}
204207

205208
void QueryInternal::RemoveEventRegistration(void* listener_ptr,
206209
const QuerySpec& query_spec) {
207210
Repo::scheduler().Schedule(NewCallback(
208-
[](Repo* repo, void* listener_ptr, QuerySpec query_spec) {
209-
repo->RemoveEventCallback(listener_ptr, query_spec);
211+
[](Repo::ThisRef ref, void* listener_ptr, QuerySpec query_spec) {
212+
Repo::ThisRefLock lock(&ref);
213+
if (lock.GetReference() != nullptr) {
214+
lock.GetReference()->RemoveEventCallback(listener_ptr, query_spec);
215+
}
210216
},
211-
database_->repo(), listener_ptr, query_spec));
217+
database_->repo()->this_ref(), listener_ptr, query_spec));
212218
}
213219

214220
void QueryInternal::RemoveEventRegistration(ValueListener* listener,
@@ -245,10 +251,14 @@ DatabaseReferenceInternal* QueryInternal::GetReference() {
245251

246252
void QueryInternal::SetKeepSynchronized(bool keep_synchronized) {
247253
Repo::scheduler().Schedule(NewCallback(
248-
[](Repo* repo, QuerySpec query_spec, bool keep_synchronized) {
249-
repo->SetKeepSynchronized(query_spec, keep_synchronized);
254+
[](Repo::ThisRef ref, QuerySpec query_spec, bool keep_synchronized) {
255+
Repo::ThisRefLock lock(&ref);
256+
if (lock.GetReference() != nullptr) {
257+
lock.GetReference()->SetKeepSynchronized(query_spec,
258+
keep_synchronized);
259+
}
250260
},
251-
database_->repo(), query_spec_, keep_synchronized));
261+
database_->repo()->this_ref(), query_spec_, keep_synchronized));
252262
}
253263

254264
QueryInternal* QueryInternal::OrderByChild(const char* path) {

0 commit comments

Comments
 (0)