Skip to content

Commit 6b100bc

Browse files
cynthiajianga-maurice
authored andcommitted
Fix future crash that cause unity editor to crash second time enter play mode.
FutureProxyManager still grabs FutureHandle that supposed to be cleaned up and released. Force release them during proxy manager's destructor. PiperOrigin-RevId: 316809477
1 parent 2a65cd1 commit 6b100bc

File tree

2 files changed

+45
-6
lines changed

2 files changed

+45
-6
lines changed

app/src/reference_counted_future_impl.cc

Lines changed: 42 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -73,7 +73,17 @@ class FutureProxyManager {
7373
const FutureHandle& subject)
7474
: api_(api), subject_(subject) {}
7575

76+
~FutureProxyManager() {
77+
MutexLock lock(mutex_);
78+
for (FutureHandle& h : clients_) {
79+
api_->ForceReleaseFuture(h);
80+
h = ReferenceCountedFutureImpl::kInvalidHandle;
81+
}
82+
clients_.clear();
83+
}
84+
7685
void RegisterClient(const FutureHandle& handle) {
86+
MutexLock lock(mutex_);
7787
// We create one reference per client to the Future.
7888
// This way the ReferenceCountedFutureImpl will do the right thing if one
7989
// thread tries to unregister the last client while adding a new one.
@@ -89,12 +99,18 @@ class FutureProxyManager {
8999
};
90100

91101
static void UnregisterCallback(void* data) {
102+
if (data == nullptr) {
103+
return;
104+
}
92105
UnregisterData* udata = static_cast<UnregisterData*>(data);
93-
udata->proxy->UnregisterClient(udata->handle);
94-
delete udata;
106+
if (udata != nullptr) {
107+
udata->proxy->UnregisterClient(udata->handle);
108+
delete udata;
109+
}
95110
}
96111

97112
void UnregisterClient(const FutureHandle& handle) {
113+
MutexLock lock(mutex_);
98114
for (FutureHandle& h : clients_) {
99115
if (h == handle) {
100116
h = ReferenceCountedFutureImpl::kInvalidHandle;
@@ -108,6 +124,7 @@ class FutureProxyManager {
108124
}
109125

110126
void CompleteClients(int error, const char* error_msg) {
127+
MutexLock lock(mutex_);
111128
for (const FutureHandle& h : clients_) {
112129
if (h != ReferenceCountedFutureImpl::kInvalidHandle) {
113130
api_->Complete(h, error, error_msg);
@@ -120,6 +137,8 @@ class FutureProxyManager {
120137
ReferenceCountedFutureImpl* api_;
121138
// We need to keep the subject alive, as it owns us and the data.
122139
FutureHandle subject_;
140+
// mutex to protect register/unregister operation.
141+
mutable Mutex mutex_;
123142
};
124143

125144
struct CompletionCallbackData {
@@ -245,7 +264,10 @@ FutureBackingData::~FutureBackingData() {
245264
context_data = nullptr;
246265
}
247266

248-
delete proxy;
267+
if (proxy != nullptr) {
268+
delete proxy;
269+
proxy = nullptr;
270+
}
249271
}
250272

251273
void FutureBackingData::ClearExistingCallbacks() {
@@ -466,11 +488,14 @@ void ReferenceCountedFutureImpl::ReleaseFuture(const FutureHandle& handle) {
466488
MutexLock lock(mutex_);
467489
FIREBASE_FUTURE_TRACE("API: Release future %d", (int)handle.id());
468490

469-
// Assert if the handle isn't registered.
470491
// If a Future exists with a handle, then the backing should still exist for
471-
// it, too.
492+
// it, too. However it might be possible during the deallocate phase when
493+
// FutureBase and FutureHandle and FutureProxyManager are still having
494+
// dependencies.
472495
auto it = backings_.find(handle.id());
473-
FIREBASE_ASSERT(it != backings_.end());
496+
if (it == backings_.end()) {
497+
return;
498+
}
474499

475500
// Decrement the reference count.
476501
FutureBackingData* backing = it->second;
@@ -767,6 +792,17 @@ TypedCleanupNotifier<FutureHandle>& CleanupMgr(
767792
return static_cast<ReferenceCountedFutureImpl*>(api)->cleanup_handles();
768793
}
769794

795+
void ReferenceCountedFutureImpl::ForceReleaseFuture(
796+
const FutureHandle& handle) {
797+
MutexLock lock(mutex_);
798+
FutureBackingData* backing = BackingFromHandle(handle.id());
799+
if (backing != nullptr) {
800+
backing->reference_count = 1;
801+
ReleaseFuture(handle);
802+
}
803+
FIREBASE_FUTURE_TRACE("API: ForceReleaseFuture handle %d", handle.id());
804+
}
805+
770806
// Implementation of FutureHandle from future.h
771807
FutureHandle::FutureHandle() : id_(0), api_(nullptr) {}
772808

app/src/reference_counted_future_impl.h

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -419,6 +419,9 @@ class ReferenceCountedFutureImpl : public detail::FutureApiInterface {
419419
return cleanup_handles_;
420420
}
421421

422+
/// Force reset the ref count and release the handle.
423+
void ForceReleaseFuture(const FutureHandle& handle);
424+
422425
private:
423426
template <typename T>
424427
static void DeleteT(void* ptr_to_delete) {

0 commit comments

Comments
 (0)