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.

This commit is contained in:
Eric House 2014-03-08 20:57:27 -08:00
parent 8a6adfec34
commit a7c114e3f9
11 changed files with 73 additions and 50 deletions

View file

@ -562,7 +562,10 @@ and_dictionary_make_empty( MPFORMAL JNIEnv* env, JNIUtilCtxt* jniutil )
#endif #endif
dict_super_init( (DictionaryCtxt*)anddict ); dict_super_init( (DictionaryCtxt*)anddict );
MPASSIGN( anddict->super.mpool, mpool ); MPASSIGN( anddict->super.mpool, mpool );
return (DictionaryCtxt*)anddict;
DictionaryCtxt* result = dict_ref( (DictionaryCtxt*)anddict );
LOG_RETURNF( "%p", result );
return result;
} }
void void
@ -659,21 +662,6 @@ makeDict( MPFORMAL JNIEnv *env, JNIUtilCtxt* jniutil, jstring jname,
return (DictionaryCtxt*)anddict; return (DictionaryCtxt*)anddict;
} /* makeDict */ } /* 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 #ifdef DEBUG
uint32_t uint32_t
andDictID( const DictionaryCtxt* dict ) andDictID( const DictionaryCtxt* dict )

View file

@ -336,7 +336,7 @@ Java_org_eehouse_android_xw4_jni_XwJNI_dict_1getInfo
setInt( env, jinfo, "wordCount", dict_getWordCount( dict ) ); setInt( env, jinfo, "wordCount", dict_getWordCount( dict ) );
setString( env, jinfo, "md5Sum", dict_getMd5Sum( dict ) ); setString( env, jinfo, "md5Sum", dict_getMd5Sum( dict ) );
} }
dict_destroy( dict ); dict_unref( dict );
result = true; result = true;
} }
destroyJNIUtil( &jniutil ); destroyJNIUtil( &jniutil );
@ -481,7 +481,9 @@ Java_org_eehouse_android_xw4_jni_XwJNI_game_1makeNewGame
} }
#endif #endif
model_setDictionary( state->game.model, dict ); model_setDictionary( state->game.model, dict );
dict_unref( dict ); /* game owns it now */
model_setPlayerDicts( state->game.model, &dicts ); model_setPlayerDicts( state->game.model, &dicts );
dict_unref_all( &dicts );
XWJNI_END(); XWJNI_END();
} /* makeNewGame */ } /* makeNewGame */
@ -544,6 +546,8 @@ Java_org_eehouse_android_xw4_jni_XwJNI_game_1makeFromStream
globals->util, globals->dctx, &cp, globals->util, globals->dctx, &cp,
globals->xportProcs ); globals->xportProcs );
stream_destroy( stream ); stream_destroy( stream );
dict_unref( dict ); /* game owns it now */
dict_unref_all( &dicts );
if ( result ) { if ( result ) {
XP_ASSERT( 0 != globals->gi->gameID ); XP_ASSERT( 0 != globals->gi->gameID );
@ -553,10 +557,6 @@ Java_org_eehouse_android_xw4_jni_XwJNI_game_1makeFromStream
} else { } else {
destroyDraw( &globals->dctx ); destroyDraw( &globals->dctx );
destroyXportProcs( &globals->xportProcs ); destroyXportProcs( &globals->xportProcs );
destroyDicts( &dicts );
if ( NULL != dict ) {
dict_destroy( dict );
}
destroyUtil( &globals->util ); destroyUtil( &globals->util );
destroyJNIUtil( &globals->jniutil ); destroyJNIUtil( &globals->jniutil );
destroyGI( MPPARM(mpool) &globals->gi ); destroyGI( MPPARM(mpool) &globals->gi );
@ -1674,7 +1674,7 @@ Java_org_eehouse_android_xw4_jni_XwJNI_dict_1iter_1destroy
#ifdef MEM_DEBUG #ifdef MEM_DEBUG
MemPoolCtx* mpool = data->mpool; MemPoolCtx* mpool = data->mpool;
#endif #endif
dict_destroy( data->dict ); dict_unref( data->dict );
destroyJNIUtil( &data->jniutil ); destroyJNIUtil( &data->jniutil );
freeIndices( data ); freeIndices( data );
vtmgr_destroy( MPPARM(mpool) data->vtMgr ); vtmgr_destroy( MPPARM(mpool) data->vtMgr );

View file

@ -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 void
setBlankTile( DictionaryCtxt* dict ) setBlankTile( DictionaryCtxt* dict )
{ {

View file

@ -87,6 +87,7 @@ struct DictionaryCtxt {
XP_LangCode langCode; XP_LangCode langCode;
XP_U16 refCount;
XP_U8 nFaces; XP_U8 nFaces;
XP_U8 nodeSize; XP_U8 nodeSize;
XP_Bool is_4_byte; XP_Bool is_4_byte;
@ -125,7 +126,6 @@ struct DictionaryCtxt {
/* #define dict_numTileFaces(dc) (dc)->vtable->m_numTileFaces(dc) */ /* #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_edge_for_index(d, i) (*((d)->func_edge_for_index))((d), (i))
#define dict_getTopEdge(d) (*((d)->func_dict_getTopEdge))(d) #define dict_getTopEdge(d) (*((d)->func_dict_getTopEdge))(d)
#define dict_index_from(d,e) (*((d)->func_dict_index_from))(d,e) #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 & \ ((Tile)(((array_edge_old*)(edge))->bits & \
((d)->is_4_byte?LETTERMASK_NEW_4:LETTERMASK_NEW_3))) ((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, XP_Bool dict_tilesAreSame( const DictionaryCtxt* dict1,
const DictionaryCtxt* dict2 ); const DictionaryCtxt* dict2 );

View file

@ -203,7 +203,6 @@ game_reset( MPFORMAL XWGame* game, CurGameInfo* gi,
void void
game_changeDict( MPFORMAL XWGame* game, CurGameInfo* gi, DictionaryCtxt* dict ) game_changeDict( MPFORMAL XWGame* game, CurGameInfo* gi, DictionaryCtxt* dict )
{ {
model_destroyDicts( game->model );
model_setDictionary( game->model, dict ); model_setDictionary( game->model, dict );
gi_setDict( MPPARM(mpool) gi, dict ); gi_setDict( MPPARM(mpool) gi, dict );
server_resetEngines( game->server ); server_resetEngines( game->server );
@ -351,7 +350,6 @@ game_dispose( XWGame* game )
} }
#endif #endif
if ( !!game->model ) { if ( !!game->model ) {
model_destroyDicts( game->model );
model_destroy( game->model ); model_destroy( game->model );
game->model = NULL; game->model = NULL;
} }

View file

@ -52,6 +52,7 @@ static void notifyTrayListeners( ModelCtxt* model, XP_U16 turn,
static void notifyDictListeners( ModelCtxt* model, XP_S16 playerNum, static void notifyDictListeners( ModelCtxt* model, XP_S16 playerNum,
DictionaryCtxt* oldDict, DictionaryCtxt* oldDict,
DictionaryCtxt* newDict ); DictionaryCtxt* newDict );
static void model_unrefDicts( ModelCtxt* model );
static CellTile getModelTileRaw( const ModelCtxt* model, XP_U16 col, static CellTile getModelTileRaw( const ModelCtxt* model, XP_U16 col,
XP_U16 row ); XP_U16 row );
@ -79,6 +80,7 @@ static void writePlayerCtxt( const ModelCtxt* model, XWStreamCtxt* stream,
static XP_U16 model_getRecentPassCount( ModelCtxt* model ); static XP_U16 model_getRecentPassCount( ModelCtxt* model );
static XP_Bool recordWord( const XP_UCHAR* word, XP_Bool isLegal, static XP_Bool recordWord( const XP_UCHAR* word, XP_Bool isLegal,
const DictionaryCtxt* dict, const DictionaryCtxt* dict,
#ifdef XWFEATURE_BOARDWORDS #ifdef XWFEATURE_BOARDWORDS
const MoveInfo* movei, XP_U16 start, XP_U16 end, const MoveInfo* movei, XP_U16 start, XP_U16 end,
#endif #endif
@ -151,7 +153,7 @@ model_makeFromStream( MPFORMAL XWStreamCtxt* stream, DictionaryCtxt* dict,
if ( hasDict ) { if ( hasDict ) {
DictionaryCtxt* savedDict = util_makeEmptyDict( util ); DictionaryCtxt* savedDict = util_makeEmptyDict( util );
dict_loadFromStream( savedDict, stream ); dict_loadFromStream( savedDict, stream );
dict_destroy( savedDict ); dict_unref( savedDict );
} }
model = model_make( MPPARM(mpool) dict, dicts, util, nCols ); model = model_make( MPPARM(mpool) dict, dicts, util, nCols );
@ -299,6 +301,7 @@ model_setSize( ModelCtxt* model, XP_U16 nCols )
void void
model_destroy( ModelCtxt* model ) model_destroy( ModelCtxt* model )
{ {
model_unrefDicts( model );
stack_destroy( model->vol.stack ); stack_destroy( model->vol.stack );
/* is this it!? */ /* is this it!? */
if ( !!model->vol.bonuses ) { if ( !!model->vol.bonuses ) {
@ -491,12 +494,14 @@ void
model_setDictionary( ModelCtxt* model, DictionaryCtxt* dict ) model_setDictionary( ModelCtxt* model, DictionaryCtxt* dict )
{ {
DictionaryCtxt* oldDict = model->vol.dict; DictionaryCtxt* oldDict = model->vol.dict;
model->vol.dict = dict; model->vol.dict = dict_ref( dict );
if ( !!dict ) { if ( !!dict ) {
setStackBits( model, dict ); setStackBits( model, dict );
notifyDictListeners( model, -1, oldDict, dict );
} }
notifyDictListeners( model, -1, oldDict, dict );
dict_unref( oldDict );
} /* model_setDictionary */ } /* model_setDictionary */
void void
@ -513,9 +518,12 @@ model_setPlayerDicts( ModelCtxt* model, const PlayerDicts* dicts )
if ( oldDict != newDict ) { if ( oldDict != newDict ) {
XP_ASSERT( NULL == newDict || NULL == gameDict XP_ASSERT( NULL == newDict || NULL == gameDict
|| dict_tilesAreSame( gameDict, newDict ) ); || dict_tilesAreSame( gameDict, newDict ) );
model->vol.dicts.dicts[ii] = newDict; model->vol.dicts.dicts[ii] = dict_ref( newDict );
notifyDictListeners( model, ii, oldDict, newDict ); notifyDictListeners( model, ii, oldDict, newDict );
setStackBits( model, newDict ); setStackBits( model, newDict );
dict_unref( oldDict );
} }
} }
} }
@ -544,22 +552,15 @@ model_getPlayerDict( const ModelCtxt* model, XP_U16 playerNum )
} }
static void static void
destroyNotNull( DictionaryCtxt** dictp ) model_unrefDicts( ModelCtxt* model )
{
if ( !!*dictp ) {
dict_destroy( *dictp );
*dictp = NULL;
}
}
void
model_destroyDicts( ModelCtxt* model )
{ {
XP_U16 ii; XP_U16 ii;
for ( ii = 0; ii < VSIZE(model->vol.dicts.dicts); ++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 static XP_Bool
@ -1934,7 +1935,6 @@ static void
notifyDictListeners( ModelCtxt* model, XP_S16 playerNum, notifyDictListeners( ModelCtxt* model, XP_S16 playerNum,
DictionaryCtxt* oldDict, DictionaryCtxt* newDict ) DictionaryCtxt* oldDict, DictionaryCtxt* newDict )
{ {
XP_ASSERT( !!newDict );
if ( model->vol.dictListenerFunc != NULL ) { if ( model->vol.dictListenerFunc != NULL ) {
(*model->vol.dictListenerFunc)( model->vol.dictListenerData, playerNum, (*model->vol.dictListenerFunc)( model->vol.dictListenerData, playerNum,
oldDict, newDict ); oldDict, newDict );

View file

@ -124,7 +124,6 @@ DictionaryCtxt* model_getDictionary( const ModelCtxt* model );
void model_setPlayerDicts( ModelCtxt* model, const PlayerDicts* dicts ); void model_setPlayerDicts( ModelCtxt* model, const PlayerDicts* dicts );
DictionaryCtxt* model_getPlayerDict( const ModelCtxt* model, XP_U16 playerNum ); 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 model_getTile( const ModelCtxt* model, XP_U16 col, XP_U16 row,
XP_Bool getPending, XP_S16 turn, XP_Bool getPending, XP_S16 turn,

View file

@ -1309,7 +1309,6 @@ client_readInitialMessage( ServerCtxt* server, XWStreamCtxt* stream )
model_setDictionary( model, newDict ); model_setDictionary( model, newDict );
} else if ( dict_tilesAreSame( newDict, curDict ) ) { } else if ( dict_tilesAreSame( newDict, curDict ) ) {
/* keep the dict the local user installed */ /* keep the dict the local user installed */
dict_destroy( newDict );
#ifdef STREAM_VERS_BIGBOARD #ifdef STREAM_VERS_BIGBOARD
if ( '\0' != rmtDictName[0] ) { if ( '\0' != rmtDictName[0] ) {
const XP_UCHAR* ourName = dict_getShortName( curDict ); const XP_UCHAR* ourName = dict_getShortName( curDict );
@ -1320,11 +1319,11 @@ client_readInitialMessage( ServerCtxt* server, XWStreamCtxt* stream )
} }
#endif #endif
} else { } else {
dict_destroy( curDict );
model_setDictionary( model, newDict ); model_setDictionary( model, newDict );
util_userError( server->vol.util, ERR_SERVER_DICT_WINS ); util_userError( server->vol.util, ERR_SERVER_DICT_WINS );
clearLocalRobots( server ); clearLocalRobots( server );
} }
dict_unref( newDict ); /* new owner will have ref'd */
XP_ASSERT( !server->pool ); XP_ASSERT( !server->pool );
pool = server->pool = pool_make( MPPARM_NOCOMMA(server->mpool) ); pool = server->pool = pool_make( MPPARM_NOCOMMA(server->mpool) );

View file

@ -801,8 +801,9 @@ cleanup( GtkGameGlobals* globals )
#ifdef XWFEATURE_RELAY #ifdef XWFEATURE_RELAY
linux_close_socket( cGlobals ); linux_close_socket( cGlobals );
#endif #endif
game_dispose( &cGlobals->game ); /* takes care of the dict */ game_dispose( &cGlobals->game );
gi_disposePlayerInfo( MEMPOOL cGlobals->gi ); gi_disposePlayerInfo( MEMPOOL cGlobals->gi );
dict_unref( cGlobals->dict );
linux_util_vt_destroy( cGlobals->util ); linux_util_vt_destroy( cGlobals->util );
free( cGlobals->util ); free( cGlobals->util );

View file

@ -83,7 +83,7 @@ linux_dictionary_make( MPFORMAL const LaunchParams* params,
} }
} }
return (DictionaryCtxt*)result; return dict_ref( (DictionaryCtxt*)result );
} /* gtk_dictionary_make */ } /* gtk_dictionary_make */
static XP_UCHAR* static XP_UCHAR*

View file

@ -1688,7 +1688,7 @@ walk_dict_test_all( MPFORMAL const LaunchParams* params, GSList* testDicts,
if ( NULL != dict ) { if ( NULL != dict ) {
XP_LOGF( "walk_dict_test(%s)", name ); XP_LOGF( "walk_dict_test(%s)", name );
walk_dict_test( MPPARM(mpool) dict, testPrefixes, testMinMax ); 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 ); params->useMmap );
if ( NULL != dict ) { if ( NULL != dict ) {
dumpDict( dict ); dumpDict( dict );
dict_destroy( dict ); dict_unref( dict );
} }
} }
return 0; return 0;