Skip to content

Commit 0793d9e

Browse files
chkuang-ga-maurice
authored andcommitted
Check if the destructor has been started when an event is triggered.
Whenever an event is triggered from a owned object, ex. from PersistentConnection to Repo, check if the destructor has been started using SafeReference. If so, early out. If not, lock the reference until the end of the event. PiperOrigin-RevId: 265122249
1 parent b378a74 commit 0793d9e

File tree

4 files changed

+37
-1
lines changed

4 files changed

+37
-1
lines changed

app/src/safe_reference.h

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -77,6 +77,12 @@ class SafeReferenceLock {
7777
MutexLock lock_;
7878
};
7979

80+
// A macro to check if the safe_reference is valid. Early out if not.
81+
#define SAFE_REFERENCE_RETURN_VOID_IF_INVALID(lock_type, lock_name, \
82+
safe_reference) \
83+
lock_type lock_name(&safe_reference); \
84+
if (!lock_name.GetReference()) return;
85+
8086
} // namespace internal
8187
} // namespace FIREBASE_NAMESPACE
8288

database/src/desktop/connection/connection.cc

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -166,6 +166,8 @@ void Connection::Send(const Variant& message, bool is_sensitive) {
166166
}
167167

168168
void Connection::OnOpen() {
169+
SAFE_REFERENCE_RETURN_VOID_IF_INVALID(ConnectionRefLock, lock, safe_this_);
170+
169171
LogDebug("%s websocket opened", log_id_.c_str());
170172

171173
scheduler_->Schedule(new callback::CallbackValue1<ConnectionRef>(
@@ -196,6 +198,8 @@ void Connection::OnOpen() {
196198
}
197199

198200
void Connection::OnMessage(const char* msg) {
201+
SAFE_REFERENCE_RETURN_VOID_IF_INVALID(ConnectionRefLock, lock, safe_this_);
202+
199203
LogDebug("%s websocket message received", log_id_.c_str());
200204
scheduler_->Schedule(new callback::CallbackValue1String1<ConnectionRef>(
201205
safe_this_, msg, [](ConnectionRef conn_ref, const char* msg) {
@@ -208,6 +212,8 @@ void Connection::OnMessage(const char* msg) {
208212
}
209213

210214
void Connection::OnClose() {
215+
SAFE_REFERENCE_RETURN_VOID_IF_INVALID(ConnectionRefLock, lock, safe_this_);
216+
211217
LogDebug("%s websocket closed", log_id_.c_str());
212218

213219
scheduler_->Schedule(new callback::CallbackValue1<ConnectionRef>(
@@ -228,6 +234,8 @@ void Connection::OnClose() {
228234
}
229235

230236
void Connection::OnError(const WebSocketClientErrorData& error_data) {
237+
SAFE_REFERENCE_RETURN_VOID_IF_INVALID(ConnectionRefLock, lock, safe_this_);
238+
231239
LogDebug("%s websocket error occurred. Uri: %s", log_id_.c_str(),
232240
error_data.GetUri().c_str());
233241

database/src/desktop/connection/persistent_connection.cc

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -145,6 +145,8 @@ PersistentConnection::~PersistentConnection() {
145145
}
146146

147147
void PersistentConnection::OnCacheHost(const std::string& host) {
148+
SAFE_REFERENCE_RETURN_VOID_IF_INVALID(ThisRefLock, lock, safe_this_);
149+
148150
// TODO(chkuang): Ignore cache host for now.
149151
}
150152

@@ -177,6 +179,8 @@ bool HasKey(const Variant& data, const char* key) {
177179

178180
void PersistentConnection::OnReady(int64_t timestamp,
179181
const std::string& session_id) {
182+
SAFE_REFERENCE_RETURN_VOID_IF_INVALID(ThisRefLock, lock, safe_this_);
183+
180184
LogDebug("%s OnReady", log_id_.c_str());
181185

182186
// Trigger OnServerInfoUpdate based on timestamp delta
@@ -241,6 +245,8 @@ void PersistentConnection::HandleConnectStatsResponse(
241245
void PersistentConnection::OnDataMessage(const Variant& message) {
242246
assert(message.is_map());
243247

248+
SAFE_REFERENCE_RETURN_VOID_IF_INVALID(ThisRefLock, lock, safe_this_);
249+
244250
if (HasKey(message, kRequestNumber)) {
245251
auto it_request_number = message.map().find(kRequestNumber);
246252
assert(it_request_number->second.is_numeric());
@@ -285,6 +291,8 @@ void PersistentConnection::OnDataMessage(const Variant& message) {
285291
}
286292

287293
void PersistentConnection::OnDisconnect(Connection::DisconnectReason reason) {
294+
SAFE_REFERENCE_RETURN_VOID_IF_INVALID(ThisRefLock, lock, safe_this_);
295+
288296
LogDebug("%s Got on disconnect due to %d", log_id_.c_str(),
289297
static_cast<int>(reason));
290298

@@ -311,6 +319,8 @@ void PersistentConnection::OnDisconnect(Connection::DisconnectReason reason) {
311319
}
312320

313321
void PersistentConnection::OnKill(const std::string& reason) {
322+
SAFE_REFERENCE_RETURN_VOID_IF_INVALID(ThisRefLock, lock, safe_this_);
323+
314324
LogDebug(
315325
"%s Firebase Database connection was forcefully killed by the server. "
316326
"Will not attempt reconnect. Reason: %s",

database/src/desktop/core/repo.cc

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -487,9 +487,15 @@ static std::map<Path, Variant> VariantToPathMap(const Variant& data) {
487487
return path_map;
488488
}
489489

490-
void Repo::OnConnect() { OnServerInfoUpdate(kDotInfoConnected, true); }
490+
void Repo::OnConnect() {
491+
SAFE_REFERENCE_RETURN_VOID_IF_INVALID(ThisRefLock, lock, safe_this_);
492+
493+
OnServerInfoUpdate(kDotInfoConnected, true);
494+
}
491495

492496
void Repo::OnDisconnect() {
497+
SAFE_REFERENCE_RETURN_VOID_IF_INVALID(ThisRefLock, lock, safe_this_);
498+
493499
OnServerInfoUpdate(kDotInfoConnected, false);
494500
RunOnDisconnectEvents();
495501
}
@@ -513,6 +519,8 @@ void Repo::RunOnDisconnectEvents() {
513519
}
514520

515521
void Repo::OnAuthStatus(bool auth_ok) {
522+
SAFE_REFERENCE_RETURN_VOID_IF_INVALID(ThisRefLock, lock, safe_this_);
523+
516524
OnServerInfoUpdate(kDotInfoAuthenticated, auth_ok);
517525
}
518526

@@ -521,6 +529,8 @@ void Repo::OnServerInfoUpdate(const std::string& key, const Variant& value) {
521529
}
522530

523531
void Repo::OnServerInfoUpdate(const std::map<Variant, Variant>& updates) {
532+
SAFE_REFERENCE_RETURN_VOID_IF_INVALID(ThisRefLock, lock, safe_this_);
533+
524534
for (const std::pair<Variant, Variant> element : updates) {
525535
const Variant& key = element.first;
526536
const Variant& value = element.second;
@@ -530,6 +540,8 @@ void Repo::OnServerInfoUpdate(const std::map<Variant, Variant>& updates) {
530540

531541
void Repo::OnDataUpdate(const Path& path, const Variant& data, bool is_merge,
532542
const connection::PersistentConnection::Tag& tag) {
543+
SAFE_REFERENCE_RETURN_VOID_IF_INVALID(ThisRefLock, lock, safe_this_);
544+
533545
std::vector<Event> events;
534546
if (is_merge) {
535547
std::map<Path, Variant> changed_children = VariantToPathMap(data);

0 commit comments

Comments
 (0)