Skip to content

Commit 5063e9f

Browse files
authored
Fix leaks in Firestore (#2199)
* Clean up retain cycle in FSTLevelDB. * Explicitly CFRelease our SCNetworkReachabilityRef. * Make gRPC stream delegates weak
1 parent ab05b96 commit 5063e9f

File tree

5 files changed

+23
-11
lines changed

5 files changed

+23
-11
lines changed

Firestore/Source/Local/FSTLRUGarbageCollector.mm

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -81,7 +81,7 @@ size_t size() const {
8181
};
8282

8383
@implementation FSTLRUGarbageCollector {
84-
id<FSTLRUDelegate> _delegate;
84+
__weak id<FSTLRUDelegate> _delegate;
8585
LruParams _params;
8686
}
8787

Firestore/Source/Local/FSTLevelDBMutationQueue.mm

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -86,7 +86,8 @@ - (instancetype)initWithUserID:(std::string)userID
8686
@end
8787

8888
@implementation FSTLevelDBMutationQueue {
89-
FSTLevelDB *_db;
89+
// This instance is owned by FSTLevelDB; avoid a retain cycle.
90+
__weak FSTLevelDB *_db;
9091

9192
/** The normalized userID (e.g. nil UID => @"" userID) used in our LevelDB keys. */
9293
std::string _userID;

Firestore/Source/Local/FSTLevelDBQueryCache.mm

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -62,7 +62,8 @@ @interface FSTLevelDBQueryCache ()
6262
@end
6363

6464
@implementation FSTLevelDBQueryCache {
65-
FSTLevelDB *_db;
65+
// This instance is owned by FSTLevelDB; avoid a retain cycle.
66+
__weak FSTLevelDB *_db;
6667

6768
/**
6869
* The last received snapshot version. This is part of `metadata` but we store it separately to

Firestore/core/src/firebase/firestore/remote/connectivity_monitor_apple.mm

Lines changed: 16 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -77,7 +77,13 @@ void OnReachabilityChangedCallback(SCNetworkReachabilityRef /*unused*/,
7777
class ConnectivityMonitorApple : public ConnectivityMonitor {
7878
public:
7979
explicit ConnectivityMonitorApple(AsyncQueue* worker_queue)
80-
: ConnectivityMonitor{worker_queue}, reachability_{CreateReachability()} {
80+
: ConnectivityMonitor{worker_queue} {
81+
reachability_ = CreateReachability();
82+
if (!reachability_) {
83+
LOG_DEBUG("Failed to create reachability monitor.");
84+
return;
85+
}
86+
8187
SCNetworkReachabilityFlags flags;
8288
if (SCNetworkReachabilityGetFlags(reachability_, &flags)) {
8389
SetInitialStatus(ToNetworkStatus(flags));
@@ -108,10 +114,14 @@ explicit ConnectivityMonitorApple(AsyncQueue* worker_queue)
108114
}
109115

110116
~ConnectivityMonitorApple() {
111-
bool success =
112-
SCNetworkReachabilitySetDispatchQueue(reachability_, nullptr);
113-
if (!success) {
114-
LOG_DEBUG("Couldn't unset reachability queue");
117+
if (reachability_) {
118+
bool success =
119+
SCNetworkReachabilitySetDispatchQueue(reachability_, nullptr);
120+
if (!success) {
121+
LOG_DEBUG("Couldn't unset reachability queue");
122+
}
123+
124+
CFRelease(reachability_);
115125
}
116126
}
117127

@@ -121,7 +131,7 @@ void OnReachabilityChanged(SCNetworkReachabilityFlags flags) {
121131
}
122132

123133
private:
124-
SCNetworkReachabilityRef reachability_;
134+
SCNetworkReachabilityRef reachability_ = nil;
125135
};
126136

127137
namespace {

Firestore/core/src/firebase/firestore/remote/remote_objc_bridge.h

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -185,7 +185,7 @@ class WatchStreamDelegate {
185185
void NotifyDelegateOnClose(const util::Status& status);
186186

187187
private:
188-
id<FSTWatchStreamDelegate> delegate_;
188+
__weak id<FSTWatchStreamDelegate> delegate_;
189189
};
190190

191191
/** A C++ bridge that invokes methods on an `FSTWriteStreamDelegate`. */
@@ -202,7 +202,7 @@ class WriteStreamDelegate {
202202
void NotifyDelegateOnClose(const util::Status& status);
203203

204204
private:
205-
id<FSTWriteStreamDelegate> delegate_;
205+
__weak id<FSTWriteStreamDelegate> delegate_;
206206
};
207207

208208
} // namespace bridge

0 commit comments

Comments
 (0)