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 c7cfadc commit f9ba900
Show file tree
Hide file tree
Showing 13 changed files with 166 additions and 45 deletions.
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
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();
}
}
Original file line number Diff line number Diff line change
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();
}
}
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,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
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
Expand Up @@ -1567,15 +1567,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
Original file line number Diff line number Diff line change
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 f9ba900

Please sign in to comment.