From a2671f5ef0db6fd56c31cfe82e36fd13a8c14103 Mon Sep 17 00:00:00 2001 From: Eric House Date: Sun, 9 Mar 2014 14:46:33 -0700 Subject: [PATCH] add global mempool that can survive individual games, and include it every game's globals. Use it instead of game's mempool to create dictionaries so they can outlive a game and not trip asserts on pool delete. Wrap dicts in a java class that refcounts them when BoardCanvas wants to keep one so that, I hope, the bug using a deleted dict will go away. --- xwords4/android/XWords4/jni/xwjni.c | 70 ++++++++++++++-- .../org/eehouse/android/xw4/BoardCanvas.java | 20 +++-- .../src/org/eehouse/android/xw4/XWApp.java | 9 ++ .../org/eehouse/android/xw4/jni/XwJNI.java | 83 ++++++++++++++++++- 4 files changed, 165 insertions(+), 17 deletions(-) diff --git a/xwords4/android/XWords4/jni/xwjni.c b/xwords4/android/XWords4/jni/xwjni.c index 504b42b5d..2fc4e1d72 100644 --- a/xwords4/android/XWords4/jni/xwjni.c +++ b/xwords4/android/XWords4/jni/xwjni.c @@ -39,6 +39,42 @@ #include "jniutlswrapper.h" #include "paths.h" +/* Globals for the whole game */ +typedef struct _JNIGlobalState { + JNIEnv* env; + MPSLOT +} JNIGlobalState; + +JNIEXPORT jint JNICALL +Java_org_eehouse_android_xw4_jni_XwJNI_initGlobals +( JNIEnv* env, jclass C ) +{ +#ifdef MEM_DEBUG + MemPoolCtx* mpool = mpool_make(); +#endif + JNIGlobalState* state = (JNIGlobalState*)XP_CALLOC( mpool, sizeof(*state) ); + state->env = env; + MPASSIGN( state->mpool, mpool ); + LOG_RETURNF( "%p", state ); + return (jint)state; +} + +JNIEXPORT void JNICALL +Java_org_eehouse_android_xw4_jni_XwJNI_cleanGlobals +( JNIEnv* env, jclass C, jint ptr ) +{ + LOG_FUNC(); + if ( 0 != ptr ) { + JNIGlobalState* state = (JNIGlobalState*)ptr; + XP_ASSERT( state->env == env ); +#ifdef MEM_DEBUG + MemPoolCtx* mpool = state->mpool; +#endif + XP_FREE( mpool, state ); + mpool_destroy( mpool ); + } +} + static const SetInfo gi_ints[] = { ARR_MEMBER( CurGameInfo, nPlayers ) ,ARR_MEMBER( CurGameInfo, gameSeconds ) @@ -316,6 +352,26 @@ Java_org_eehouse_android_xw4_jni_XwJNI_comms_1getUUID return jstr; } +JNIEXPORT void JNICALL +Java_org_eehouse_android_xw4_jni_XwJNI_dict_1ref +( JNIEnv* env, jclass C, jint dictPtr ) +{ + if ( 0 != dictPtr ) { + DictionaryCtxt* dict = (DictionaryCtxt*)dictPtr; + dict_ref( dict ); + } +} + +JNIEXPORT void JNICALL +Java_org_eehouse_android_xw4_jni_XwJNI_dict_1unref +( JNIEnv* env, jclass C, jint dictPtr ) +{ + if ( 0 != dictPtr ) { + DictionaryCtxt* dict = (DictionaryCtxt*)dictPtr; + dict_unref( dict ); + } +} + JNIEXPORT jboolean JNICALL Java_org_eehouse_android_xw4_jni_XwJNI_dict_1getInfo ( JNIEnv* env, jclass C, jbyteArray jDictBytes, jstring jname, jstring jpath, @@ -380,6 +436,7 @@ Java_org_eehouse_android_xw4_jni_XwJNI_dict_1getTileValue typedef struct _JNIState { XWGame game; JNIEnv* env; + JNIGlobalState* globalJNI; AndGlobals globals; XP_U16 curSaveCount; XP_U16 lastSavedSize; @@ -422,7 +479,7 @@ typedef struct _JNIState { JNIEXPORT jint JNICALL Java_org_eehouse_android_xw4_jni_XwJNI_initJNI -( JNIEnv* env, jclass C ) +( JNIEnv* env, jclass C, int jniGlobalPtr ) { /* Why am I doing this twice? */ /* struct timeval tv; */ @@ -432,6 +489,7 @@ Java_org_eehouse_android_xw4_jni_XwJNI_initJNI MemPoolCtx* mpool = mpool_make(); #endif JNIState* state = (JNIState*)XP_CALLOC( mpool, sizeof(*state) ); + state->globalJNI = (JNIGlobalState*)jniGlobalPtr; AndGlobals* globals = &state->globals; globals->state = (struct JNIState*)state; MPASSIGN( state->mpool, mpool ); @@ -472,7 +530,8 @@ Java_org_eehouse_android_xw4_jni_XwJNI_game_1makeNewGame DictionaryCtxt* dict; PlayerDicts dicts; - makeDicts( MPPARM(mpool) env, globals->jniutil, &dict, &dicts, j_names, + + makeDicts( MPPARM(state->globalJNI->mpool) env, globals->jniutil, &dict, &dicts, j_names, j_dicts, j_paths, j_lang ); #ifdef STUBBED_DICT if ( !dict ) { @@ -529,8 +588,8 @@ Java_org_eehouse_android_xw4_jni_XwJNI_game_1makeFromStream globals->util = makeUtil( MPPARM(mpool) &state->env, jutil, globals->gi, globals ); globals->jniutil = makeJNIUtil( MPPARM(mpool) &state->env, jniu ); - makeDicts( MPPARM(mpool) env, globals->jniutil, &dict, &dicts, jdictNames, - jdicts, jpaths, jlang ); + makeDicts( MPPARM(state->globalJNI->mpool) env, globals->jniutil, &dict, + &dicts, jdictNames, jdicts, jpaths, jlang ); if ( !!jdraw ) { globals->dctx = makeDraw( MPPARM(mpool) &state->env, jdraw ); } @@ -1456,9 +1515,10 @@ Java_org_eehouse_android_xw4_jni_XwJNI_game_1changeDict jbyteArray jDictBytes, jstring jpath ) { XWJNI_START_GLOBALS(); - DictionaryCtxt* dict = makeDict( MPPARM(mpool) env, globals->jniutil, + DictionaryCtxt* dict = makeDict( MPPARM(state->globalJNI->mpool) env, globals->jniutil, jname, jDictBytes, jpath, NULL, false ); game_changeDict( MPPARM(mpool) &state->game, globals->gi, dict ); + dict_unref( dict ); setJGI( env, jgi, globals->gi ); XWJNI_END(); return XP_FALSE; /* no need to redraw */ diff --git a/xwords4/android/XWords4/src/org/eehouse/android/xw4/BoardCanvas.java b/xwords4/android/XWords4/src/org/eehouse/android/xw4/BoardCanvas.java index ce7e90bad..8e1d7ae2e 100644 --- a/xwords4/android/XWords4/src/org/eehouse/android/xw4/BoardCanvas.java +++ b/xwords4/android/XWords4/src/org/eehouse/android/xw4/BoardCanvas.java @@ -38,6 +38,7 @@ import org.eehouse.android.xw4.jni.DrawCtx; import org.eehouse.android.xw4.jni.DrawScoreInfo; import org.eehouse.android.xw4.jni.JNIThread; import org.eehouse.android.xw4.jni.XwJNI; +import org.eehouse.android.xw4.jni.XwJNI.DictWrapper; import junit.framework.Assert; @@ -84,7 +85,7 @@ public class BoardCanvas extends Canvas implements DrawCtx { private Drawable m_downArrow; private int m_trayOwner = -1; private int m_pendingScore; - private int m_dictPtr = 0; + private DictWrapper m_dict; protected String[] m_dictChars; private boolean m_hasSmallScreen; private int m_backgroundUsed = 0x00000000; @@ -142,6 +143,7 @@ public class BoardCanvas extends Canvas implements DrawCtx { m_activity = activity; m_bitmap = bitmap; m_jniThread = jniThread; + m_dict = new DictWrapper(); m_hasSmallScreen = Utils.hasSmallScreen( m_context ); @@ -542,20 +544,21 @@ public class BoardCanvas extends Canvas implements DrawCtx { } } - public void dictChanged( final int dictPtr ) + public void dictChanged( final int newPtr ) { - if ( m_dictPtr != dictPtr ) { - if ( 0 == dictPtr ) { + int curPtr = m_dict.getDictPtr(); + if ( curPtr != newPtr ) { + if ( 0 == newPtr ) { m_fontDims = null; m_dictChars = null; - } else if ( m_dictPtr == 0 || - !XwJNI.dict_tilesAreSame( m_dictPtr, dictPtr ) ) { + } else if ( 0 == curPtr + || !XwJNI.dict_tilesAreSame( curPtr, newPtr ) ) { m_fontDims = null; m_dictChars = null; m_activity.runOnUiThread( new Runnable() { public void run() { if ( null != m_jniThread ) { - m_dictChars = XwJNI.dict_getChars( dictPtr ); + m_dictChars = XwJNI.dict_getChars( newPtr ); // draw again m_jniThread.handle( JNIThread.JNICmd .CMD_INVALALL ); @@ -563,7 +566,8 @@ public class BoardCanvas extends Canvas implements DrawCtx { } }); } - m_dictPtr = dictPtr; + m_dict.release(); + m_dict = new DictWrapper( newPtr ); } } diff --git a/xwords4/android/XWords4/src/org/eehouse/android/xw4/XWApp.java b/xwords4/android/XWords4/src/org/eehouse/android/xw4/XWApp.java index 4ec4313d6..9b59ff888 100644 --- a/xwords4/android/XWords4/src/org/eehouse/android/xw4/XWApp.java +++ b/xwords4/android/XWords4/src/org/eehouse/android/xw4/XWApp.java @@ -70,6 +70,15 @@ public class XWApp extends Application { GCMIntentService.init( this ); } + // This is called on emulator only, but good for ensuring no memory leaks + // by forcing JNI cleanup + public void onTerminate() + { + DbgUtils.logf( "XwApp.onTerminate() called" ); + XwJNI.cleanGlobals(); + super.onTerminate(); + } + public static UUID getAppUUID() { if ( null == s_UUID ) { diff --git a/xwords4/android/XWords4/src/org/eehouse/android/xw4/jni/XwJNI.java b/xwords4/android/XWords4/src/org/eehouse/android/xw4/jni/XwJNI.java index c36d8e519..c9c1fb18f 100644 --- a/xwords4/android/XWords4/src/org/eehouse/android/xw4/jni/XwJNI.java +++ b/xwords4/android/XWords4/src/org/eehouse/android/xw4/jni/XwJNI.java @@ -20,11 +20,48 @@ package org.eehouse.android.xw4.jni; +import org.eehouse.android.xw4.DbgUtils; + import android.graphics.Rect; -// Collection of native methods +// Collection of native methods and a bit of state public class XwJNI { + private static XwJNI s_JNI = null; + private static XwJNI getJNI() + { + if ( null == s_JNI ) { + s_JNI = new XwJNI(); + } + return s_JNI; + } + + private int m_ptr; + private XwJNI() + { + m_ptr = initGlobals(); + } + + public static void cleanGlobals() + { + synchronized( XwJNI.class ) { // let's be safe here + XwJNI jni = getJNI(); + cleanGlobals( jni.m_ptr ); + jni.m_ptr = 0; + } + } + + // @Override + public void finalize() + { + cleanGlobals( m_ptr ); + try { + super.finalize(); + } catch ( java.lang.Throwable err ){ + DbgUtils.logf( "%s", err.toString() ); + } + } + // This needs to be called before the first attempt to use the // jni. static { @@ -52,7 +89,10 @@ public class XwJNI { public static native String comms_getUUID(); // Game methods - public static native int initJNI(); + public static int initJNI() + { + return initJNI( getJNI().m_ptr ); + } public static native void game_makeNewGame( int gamePtr, CurGameInfo gi, UtilCtxt util, @@ -263,8 +303,36 @@ public class XwJNI { public static native String comms_getStats( int gamePtr ); // Dicts - public static native boolean dict_tilesAreSame( int dictPtr1, int dictPtr2 ); - public static native String[] dict_getChars( int dictPtr ); + public static class DictWrapper { + private int m_dictPtr; + + public DictWrapper() + { + m_dictPtr = 0; + } + + public DictWrapper( int dictPtr ) + { + m_dictPtr = dictPtr; + dict_ref( dictPtr ); + } + + public void release() + { + if ( 0 != m_dictPtr ) { + dict_unref( m_dictPtr ); + m_dictPtr = 0; + } + } + + public int getDictPtr() + { + return m_dictPtr; + } + } + + public static native boolean dict_tilesAreSame( int dict1, int dict2 ); + public static native String[] dict_getChars( int dict ); public static native boolean dict_getInfo( byte[] dict, String name, String path, JNIUtils jniu, boolean check, DictInfo info ); @@ -289,4 +357,11 @@ public class XwJNI { // base64 stuff since 2.1 doesn't support it in java public static native String base64Encode( byte[] in ); public static native byte[] base64Decode( String in ); + + // Private methods -- called only here + private static native int initGlobals(); + private static native void cleanGlobals( int ptr ); + private static native int initJNI( int jniState ); + private static native void dict_ref( int dictPtr ); + private static native void dict_unref( int dictPtr ); }