Skip to content

Commit

Permalink
Fix race condition when finalizing weakrefs (#41)
Browse files Browse the repository at this point in the history
Co-authored-by: Li Feng <[email protected]>
  • Loading branch information
finalpatch and li-feng-sc authored Dec 29, 2021
1 parent fa64e75 commit dba7ee3
Show file tree
Hide file tree
Showing 2 changed files with 20 additions and 6 deletions.
2 changes: 1 addition & 1 deletion support-lib/wasm/djinni_wasm.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ Date::JsType Date::fromCpp(const CppType& c) {

JsProxyId nextId = 0;
std::unordered_map<JsProxyId, std::weak_ptr<JsProxyBase>> jsProxyCache;
std::unordered_map<void*, em::val> cppProxyCache;
std::unordered_map<void*, CppProxyCacheEntry> cppProxyCache;
std::mutex jsProxyCacheMutex;
std::mutex cppProxyCacheMutex;

Expand Down
24 changes: 19 additions & 5 deletions support-lib/wasm/djinni_wasm.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -426,18 +426,26 @@ class JsProxyBase {
JsProxyId _id;
};

struct CppProxyCacheEntry {
em::val ref;
int count;
};

extern JsProxyId nextId;
extern std::unordered_map<JsProxyId, std::weak_ptr<JsProxyBase>> jsProxyCache;
extern std::unordered_map<void*, em::val> cppProxyCache;
extern std::unordered_map<void*, CppProxyCacheEntry> cppProxyCache;
extern std::mutex jsProxyCacheMutex;
extern std::mutex cppProxyCacheMutex;

template<typename I, typename Self>
struct JsInterface {
static void nativeDestroy(const std::shared_ptr<I>& cpp) {
std::lock_guard lk(cppProxyCacheMutex);
assert(cppProxyCache.find(cpp.get()) != cppProxyCache.end());
cppProxyCache.erase(cpp.get());
auto i = cppProxyCache.find(cpp.get());
assert(i != cppProxyCache.end());
if (--(i->second.count) == 0) {
cppProxyCache.erase(cpp.get());
}
}
// enable this only when the derived class has `cppProxyMethods` defined
// (interface +c)
Expand All @@ -454,9 +462,11 @@ struct JsInterface {
// look up in cpp proxy cache
std::lock_guard lk(cppProxyCacheMutex);
auto i = cppProxyCache.find(c.get());
int jsRefCount = 1;
if (i != cppProxyCache.end()) {
// found existing cpp proxy
auto strongRef = i->second.template call<em::val>("deref");
jsRefCount += i->second.count;
auto strongRef = i->second.ref.template call<em::val>("deref");
if (!strongRef.isUndefined()) {
// and it's not expired
return strongRef;
Expand All @@ -468,7 +478,11 @@ struct JsInterface {
em::val nativeRef(c);
em::val cppProxy = getCppProxyClass().new_(nativeRef, Self::cppProxyMethods());
em::val weakRef = weakRefClass.new_(cppProxy);
cppProxyCache.emplace(c.get(), weakRef);
if (i == cppProxyCache.end()) {
cppProxyCache.emplace(c.get(), CppProxyCacheEntry{weakRef, jsRefCount});
} else {
i->second = CppProxyCacheEntry{weakRef, jsRefCount};
}
getCppProxyFinalizerRegistry().call<void>("register", cppProxy, nativeRef);
return cppProxy;
}
Expand Down

0 comments on commit dba7ee3

Please sign in to comment.