Skip to content
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.

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

[TIMOB-26202] Android: Improve memory management #10195

Merged
merged 2 commits into from Jul 26, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
Expand Up @@ -720,3 +720,15 @@ beprovided to handle the result.
}
}
</#macro>

<#macro getHolder>
Local<Object> holder = args.Holder();
if (!JavaObject::isJavaObject(holder)) {
holder = holder->FindInstanceInPrototypeChain(getProxyTemplate(isolate));
}
if (holder.IsEmpty() || holder->IsNull()) {
LOGE(TAG, "Couldn't obtain argument holder");
args.GetReturnValue().Set(v8::Undefined(isolate));
return;
}
</#macro>
Expand Up @@ -243,13 +243,12 @@ void ${className}::${method.apiName}(const FunctionCallbackInfo<Value>& args)
<@Proxy.initJNIEnv/>
<@Proxy.initMethodID className=className name=name signature=signature logOnly=false/>

Local<Object> holder = args.Holder();
// If holder isn't the JavaObject wrapper we expect, look up the prototype chain
if (!JavaObject::isJavaObject(holder)) {
holder = holder->FindInstanceInPrototypeChain(getProxyTemplate(isolate));
}

<@Proxy.getHolder/>
titanium::Proxy* proxy = NativeObject::Unwrap<titanium::Proxy>(holder);
if (!proxy) {
args.GetReturnValue().Set(Undefined(isolate));
return;
}

<#if method.args?size &gt; 0>
<@Proxy.verifyAndConvertArguments method.args method />
Expand Down Expand Up @@ -287,8 +286,8 @@ void ${className}::getter_${name}(Local<Name> property, const PropertyCallbackIn
<@Proxy.initJNIEnv/>
<@Proxy.initMethodID className=className name=property.getMethodName signature=getSignature logOnly=false/>

titanium::Proxy* proxy = NativeObject::Unwrap<titanium::Proxy>(args.Holder());

<@Proxy.getHolder/>
titanium::Proxy* proxy = NativeObject::Unwrap<titanium::Proxy>(holder);
if (!proxy) {
args.GetReturnValue().Set(Undefined(isolate));
return;
Expand Down Expand Up @@ -324,7 +323,8 @@ void ${className}::setter_${name}(Local<Name> property, Local<Value> value, cons

<@Proxy.initMethodID className=className name=property.setMethodName signature=setSignature logOnly=true />

titanium::Proxy* proxy = NativeObject::Unwrap<titanium::Proxy>(args.Holder());
<@Proxy.getHolder/>
titanium::Proxy* proxy = NativeObject::Unwrap<titanium::Proxy>(holder);
if (!proxy) {
return;
}
Expand Down Expand Up @@ -363,8 +363,8 @@ void ${className}::interceptor(Local<String> property, const v8::PropertyCallbac
<@Proxy.initJNIEnv/>
<@Proxy.initMethodID className=className name=interceptor.name signature="(Ljava/lang/String;)Ljava/lang/Object;" logOnly=false/>

titanium::Proxy* proxy = NativeObject::Unwrap<titanium::Proxy>(args.Holder());

<@Proxy.getHolder/>
titanium::Proxy* proxy = NativeObject::Unwrap<titanium::Proxy>(holder);
if (!proxy) {
args.GetReturnValue().Set(Undefined(isolate));
return;
Expand Down
Expand Up @@ -675,6 +675,7 @@ private void cleanupSections()
{
ArrayList<TableViewSectionProxy> sections = getSectionsArray();
for (TableViewSectionProxy section : sections) {
section.releaseViews();
section.setParent(null);
}
sections.clear();
Expand Down
Expand Up @@ -272,30 +272,33 @@ public void setLabelsClickable(boolean clickable)
@Override
public void releaseViews()
{
super.releaseViews();

if (tableViewItem != null) {
tableViewItem.release();
tableViewItem = null;
}
if (controls != null) {
for (TiViewProxy control : controls) {
control.releaseKroll();
control.releaseViews();
}
}

super.releaseViews();
}

@Override
public void release()
{
super.release();

releaseViews();

if (tableViewItem != null) {
tableViewItem.release();
tableViewItem = null;
}
if (controls != null) {
for (TiViewProxy control : controls) {
control.release();
}
controls.clear();
controls = null;
}

super.release();
}

public TiTableViewRowProxyItem getTableViewRowProxyItem()
Expand Down
Expand Up @@ -251,7 +251,7 @@ public void run()
Activity currentActivity = TiApplication.getAppCurrentActivity();
if (currentActivity != null) {
TiUIHelper.showSoftKeyboard(currentActivity.getWindow().getDecorView(), false);
} else if (activity != null) {
} else if (getActivity() != null) {
TiUIHelper.showSoftKeyboard(getActivity().getWindow().getDecorView(), false);
} else {
Log.w(TAG, "Unable to hide soft keyboard. Activity is null", Log.DEBUG_MODE);
Expand Down
Expand Up @@ -162,4 +162,19 @@ public void setOnSearchChangeListener(OnSearchChangeListener listener)
{
searchChangeListener = listener;
}

@Override
public void release()
{
if (searchView != null) {
searchView.setOnQueryTextListener(null);
searchView.setOnCloseListener(null);
searchView.setOnQueryTextFocusChangeListener(null);
searchView = null;
}
if (searchChangeListener != null) {
searchChangeListener = null;
}
super.release();
}
}
Expand Up @@ -434,6 +434,11 @@ public void setRowData(TableViewRowProxy rp)
}
}

if (content == null) {
this.content = new TiCompositeLayout(getRowProxy().getActivity());
addView(content, new LayoutParams(LayoutParams.MATCH_PARENT, LayoutParams.MATCH_PARENT));
}

if (props.containsKey(TiC.PROPERTY_LAYOUT)) {
content.setLayoutArrangement(TiConvert.toString(props, TiC.PROPERTY_LAYOUT));
}
Expand Down Expand Up @@ -647,14 +652,16 @@ public Drawable getSelectorDrawable()
@Override
public void release()
{
super.release();
if (views != null) {
for (TiUIView view : views) {
view.release();
}
views.clear();
views = null;
}
if (item != null) {
item = null;
}
if (content != null) {
content.removeAllViews();
content = null;
Expand All @@ -667,5 +674,7 @@ public void release()
hasChildDrawable.setCallback(null);
hasChildDrawable = null;
}

super.release();
}
}
Expand Up @@ -61,10 +61,7 @@ public static long createReference(Object object)
public static void destroyReference(long key)
{
Log.d(TAG, "Destroying reference under key: " + key, Log.DEBUG_MODE);
Object obj = references.remove(key);
if (obj instanceof Reference) {
obj = ((Reference<?>) obj).get();
}
Object obj = getReference(key);
// If it's an V8Object, set the ptr to 0, because the proxy is dead on C++ side
// This *should* prevent the native code from trying to reconstruct the proxy for any reason
if (obj instanceof KrollProxySupport) {
Expand All @@ -75,6 +72,10 @@ public static void destroyReference(long key)
v8.setPointer(0);
}
}
if (obj instanceof KrollProxy) {
((KrollProxy) obj).setReferenceKey(0);
}
references.remove(key);
}

/**
Expand All @@ -85,7 +86,7 @@ public static void destroyReference(long key)
public static void makeWeakReference(long key)
{
Log.d(TAG, "Downgrading to weak reference for key: " + key, Log.DEBUG_MODE);
Object ref = references.get(key);
Object ref = getReference(key);
references.remove(key);
references.put(key, new WeakReference<Object>(ref));
}
Expand All @@ -98,7 +99,7 @@ public static void makeWeakReference(long key)
public static void makeSoftReference(long key)
{
Log.d(TAG, "Downgrading to soft reference for key: " + key, Log.DEBUG_MODE);
Object ref = references.get(key);
Object ref = getReference(key);
references.remove(key);
references.put(key, new SoftReference<Object>(ref));
}
Expand Down Expand Up @@ -144,6 +145,6 @@ public static Object getReference(long key)
public static boolean isStrongReference(long key)
{
Object ref = getReference(key);
return !(ref instanceof WeakReference || ref instanceof SoftReference);
return !(ref instanceof Reference);
}
}
15 changes: 11 additions & 4 deletions android/runtime/v8/src/native/JavaObject.cpp
Expand Up @@ -80,7 +80,9 @@ jobject JavaObject::getJavaObject()
jobject ref = ReferenceTable::getReference(refTableKey_);
if (ref == NULL) {
// Sanity check. Did we get into a state where it was weak on Java, got GC'd but the C++ proxy didn't get deleted yet?
LOGE(TAG, "!!! OH NO! We tried to grab a Java Object back out of the reference table, but it must have been GC'd, because it's null! Key: %d", refTableKey_);
LOGW(TAG, "Could not obtain reference, java object has already been collected! (Key: %d)", refTableKey_);
refTableKey_ = 0;
javaObject_ = NULL;
}
return ref;
}
Expand Down Expand Up @@ -124,7 +126,7 @@ void JavaObject::MakeJSWeak()
// but this time, we say call us back as a finalizer so we can resurrect the
// object (save it from really being GCd by V8) and move it's Java object twin
// to a weak reference in the JVM. (where we can track when that gets GC'd by the JVM to call back and kill this)
if (!isDetached()) {
if (!isDetached() && !persistent().IsEmpty()) {
persistent().SetWeak(this, DetachCallback, v8::WeakCallbackType::kFinalizer); // MUST BE kFinalizer or our object cannot be resurrected!
persistent().MarkIndependent();
}
Expand Down Expand Up @@ -181,7 +183,9 @@ void JavaObject::MakeJavaStrong()
jobject stored = ReferenceTable::clearReference(refTableKey_);
if (stored == NULL) {
// Sanity check. Did we get into a state where it was weak on Java, got GC'd but the C++ proxy didn't get deleted yet?
LOGE(TAG, "!!! OH NO! We tried to move a weak Java object back to strong, but it's aleady been GC'd by JVM! We're in a bad state! Key: %d", refTableKey_);
LOGW(TAG, "Could not move weak reference to strong, java object has already been collected! (Key: %d)", refTableKey_);
refTableKey_ = 0;
javaObject_ = NULL;
} else {
env->DeleteLocalRef(stored);
}
Expand Down Expand Up @@ -238,15 +242,18 @@ void JavaObject::DeleteJavaRef()
} else {
env->DeleteGlobalRef(javaObject_);
}
javaObject_ = NULL;
} else {
LOGD(TAG, "Deleting ref in ReferenceTable for key: %d, pointer: %p", refTableKey_, this);
ReferenceTable::destroyReference(refTableKey_); // Kill the Java side
refTableKey_ = 0; // throw away the key
}
javaObject_ = NULL;
// When we're done we should be wrapping nothing!
ASSERT(javaObject_ == NULL);
ASSERT(refTableKey_ == 0);

// our java object can be collected, make our JS reference weak too
// MakeJSWeak();
}

}
13 changes: 7 additions & 6 deletions android/runtime/v8/src/native/Proxy.cpp
Expand Up @@ -154,12 +154,13 @@ static void onPropertyChangedForProxy(Isolate* isolate, Local<String> property,
jobject javaValue = TypeConverter::jsValueToJavaObject(isolate, env, value, &javaValueIsNew);

jobject javaProxy = proxy->getJavaObject();
env->CallVoidMethod(javaProxy,
JNIUtil::krollProxyOnPropertyChangedMethod,
javaProperty,
javaValue);

proxy->unreferenceJavaObject(javaProxy);
if (javaProxy != NULL) {
env->CallVoidMethod(javaProxy,
JNIUtil::krollProxyOnPropertyChangedMethod,
javaProperty,
javaValue);
proxy->unreferenceJavaObject(javaProxy);
}

env->DeleteLocalRef(javaProperty);
if (javaValueIsNew) {
Expand Down
9 changes: 7 additions & 2 deletions android/runtime/v8/src/native/V8Object.cpp
Expand Up @@ -98,8 +98,13 @@ Java_org_appcelerator_kroll_runtime_v8_V8Object_nativeFireEvent
emitter = TypeConverter::javaObjectToJsValue(V8Runtime::v8_isolate, env, jEmitter).As<Object>();
}

Local<Value> fireEventValue = emitter->Get(EventEmitter::emitSymbol.Get(V8Runtime::v8_isolate));
if (!fireEventValue->IsFunction()) {
Local<String> symbol = EventEmitter::emitSymbol.Get(V8Runtime::v8_isolate);
if (emitter.IsEmpty() || symbol.IsEmpty()) {
return JNI_FALSE;
}

Local<Value> fireEventValue = emitter->Get(symbol);
if (fireEventValue.IsEmpty() || !fireEventValue->IsFunction()) {
return JNI_FALSE;
}

Expand Down
Expand Up @@ -1571,15 +1571,20 @@ protected void onDestroy()

//LW windows
if (window == null && view != null) {
view.releaseViews();
view.release();
view = null;
}
if (view != null) {
view.releaseViews();
view = null;
}

if (window != null) {
if (windowStack.contains(window)) {
removeWindowFromStack(window);
}
window.closeFromActivity(isFinishing);
window.releaseViews();
window.releaseKroll();
window = null;
}

Expand Down
Expand Up @@ -617,23 +617,40 @@ public void realizeViews(TiUIView view)

public void releaseViews()
{
if (view != null) {
if (children != null) {
for (TiViewProxy p : children) {
p.releaseViews();
}
if (children != null) {
for (TiViewProxy p : children) {
p.releaseViews();
}
}
if (view != null) {
view.release();
view = null;
}
releaseKroll();
if (runtimeHandler != null) {
runtimeHandler = null;
}
if (mainHandler != null) {
mainHandler = null;
}
setModelListener(null);

releaseKroll();
KrollRuntime.suggestGC();
}

@Override
public void release()
{
releaseViews();

if (children != null) {
children.clear();
children = null;
}

super.release();
}

/**
* Implementing classes should use this method to create and return the appropriate view.
* @param activity the context activity.
Expand Down