Skip to content

Commit

Permalink
[TIMOB-26202] Improve memory management
Browse files Browse the repository at this point in the history
  • Loading branch information
Gary Mathews committed Jul 26, 2018
1 parent 327d283 commit 4a76b6f
Show file tree
Hide file tree
Showing 12 changed files with 137 additions and 45 deletions.
Expand Up @@ -244,12 +244,18 @@ void ${className}::${method.apiName}(const FunctionCallbackInfo<Value>& args)
<@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));
}

if (holder.IsEmpty() || holder->IsNull()) {
args.GetReturnValue().Set(v8::Undefined(isolate));
return;
}
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 +293,15 @@ 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());

Local<Object> holder = args.Holder();
if (!JavaObject::isJavaObject(holder)) {
holder = holder->FindInstanceInPrototypeChain(getProxyTemplate(isolate));
}
if (holder.IsEmpty() || holder->IsNull()) {
args.GetReturnValue().Set(v8::Undefined(isolate));
return;
}
titanium::Proxy* proxy = NativeObject::Unwrap<titanium::Proxy>(holder);
if (!proxy) {
args.GetReturnValue().Set(Undefined(isolate));
return;
Expand Down Expand Up @@ -324,7 +337,15 @@ 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());
Local<Object> holder = args.Holder();
if (!JavaObject::isJavaObject(holder)) {
holder = holder->FindInstanceInPrototypeChain(getProxyTemplate(isolate));
}
if (holder.IsEmpty() || holder->IsNull()) {
args.GetReturnValue().Set(v8::Undefined(isolate));
return;
}
titanium::Proxy* proxy = NativeObject::Unwrap<titanium::Proxy>(holder);
if (!proxy) {
return;
}
Expand Down Expand Up @@ -363,8 +384,15 @@ 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());

Local<Object> holder = args.Holder();
if (!JavaObject::isJavaObject(holder)) {
holder = holder->FindInstanceInPrototypeChain(getProxyTemplate(isolate));
}
if (holder.IsEmpty() || holder->IsNull()) {
args.GetReturnValue().Set(v8::Undefined(isolate));
return;
}
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

0 comments on commit 4a76b6f

Please sign in to comment.