Skip to content

fix(android): release Java-returned wrappers when JavaScript drops them #127

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
ammarahm-ed wants to merge 1 commit into from
Open

fix(android): release Java-returned wrappers when JavaScript drops them #127

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
100 changes: 86 additions & 14 deletions NativeScript/ffi/jni/jsi/objectmanager/ObjectManager.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
#include "NativeScriptException.h"
#include "Runtime.h"
#include "CallbackHandlers.h"
#include "EngineHost.h"
#include <algorithm>
#include <sstream>

Expand Down Expand Up @@ -60,6 +61,9 @@ ObjectManager::ObjectManager(jobject javaRuntimeObject) :

void ObjectManager::Init(JsRuntime &rt) {
m_rt = &rt;
m_ownerToken = std::make_shared<OwnerToken>();
m_ownerToken->manager = this;
m_ownerToken->runtime = &rt;

JsFunction jsObjectCtor = JsFunction::createFromHostConstructor(
rt, JsPropNameID::forAscii(rt, "JSObject"), 0,
Expand All @@ -84,6 +88,10 @@ void ObjectManager::OnDisposeRuntime() {
m_idToProxy.clear();
m_idToObject.clear();
m_jsObjectCtor = JsFunction();
if (m_ownerToken != nullptr) {
m_ownerToken->manager = nullptr;
m_ownerToken->runtime = nullptr;
}

// Its destructor leaves the weak global refs alive, and the process-wide
// weak reference table is finite.
Expand Down Expand Up @@ -141,15 +149,7 @@ JsValue ObjectManager::GetOrCreateProxy(jint javaObjectID, const JsValue &instan
}
JsValue proxy = CreateHostObjectProxy(instance, info, /*isPrimary=*/true);

auto javaObjectIdFound = m_weakObjectIds.find(javaObjectID);
if (javaObjectIdFound != m_weakObjectIds.end()) {
m_weakObjectIds.erase(javaObjectID);
JEnv jenv;
jenv.CallVoidMethod(m_javaRuntimeObject,
MAKE_INSTANCE_STRONG_METHOD_ID,
javaObjectID);
DEBUG_WRITE("Making instance strong: %d", javaObjectID);
}
EnsureInstanceStrong(javaObjectID);

m_idToProxy.emplace(javaObjectID, engine::WeakObject(*m_rt, proxy));

Expand Down Expand Up @@ -608,11 +608,26 @@ JsValue ObjectManager::GetJsObjectByJavaObject(int javaObjectID) {
return js_util::undefined();
}

JsValue instance = it->second;
JsValue instance = LockWrapper(it->second);
if (js_util::is_null_or_undefined(instance)) return js_util::undefined();

auto proxyIt = m_idToProxy.find(javaObjectID);
if (proxyIt != m_idToProxy.end() && !proxyIt->second.empty()) {
JsValue proxy = proxyIt->second.lock(*m_rt);
if (!js_util::is_null_or_undefined(proxy)) return proxy;
}

MetadataNode *node = GetInstanceNode(instance);
if (node != nullptr && !node->isArray()) return instance;
return GetOrCreateProxy(javaObjectID, instance);
}

JsValue ObjectManager::LockWrapper(const WrapperHandle &handle) {
if (handle.isStrong) return JsValue(*m_rt, handle.strong);
if (handle.weak.empty()) return js_util::undefined();
return handle.weak.lock(*m_rt);
}


JsValue ObjectManager::CreateJSWrapper(jint javaObjectID, const std::string &typeName) {
return CreateJSWrapperHelper(javaObjectID, typeName, nullptr);
Expand Down Expand Up @@ -648,18 +663,30 @@ JsValue ObjectManager::CreateJSWrapper(jint javaObjectID, MetadataNode *node, jc
JsValue proxy = js_util::undefined();
JsValue jsWrapper = node->CreateJSWrapper(*m_rt, this);
if (jsWrapper.isObject()) {
Link(jsWrapper, javaObjectID, clazz, node);
Link(jsWrapper, javaObjectID, clazz, node, /*strongRef*/ false);
EnsureInstanceStrong(javaObjectID);
if (node->isArray()) {
jsWrapper.asObject(*m_rt).setProperty(*m_rt, "__is__javaArray", true);
proxy = GetOrCreateProxy(javaObjectID, jsWrapper);
} else {
proxy = jsWrapper;
}
proxy = GetOrCreateProxy(javaObjectID, jsWrapper);
}

return proxy;
}

void ObjectManager::EnsureInstanceStrong(int javaObjectID) {
auto it = m_weakObjectIds.find(javaObjectID);
if (it == m_weakObjectIds.end()) return;
m_weakObjectIds.erase(it);
JEnv jenv;
jenv.CallVoidMethod(m_javaRuntimeObject, MAKE_INSTANCE_STRONG_METHOD_ID, javaObjectID);
DEBUG_WRITE("Making instance strong: %d", javaObjectID);
}

void ObjectManager::Link(const JsValue &object, uint32_t javaObjectID, jclass clazz,
MetadataNode *node) {
MetadataNode *node, bool strongRef) {
if (!IsRuntimeJsObject(object)) {
std::string errMsg("Trying to link invalid 'this' to a Java object");
throw NativeScriptException(errMsg);
Expand All @@ -669,12 +696,57 @@ void ObjectManager::Link(const JsValue &object, uint32_t javaObjectID, jclass cl

auto jsInstanceInfo = std::make_shared<JSInstanceInfo>(javaObjectID, clazz);
jsInstanceInfo->node = node;
if (!strongRef) {
jsInstanceInfo->owner = m_ownerToken;
auto rtOwner = Runtime::GetRuntimeUnchecked(*m_rt);
if (rtOwner != nullptr) rtOwner->GetEngineHost()->AdjustExternalMemory(kWrapperExternalCost);
}

// One slot, one owner: the native state both carries the record and keeps it
// alive, replacing the napi tree's external-plus-wrap pair.
object.asObjectBorrowed(*m_rt).setNativeState<JSInstanceInfo>(*m_rt, jsInstanceInfo);

m_idToObject.emplace(javaObjectID, JsValue(*m_rt, object));
WrapperHandle handle;
if (strongRef) {
handle.strong = JsValue(*m_rt, object);
handle.isStrong = true;
} else {
handle.weak = engine::WeakObject(*m_rt, object);
}
m_idToObject[javaObjectID] = std::move(handle);
}

ObjectManager::JSInstanceInfo::~JSInstanceInfo() {
if (owner == nullptr || owner->manager == nullptr || owner->runtime == nullptr) return;
auto *pending = new int(static_cast<int>(JavaObjectID));
Runtime::PostFinalizer(*owner->runtime, WrapperPostFinalizer, pending, owner->manager);
}

void ObjectManager::WrapperPostFinalizer(JsRuntime &rt, void *data, void *hint) {
auto *pending = reinterpret_cast<int *>(data);
if (pending == nullptr) return;
int javaObjectID = *pending;
delete pending;

auto rtOwner = Runtime::GetRuntimeUnchecked(rt);
if (rtOwner == nullptr || rtOwner->is_destroying) return;
auto objManager = reinterpret_cast<ObjectManager *>(hint);
if (objManager == nullptr || objManager != rtOwner->GetObjectManager()) return;

rtOwner->GetEngineHost()->AdjustExternalMemory(-kWrapperExternalCost);

auto it = objManager->m_idToObject.find(javaObjectID);
if (it != objManager->m_idToObject.end() &&
!js_util::is_null_or_undefined(objManager->LockWrapper(it->second))) {
return;
}
if (objManager->m_weakObjectIds.find(javaObjectID) == objManager->m_weakObjectIds.end()) {
objManager->m_weakObjectIds.emplace(javaObjectID);
JEnv jEnv;
jEnv.CallVoidMethod(objManager->m_javaRuntimeObject, objManager->MAKE_INSTANCE_WEAK_METHOD_ID,
javaObjectID);
}
if (it != objManager->m_idToObject.end()) objManager->m_idToObject.erase(it);
}

bool ObjectManager::CloneLink(const JsValue &src, const JsValue &dest) {
Expand Down
33 changes: 31 additions & 2 deletions NativeScript/ffi/jni/jsi/objectmanager/ObjectManager.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,9 @@ namespace tns {
JsValue GetOrCreateProxyWeak(jint javaObjectID, const JsValue &instance);

void Link(const JsValue &object, uint32_t javaObjectID, jclass clazz,
MetadataNode *node = nullptr);
MetadataNode *node = nullptr, bool strongRef = true);

static constexpr int64_t kWrapperExternalCost = 1024;

// Returns the class metadata stored on the per-instance JSInstanceInfo
// (host proxy's, or the raw instance's native state). Used by
Expand Down Expand Up @@ -99,13 +101,26 @@ namespace tns {
// It derives from engine::HostObject because that is what the native
// state slot stores; it overrides none of the traps and is never exposed
// to JS as an object of its own.
// Lets a weakly held wrapper's native state reach its manager. The engine
// destroys native state on its own schedule, possibly after the manager is
// gone, so OnDisposeRuntime clears the token.
struct OwnerToken {
ObjectManager *manager = nullptr;
JsRuntime *runtime = nullptr;
};

struct JSInstanceInfo : public engine::HostObject {
public:
JSInstanceInfo(uint32_t javaObjectID, jclass claz)
: JavaObjectID(javaObjectID), ObjectClazz(claz) {
}

// For a weakly held wrapper this runs when the engine collects it, and
// queues the release of the Java side.
~JSInstanceInfo() override;

uint32_t JavaObjectID;
std::shared_ptr<OwnerToken> owner;
jclass ObjectClazz;
// Cached super-call flag (-1 = unresolved, 0 = false, 1 = true).
int8_t isSuper = -1;
Expand Down Expand Up @@ -199,6 +214,10 @@ namespace tns {

JsValue CreateJSWrapperHelper(jint javaObjectID, const std::string &typeName, jclass clazz);

void EnsureInstanceStrong(int javaObjectID);

static void WrapperPostFinalizer(JsRuntime &rt, void *data, void *hint);

jweak GetJavaObjectByID(uint32_t javaObjectID);

jobject GetJavaObjectByIDImpl(uint32_t javaObjectID);
Expand All @@ -216,7 +235,17 @@ namespace tns {
// The napi tree stored a weak napi_ref for proxies and a strong one for
// instances; those map onto engine::WeakObject and an owned Value.
robin_hood::unordered_map<int, engine::WeakObject> m_idToProxy;
robin_hood::unordered_map<int, JsValue> m_idToObject;
// Strong for JS-constructed instances, weak for wrappers of Java-returned objects.
struct WrapperHandle {
JsValue strong;
engine::WeakObject weak;
bool isStrong = false;
};

JsValue LockWrapper(const WrapperHandle &handle);

robin_hood::unordered_map<int, WrapperHandle> m_idToObject;
std::shared_ptr<OwnerToken> m_ownerToken;
robin_hood::unordered_set<int> m_weakObjectIds;
robin_hood::unordered_set<int> m_markedAsWeakIds;

Expand Down
85 changes: 71 additions & 14 deletions NativeScript/ffi/jni/napi/objectmanager/ObjectManager.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -199,15 +199,7 @@ napi_value ObjectManager::GetOrCreateProxy(jint javaObjectID, napi_value instanc

#endif

auto javaObjectIdFound = m_weakObjectIds.find(javaObjectID);
if (javaObjectIdFound != m_weakObjectIds.end()) {
m_weakObjectIds.erase(javaObjectID);
JEnv jenv;
jenv.CallVoidMethod(m_javaRuntimeObject,
MAKE_INSTANCE_STRONG_METHOD_ID,
javaObjectID);
DEBUG_WRITE("Making instance strong: %d", javaObjectID);
}
EnsureInstanceStrong(javaObjectID);

m_idToProxy.emplace(javaObjectID, napi_util::make_ref(m_env, proxy, 0));

Expand Down Expand Up @@ -799,6 +791,15 @@ napi_value ObjectManager::GetJsObjectByJavaObject(int javaObjectID) {

napi_value instance = napi_util::get_ref_value(m_env, it->second);
if (napi_util::is_null_or_undefined(m_env, instance)) return nullptr;

auto proxyIt = m_idToProxy.find(javaObjectID);
if (proxyIt != m_idToProxy.end() && proxyIt->second != nullptr) {
napi_value proxy = napi_util::get_ref_value(m_env, proxyIt->second);
if (!napi_util::is_null_or_undefined(m_env, proxy)) return proxy;
}

MetadataNode *node = GetInstanceNode(instance);
if (node != nullptr && !node->isArray()) return instance;
return GetOrCreateProxy(javaObjectID, instance);
}

Expand Down Expand Up @@ -839,19 +840,31 @@ napi_value ObjectManager::CreateJSWrapper(jint javaObjectID, MetadataNode *node,
napi_value proxy = nullptr;
napi_value jsWrapper = node->CreateJSWrapper(m_env, this);
if (jsWrapper != nullptr) {
Link(jsWrapper, javaObjectID, clazz, node);
Link(jsWrapper, javaObjectID, clazz, node, /*strongRef*/ false);
EnsureInstanceStrong(javaObjectID);
if (node->isArray()) {
NAPI_GUARD(napi_set_named_property(m_env, jsWrapper, "__is__javaArray",
napi_util::get_true(m_env))) {}
proxy = GetOrCreateProxy(javaObjectID, jsWrapper);
} else {
proxy = jsWrapper;
}
proxy = GetOrCreateProxy(javaObjectID, jsWrapper);
}

return proxy;
}

void ObjectManager::EnsureInstanceStrong(int javaObjectID) {
auto it = m_weakObjectIds.find(javaObjectID);
if (it == m_weakObjectIds.end()) return;
m_weakObjectIds.erase(it);
JEnv jenv;
jenv.CallVoidMethod(m_javaRuntimeObject, MAKE_INSTANCE_STRONG_METHOD_ID, javaObjectID);
DEBUG_WRITE("Making instance strong: %d", javaObjectID);
}

void ObjectManager::Link(napi_value object, uint32_t javaObjectID, jclass clazz,
MetadataNode *node) {
MetadataNode *node, bool strongRef) {
if (!IsRuntimeJsObject(object)) {
std::string errMsg("Trying to link invalid 'this' to a Java object");
throw NativeScriptException(errMsg);
Expand All @@ -862,8 +875,14 @@ void ObjectManager::Link(napi_value object, uint32_t javaObjectID, jclass clazz,
napi_status status;
auto jsInstanceInfo = new JSInstanceInfo(javaObjectID, clazz);
jsInstanceInfo->node = node;
jsInstanceInfo->weakWrapper = !strongRef;

napi_ref objectHandle = napi_util::make_ref(m_env, object, strongRef ? 1 : 0);

napi_ref objectHandle = napi_util::make_ref(m_env, object, 1);
if (!strongRef) {
int64_t externalMemory = 0;
js_adjust_external_memory(m_env, kWrapperExternalCost, &externalMemory);
}

napi_value jsInfo;
NAPI_GUARD(napi_create_external(m_env, jsInstanceInfo, JSObjectFinalizerCallback, jsInstanceInfo, &jsInfo)) {}
Expand All @@ -873,7 +892,13 @@ void ObjectManager::Link(napi_value object, uint32_t javaObjectID, jclass clazz,
NAPI_GUARD(napi_wrap(m_env, object, jsInstanceInfo, [](napi_env env, void *data, void *hint) {}, jsInstanceInfo,
nullptr)) {}

m_idToObject.emplace(javaObjectID, objectHandle);
auto existing = m_idToObject.find(javaObjectID);
if (existing != m_idToObject.end()) {
NAPI_GUARD(napi_delete_reference(m_env, existing->second)) {}
existing->second = objectHandle;
} else {
m_idToObject.emplace(javaObjectID, objectHandle);
}
}

bool ObjectManager::CloneLink(napi_value src, napi_value dest) {
Expand Down Expand Up @@ -922,6 +947,38 @@ ObjectManager::JSObjectFinalizerCallback(napi_env env, void *finalizeData, void
#endif

DEBUG_WRITE("JS Object finalizer called for object id: %d", data->JavaObjectID);
Runtime::PostFinalizer(env, JSObjectPostFinalizerCallback, data, nullptr);
}

void ObjectManager::JSObjectPostFinalizerCallback(napi_env env, void *finalizeData, void *) {
napi_status status;
auto data = reinterpret_cast<JSInstanceInfo *>(finalizeData);
if (data == nullptr) return;

auto rt = Runtime::GetRuntimeUnchecked(env);
if (data->weakWrapper && rt && !rt->is_destroying) {
int64_t externalMemory = 0;
js_adjust_external_memory(env, -kWrapperExternalCost, &externalMemory);

auto objManager = rt->GetObjectManager();
auto it = objManager->m_idToObject.find(data->JavaObjectID);
bool replaced = false;
if (it != objManager->m_idToObject.end()) {
napi_value current = napi_util::get_ref_value(env, it->second);
replaced = !napi_util::is_null_or_undefined(env, current);
if (!replaced) {
NAPI_GUARD(napi_delete_reference(env, it->second)) {}
objManager->m_idToObject.erase(it);
}
}
if (!replaced && objManager->m_weakObjectIds.find(data->JavaObjectID) ==
objManager->m_weakObjectIds.end()) {
objManager->m_weakObjectIds.emplace(data->JavaObjectID);
JEnv jEnv;
jEnv.CallVoidMethod(objManager->m_javaRuntimeObject,
objManager->MAKE_INSTANCE_WEAK_METHOD_ID, data->JavaObjectID);
}
}
delete data;
}

Expand Down
Loading
Loading