From a7c114e3f9acf8d6a955c127c4afca0a3dc333b5 Mon Sep 17 00:00:00 2001 From: Eric House Date: Sat, 8 Mar 2014 20:57:27 -0800 Subject: [PATCH] refcount dicts. Model "owns" its copy and so increments the refcount when gaining one and decrements the count on any being replaced (and on all on exit). This is setting up the real change, which is to let the java world wrap dicts in objects that hang onto them until they're destroyed, which should fix problems where dicts are referenced after they've been destroyed. --- xwords4/android/XWords4/jni/anddict.c | 20 +++------------ xwords4/android/XWords4/jni/xwjni.c | 12 ++++----- xwords4/common/dictnry.c | 34 +++++++++++++++++++++++++ xwords4/common/dictnry.h | 6 ++++- xwords4/common/game.c | 2 -- xwords4/common/model.c | 36 +++++++++++++-------------- xwords4/common/model.h | 1 - xwords4/common/server.c | 3 +-- xwords4/linux/gtkboard.c | 3 ++- xwords4/linux/linuxdict.c | 2 +- xwords4/linux/linuxmain.c | 4 +-- 11 files changed, 73 insertions(+), 50 deletions(-) diff --git a/xwords4/android/XWords4/jni/anddict.c b/xwords4/android/XWords4/jni/anddict.c index 7a17afe99..f6aebffc5 100644 --- a/xwords4/android/XWords4/jni/anddict.c +++ b/xwords4/android/XWords4/jni/anddict.c @@ -562,7 +562,10 @@ and_dictionary_make_empty( MPFORMAL JNIEnv* env, JNIUtilCtxt* jniutil ) #endif dict_super_init( (DictionaryCtxt*)anddict ); MPASSIGN( anddict->super.mpool, mpool ); - return (DictionaryCtxt*)anddict; + + DictionaryCtxt* result = dict_ref( (DictionaryCtxt*)anddict ); + LOG_RETURNF( "%p", result ); + return result; } void @@ -659,21 +662,6 @@ makeDict( MPFORMAL JNIEnv *env, JNIUtilCtxt* jniutil, jstring jname, return (DictionaryCtxt*)anddict; } /* makeDict */ -void -destroyDicts( PlayerDicts* dicts ) -{ - int ii; - DictionaryCtxt** ctxts; - - for ( ctxts = dicts->dicts, ii = 0; - ii < VSIZE(dicts->dicts); - ++ii, ++ctxts ) { - if ( NULL != *ctxts ) { - dict_destroy( *ctxts ); - } - } -} - #ifdef DEBUG uint32_t andDictID( const DictionaryCtxt* dict ) diff --git a/xwords4/android/XWords4/jni/xwjni.c b/xwords4/android/XWords4/jni/xwjni.c index 34f636980..504b42b5d 100644 --- a/xwords4/android/XWords4/jni/xwjni.c +++ b/xwords4/android/XWords4/jni/xwjni.c @@ -336,7 +336,7 @@ Java_org_eehouse_android_xw4_jni_XwJNI_dict_1getInfo setInt( env, jinfo, "wordCount", dict_getWordCount( dict ) ); setString( env, jinfo, "md5Sum", dict_getMd5Sum( dict ) ); } - dict_destroy( dict ); + dict_unref( dict ); result = true; } destroyJNIUtil( &jniutil ); @@ -481,7 +481,9 @@ Java_org_eehouse_android_xw4_jni_XwJNI_game_1makeNewGame } #endif model_setDictionary( state->game.model, dict ); + dict_unref( dict ); /* game owns it now */ model_setPlayerDicts( state->game.model, &dicts ); + dict_unref_all( &dicts ); XWJNI_END(); } /* makeNewGame */ @@ -544,6 +546,8 @@ Java_org_eehouse_android_xw4_jni_XwJNI_game_1makeFromStream globals->util, globals->dctx, &cp, globals->xportProcs ); stream_destroy( stream ); + dict_unref( dict ); /* game owns it now */ + dict_unref_all( &dicts ); if ( result ) { XP_ASSERT( 0 != globals->gi->gameID ); @@ -553,10 +557,6 @@ Java_org_eehouse_android_xw4_jni_XwJNI_game_1makeFromStream } else { destroyDraw( &globals->dctx ); destroyXportProcs( &globals->xportProcs ); - destroyDicts( &dicts ); - if ( NULL != dict ) { - dict_destroy( dict ); - } destroyUtil( &globals->util ); destroyJNIUtil( &globals->jniutil ); destroyGI( MPPARM(mpool) &globals->gi ); @@ -1674,7 +1674,7 @@ Java_org_eehouse_android_xw4_jni_XwJNI_dict_1iter_1destroy #ifdef MEM_DEBUG MemPoolCtx* mpool = data->mpool; #endif - dict_destroy( data->dict ); + dict_unref( data->dict ); destroyJNIUtil( &data->jniutil ); freeIndices( data ); vtmgr_destroy( MPPARM(mpool) data->vtMgr ); diff --git a/xwords4/common/dictnry.c b/xwords4/common/dictnry.c index 2a1daba2e..98ef3c4db 100644 --- a/xwords4/common/dictnry.c +++ b/xwords4/common/dictnry.c @@ -38,6 +38,40 @@ extern "C" { /***************************************************************************** * ****************************************************************************/ + +DictionaryCtxt* +dict_ref( DictionaryCtxt* dict ) +{ + if ( !!dict ) { + ++dict->refCount; + XP_LOGF( "%s(dict=%p): refCount now %d", __func__, dict, dict->refCount ); + } + return dict; +} + +void +dict_unref( DictionaryCtxt* dict ) +{ + if ( !!dict ) { + --dict->refCount; + XP_ASSERT( 0 <= dict->refCount ); + XP_LOGF( "%s(dict=%p): refCount now %d", __func__, dict, + dict->refCount ); + if ( 0 == dict->refCount ) { + (*dict->destructor)( dict ); + } + } +} + +void +dict_unref_all( PlayerDicts* pd ) +{ + XP_U16 ii; + for ( ii = 0; ii < MAX_NUM_PLAYERS; ++ii ) { + dict_unref( pd->dicts[ii] ); + } +} + void setBlankTile( DictionaryCtxt* dict ) { diff --git a/xwords4/common/dictnry.h b/xwords4/common/dictnry.h index 8f6ae4f18..15d27afce 100644 --- a/xwords4/common/dictnry.h +++ b/xwords4/common/dictnry.h @@ -87,6 +87,7 @@ struct DictionaryCtxt { XP_LangCode langCode; + XP_U16 refCount; XP_U8 nFaces; XP_U8 nodeSize; XP_Bool is_4_byte; @@ -125,7 +126,6 @@ struct DictionaryCtxt { /* #define dict_numTileFaces(dc) (dc)->vtable->m_numTileFaces(dc) */ -#define dict_destroy(d) (*((d)->destructor))(d) #define dict_edge_for_index(d, i) (*((d)->func_edge_for_index))((d), (i)) #define dict_getTopEdge(d) (*((d)->func_dict_getTopEdge))(d) #define dict_index_from(d,e) (*((d)->func_dict_index_from))(d,e) @@ -141,6 +141,10 @@ struct DictionaryCtxt { ((Tile)(((array_edge_old*)(edge))->bits & \ ((d)->is_4_byte?LETTERMASK_NEW_4:LETTERMASK_NEW_3))) +DictionaryCtxt* dict_ref( DictionaryCtxt* dict ); +void dict_unref( DictionaryCtxt* dict ); +void dict_unref_all( PlayerDicts* dicts ); + XP_Bool dict_tilesAreSame( const DictionaryCtxt* dict1, const DictionaryCtxt* dict2 ); diff --git a/xwords4/common/game.c b/xwords4/common/game.c index c9b0ddfd0..2d5bf734c 100644 --- a/xwords4/common/game.c +++ b/xwords4/common/game.c @@ -203,7 +203,6 @@ game_reset( MPFORMAL XWGame* game, CurGameInfo* gi, void game_changeDict( MPFORMAL XWGame* game, CurGameInfo* gi, DictionaryCtxt* dict ) { - model_destroyDicts( game->model ); model_setDictionary( game->model, dict ); gi_setDict( MPPARM(mpool) gi, dict ); server_resetEngines( game->server ); @@ -351,7 +350,6 @@ game_dispose( XWGame* game ) } #endif if ( !!game->model ) { - model_destroyDicts( game->model ); model_destroy( game->model ); game->model = NULL; } diff --git a/xwords4/common/model.c b/xwords4/common/model.c index ec5e4e07c..ca383163d 100644 --- a/xwords4/common/model.c +++ b/xwords4/common/model.c @@ -52,6 +52,7 @@ static void notifyTrayListeners( ModelCtxt* model, XP_U16 turn, static void notifyDictListeners( ModelCtxt* model, XP_S16 playerNum, DictionaryCtxt* oldDict, DictionaryCtxt* newDict ); +static void model_unrefDicts( ModelCtxt* model ); static CellTile getModelTileRaw( const ModelCtxt* model, XP_U16 col, XP_U16 row ); @@ -79,6 +80,7 @@ static void writePlayerCtxt( const ModelCtxt* model, XWStreamCtxt* stream, static XP_U16 model_getRecentPassCount( ModelCtxt* model ); static XP_Bool recordWord( const XP_UCHAR* word, XP_Bool isLegal, const DictionaryCtxt* dict, + #ifdef XWFEATURE_BOARDWORDS const MoveInfo* movei, XP_U16 start, XP_U16 end, #endif @@ -151,7 +153,7 @@ model_makeFromStream( MPFORMAL XWStreamCtxt* stream, DictionaryCtxt* dict, if ( hasDict ) { DictionaryCtxt* savedDict = util_makeEmptyDict( util ); dict_loadFromStream( savedDict, stream ); - dict_destroy( savedDict ); + dict_unref( savedDict ); } model = model_make( MPPARM(mpool) dict, dicts, util, nCols ); @@ -299,6 +301,7 @@ model_setSize( ModelCtxt* model, XP_U16 nCols ) void model_destroy( ModelCtxt* model ) { + model_unrefDicts( model ); stack_destroy( model->vol.stack ); /* is this it!? */ if ( !!model->vol.bonuses ) { @@ -491,12 +494,14 @@ void model_setDictionary( ModelCtxt* model, DictionaryCtxt* dict ) { DictionaryCtxt* oldDict = model->vol.dict; - model->vol.dict = dict; - + model->vol.dict = dict_ref( dict ); + if ( !!dict ) { setStackBits( model, dict ); - notifyDictListeners( model, -1, oldDict, dict ); } + + notifyDictListeners( model, -1, oldDict, dict ); + dict_unref( oldDict ); } /* model_setDictionary */ void @@ -513,9 +518,12 @@ model_setPlayerDicts( ModelCtxt* model, const PlayerDicts* dicts ) if ( oldDict != newDict ) { XP_ASSERT( NULL == newDict || NULL == gameDict || dict_tilesAreSame( gameDict, newDict ) ); - model->vol.dicts.dicts[ii] = newDict; + model->vol.dicts.dicts[ii] = dict_ref( newDict ); + notifyDictListeners( model, ii, oldDict, newDict ); setStackBits( model, newDict ); + + dict_unref( oldDict ); } } } @@ -544,22 +552,15 @@ model_getPlayerDict( const ModelCtxt* model, XP_U16 playerNum ) } static void -destroyNotNull( DictionaryCtxt** dictp ) -{ - if ( !!*dictp ) { - dict_destroy( *dictp ); - *dictp = NULL; - } -} - -void -model_destroyDicts( ModelCtxt* model ) +model_unrefDicts( ModelCtxt* model ) { XP_U16 ii; for ( ii = 0; ii < VSIZE(model->vol.dicts.dicts); ++ii ) { - destroyNotNull( &model->vol.dicts.dicts[ii] ); + dict_unref( model->vol.dicts.dicts[ii] ); + model->vol.dicts.dicts[ii] = NULL; } - destroyNotNull( &model->vol.dict ); + dict_unref( model->vol.dict ); + model->vol.dict = NULL; } static XP_Bool @@ -1934,7 +1935,6 @@ static void notifyDictListeners( ModelCtxt* model, XP_S16 playerNum, DictionaryCtxt* oldDict, DictionaryCtxt* newDict ) { - XP_ASSERT( !!newDict ); if ( model->vol.dictListenerFunc != NULL ) { (*model->vol.dictListenerFunc)( model->vol.dictListenerData, playerNum, oldDict, newDict ); diff --git a/xwords4/common/model.h b/xwords4/common/model.h index dff98fc3d..7df036386 100644 --- a/xwords4/common/model.h +++ b/xwords4/common/model.h @@ -124,7 +124,6 @@ DictionaryCtxt* model_getDictionary( const ModelCtxt* model ); void model_setPlayerDicts( ModelCtxt* model, const PlayerDicts* dicts ); DictionaryCtxt* model_getPlayerDict( const ModelCtxt* model, XP_U16 playerNum ); -void model_destroyDicts( ModelCtxt* model ); XP_Bool model_getTile( const ModelCtxt* model, XP_U16 col, XP_U16 row, XP_Bool getPending, XP_S16 turn, diff --git a/xwords4/common/server.c b/xwords4/common/server.c index 162cb1187..b804a9c90 100644 --- a/xwords4/common/server.c +++ b/xwords4/common/server.c @@ -1309,7 +1309,6 @@ client_readInitialMessage( ServerCtxt* server, XWStreamCtxt* stream ) model_setDictionary( model, newDict ); } else if ( dict_tilesAreSame( newDict, curDict ) ) { /* keep the dict the local user installed */ - dict_destroy( newDict ); #ifdef STREAM_VERS_BIGBOARD if ( '\0' != rmtDictName[0] ) { const XP_UCHAR* ourName = dict_getShortName( curDict ); @@ -1320,11 +1319,11 @@ client_readInitialMessage( ServerCtxt* server, XWStreamCtxt* stream ) } #endif } else { - dict_destroy( curDict ); model_setDictionary( model, newDict ); util_userError( server->vol.util, ERR_SERVER_DICT_WINS ); clearLocalRobots( server ); } + dict_unref( newDict ); /* new owner will have ref'd */ XP_ASSERT( !server->pool ); pool = server->pool = pool_make( MPPARM_NOCOMMA(server->mpool) ); diff --git a/xwords4/linux/gtkboard.c b/xwords4/linux/gtkboard.c index 8088cf3ae..f7e15d4a7 100644 --- a/xwords4/linux/gtkboard.c +++ b/xwords4/linux/gtkboard.c @@ -801,8 +801,9 @@ cleanup( GtkGameGlobals* globals ) #ifdef XWFEATURE_RELAY linux_close_socket( cGlobals ); #endif - game_dispose( &cGlobals->game ); /* takes care of the dict */ + game_dispose( &cGlobals->game ); gi_disposePlayerInfo( MEMPOOL cGlobals->gi ); + dict_unref( cGlobals->dict ); linux_util_vt_destroy( cGlobals->util ); free( cGlobals->util ); diff --git a/xwords4/linux/linuxdict.c b/xwords4/linux/linuxdict.c index dd18e0ee7..0a20e7c0d 100644 --- a/xwords4/linux/linuxdict.c +++ b/xwords4/linux/linuxdict.c @@ -83,7 +83,7 @@ linux_dictionary_make( MPFORMAL const LaunchParams* params, } } - return (DictionaryCtxt*)result; + return dict_ref( (DictionaryCtxt*)result ); } /* gtk_dictionary_make */ static XP_UCHAR* diff --git a/xwords4/linux/linuxmain.c b/xwords4/linux/linuxmain.c index 757614c1e..eae14366e 100644 --- a/xwords4/linux/linuxmain.c +++ b/xwords4/linux/linuxmain.c @@ -1688,7 +1688,7 @@ walk_dict_test_all( MPFORMAL const LaunchParams* params, GSList* testDicts, if ( NULL != dict ) { XP_LOGF( "walk_dict_test(%s)", name ); walk_dict_test( MPPARM(mpool) dict, testPrefixes, testMinMax ); - dict_destroy( dict ); + dict_unref( dict ); } } } @@ -1895,7 +1895,7 @@ dawg2dict( const LaunchParams* params, GSList* testDicts ) params->useMmap ); if ( NULL != dict ) { dumpDict( dict ); - dict_destroy( dict ); + dict_unref( dict ); } } return 0;