diff --git a/xwords4/android/app/src/main/java/org/eehouse/android/xw4/BoardDelegate.java b/xwords4/android/app/src/main/java/org/eehouse/android/xw4/BoardDelegate.java index 3269eb9a3..9b2b2a18c 100644 --- a/xwords4/android/app/src/main/java/org/eehouse/android/xw4/BoardDelegate.java +++ b/xwords4/android/app/src/main/java/org/eehouse/android/xw4/BoardDelegate.java @@ -587,17 +587,21 @@ public class BoardDelegate extends DelegateBase m_haveInvited = args.getBoolean( GameUtils.INVITED, false ); m_overNotShown = true; + // getRetained() can in threory fail to get the lock and so will + // return null. Let m_jniThreadRef stay null in that case; doResume() + // will finish() in that case. m_jniThreadRef = JNIThread.getRetained( m_rowid, true ); + if ( null != m_jniThreadRef ) { + // see http://stackoverflow.com/questions/680180/where-to-stop- \ + // destroy-threads-in-android-service-class + m_jniThreadRef.setDaemonOnce( true ); + m_jniThreadRef.startOnce(); - // see http://stackoverflow.com/questions/680180/where-to-stop- \ - // destroy-threads-in-android-service-class - m_jniThreadRef.setDaemonOnce( true ); // firing - m_jniThreadRef.startOnce(); + NFCUtils.register( m_activity, this ); // Don't seem to need to unregister... - NFCUtils.register( m_activity, this ); // Don't seem to need to unregister... - - setBackgroundColor(); - setKeepScreenOn(); + setBackgroundColor(); + setKeepScreenOn(); + } } // init @Override @@ -2067,9 +2071,9 @@ public class BoardDelegate extends DelegateBase private void doResume( boolean isStart ) { - boolean success = true; + boolean success = null != m_jniThreadRef; boolean firstStart = null == m_handler; - if ( firstStart ) { + if ( success && firstStart ) { m_handler = new Handler(); success = m_jniThreadRef.configure( m_activity, m_view, m_utils, this, @@ -2777,12 +2781,12 @@ public class BoardDelegate extends DelegateBase summary = thread.getSummary(); gi = thread.getGI(); } else { - GameLock lock = new GameLock( rowID, false ); - if ( lock.tryLock() ) { - summary = DBUtils.getSummary( activity, lock ); - gi = new CurGameInfo( activity ); - gamePtr = GameUtils.loadMakeGame( activity, gi, lock ); - lock.unlock(); + try ( GameLock lock = GameLock.getFor( rowID ).tryLockRO() ) { + if ( null != lock ) { + summary = DBUtils.getSummary( activity, lock ); + gi = new CurGameInfo( activity ); + gamePtr = GameUtils.loadMakeGame( activity, gi, lock ); + } } } diff --git a/xwords4/android/app/src/main/java/org/eehouse/android/xw4/DBUtils.java b/xwords4/android/app/src/main/java/org/eehouse/android/xw4/DBUtils.java index 173c0d05e..e0f9a1438 100644 --- a/xwords4/android/app/src/main/java/org/eehouse/android/xw4/DBUtils.java +++ b/xwords4/android/app/src/main/java/org/eehouse/android/xw4/DBUtils.java @@ -366,22 +366,27 @@ public class DBUtils { public static void addRematchInfo( Context context, long rowid, String btAddr, String phone, String relayID, String p2pAddr ) { - GameLock lock = new GameLock( rowid, true ).lock(); - GameSummary summary = getSummary( context, lock ); - if ( null != btAddr ) { - summary.putStringExtra( GameSummary.EXTRA_REMATCH_BTADDR, btAddr ); + try ( GameLock lock = GameLock.getFor( rowid ).tryLock() ) { + Assert.assertNotNull( lock ); + if ( null != lock ) { + GameSummary summary = getSummary( context, lock ); + if ( null != btAddr ) { + summary.putStringExtra( GameSummary.EXTRA_REMATCH_BTADDR, btAddr ); + } + if ( null != phone ) { + summary.putStringExtra( GameSummary.EXTRA_REMATCH_PHONE, phone ); + } + if ( null != relayID ) { + summary.putStringExtra( GameSummary.EXTRA_REMATCH_RELAY, relayID ); + } + if ( null != p2pAddr ) { + summary.putStringExtra( GameSummary.EXTRA_REMATCH_P2P, p2pAddr ); + } + saveSummary( context, lock, summary ); + } else { + Log.e( TAG, "addRematchInfo(%d): unable to lock game" ); + } } - if ( null != phone ) { - summary.putStringExtra( GameSummary.EXTRA_REMATCH_PHONE, phone ); - } - if ( null != relayID ) { - summary.putStringExtra( GameSummary.EXTRA_REMATCH_RELAY, relayID ); - } - if ( null != p2pAddr ) { - summary.putStringExtra( GameSummary.EXTRA_REMATCH_P2P, p2pAddr ); - } - saveSummary( context, lock, summary ); - lock.unlock(); } public static int countGamesUsingLang( Context context, int lang ) @@ -1052,7 +1057,8 @@ public class DBUtils { setCached( rowid, null ); // force reread - lock = new GameLock( rowid, true ).lock(); + lock = GameLock.getFor( rowid ).tryLock(); + Assert.assertNotNull( lock ); notifyListeners( rowid, GameChangeType.GAME_CREATED ); } @@ -1113,14 +1119,14 @@ public class DBUtils { public static void deleteGame( Context context, long rowid ) { - GameLock lock = new GameLock( rowid, true ).lock( 300 ); - if ( null != lock ) { - deleteGame( context, lock ); - lock.unlock(); - } else { - Log.e( TAG, "deleteGame: unable to lock rowid %d", rowid ); - if ( BuildConfig.DEBUG ) { - Assert.fail(); + try ( GameLock lock = GameLock.getFor( rowid ).lock( 300 ) ) { + if ( null != lock ) { + deleteGame( context, lock ); + } else { + Log.e( TAG, "deleteGame: unable to lock rowid %d", rowid ); + if ( BuildConfig.DEBUG ) { + Assert.fail(); + } } } } diff --git a/xwords4/android/app/src/main/java/org/eehouse/android/xw4/GameConfigDelegate.java b/xwords4/android/app/src/main/java/org/eehouse/android/xw4/GameConfigDelegate.java index 9c5f1d1ee..b757a4c26 100644 --- a/xwords4/android/app/src/main/java/org/eehouse/android/xw4/GameConfigDelegate.java +++ b/xwords4/android/app/src/main/java/org/eehouse/android/xw4/GameConfigDelegate.java @@ -536,10 +536,14 @@ public class GameConfigDelegate extends DelegateBase if ( null == m_giOrig ) { m_giOrig = new CurGameInfo( m_activity ); - GameLock gameLock; + GameLock gameLock = null; XwJNI.GamePtr gamePtr; if ( null == m_jniThread ) { - gameLock = new GameLock( m_rowid, false ).lock(); + try { + gameLock = GameLock.getFor( m_rowid ).lockRO(); + } catch (InterruptedException iex) { + Assert.assertFalse( BuildConfig.DEBUG ); + } gamePtr = GameUtils.loadMakeGame( m_activity, m_giOrig, gameLock ); } else { gameLock = m_jniThread.getLock(); @@ -1230,14 +1234,18 @@ public class GameConfigDelegate extends DelegateBase private void applyChanges( boolean forceNew ) { if ( !isFinishing() ) { - GameLock gameLock = m_jniThread == null - ? new GameLock( m_rowid, true ).lock() - : m_jniThread.getLock(); - GameUtils.applyChanges( m_activity, m_gi, m_car, m_disabMap, - gameLock, forceNew ); - DBUtils.saveThumbnail( m_activity, gameLock, null ); // clear it - if ( null == m_jniThread ) { - gameLock.unlock(); + try { + GameLock gameLock = m_jniThread == null + ? GameLock.getFor( m_rowid ).lock() + : m_jniThread.getLock(); + GameUtils.applyChanges( m_activity, m_gi, m_car, m_disabMap, + gameLock, forceNew ); + DBUtils.saveThumbnail( m_activity, gameLock, null ); // clear it + if ( null == m_jniThread ) { + gameLock.unlock(); + } + } catch (InterruptedException iex) { + Assert.assertFalse( BuildConfig.DEBUG ); } } } diff --git a/xwords4/android/app/src/main/java/org/eehouse/android/xw4/GameLock.java b/xwords4/android/app/src/main/java/org/eehouse/android/xw4/GameLock.java index 85d66310b..aecd935bf 100644 --- a/xwords4/android/app/src/main/java/org/eehouse/android/xw4/GameLock.java +++ b/xwords4/android/app/src/main/java/org/eehouse/android/xw4/GameLock.java @@ -20,178 +20,218 @@ package org.eehouse.android.xw4; - +import java.io.Serializable; +import java.lang.ref.WeakReference; import java.util.HashMap; +import java.util.Map; +import java.util.Stack; // Implements read-locks and write-locks per game. A read lock is // obtainable when other read locks are granted but not when a // write lock is. Write-locks are exclusive. -public class GameLock { +public class GameLock implements AutoCloseable { private static final String TAG = GameLock.class.getSimpleName(); + + public interface GotLock { + void onGotLock( GameLock lock ); + } private static final boolean DEBUG_LOCKS = false; - private static final boolean THROW_ON_LOCKED = true; - private static final int SLEEP_TIME = 100; - private static final long ASSERT_TIME = 2000; + // private static final long ASSERT_TIME = 2000; private static final long THROW_TIME = 1000; private long m_rowid; - private boolean m_isForWrite; - private int m_lockCount; - private Thread m_ownerThread; - private StackTraceElement[] m_lockTrace; + private Stack mOwners = new Stack<>(); + private boolean mReadOnly; - static { - Assert.assertTrue( THROW_TIME <= ASSERT_TIME ); + private static class Owner { + Thread mThread; + String mTrace; + + Owner() + { + mThread = Thread.currentThread(); + // mTrace = mThread.getStackTrace(); + mTrace = android.util.Log.getStackTraceString(new Exception()); + } + + @Override + public String toString() + { + return String.format( "Owner: {%s/%s}", mThread, mTrace ); + } + } + + @Override + public String toString() + { + return String.format("{this: %H; rowid: %d; count: %d; ro: %b}", + this, m_rowid, mOwners.size(), mReadOnly); } public static class GameLockedException extends RuntimeException {} - private static HashMap - s_locks = new HashMap(); - - public GameLock( long rowid, boolean isForWrite ) + private static Map> sLockMap = new HashMap<>(); + public static GameLock getFor( long rowid ) { - Assert.assertTrue( DBUtils.ROWID_NOTFOUND != rowid ); - m_rowid = rowid; - m_isForWrite = isForWrite; - m_lockCount = 0; - if ( DEBUG_LOCKS ) { - Log.i( TAG, "GameLock(rowid:%d,isForWrite:%b)=>this: %H", - rowid, isForWrite, this ); - DbgUtils.printStack( TAG ); - } - } - - // This could be written to allow multiple read locks. Let's - // see if not doing that causes problems. - public boolean tryLock() - { - return null == tryLockImpl(); // unowned? - } - - private GameLock tryLockImpl() - { - GameLock owner = null; - boolean gotIt = false; - synchronized( s_locks ) { - owner = s_locks.get( m_rowid ); - if ( null == owner ) { // unowned - Assert.assertTrue( 0 == m_lockCount ); - s_locks.put( m_rowid, this ); - if ( BuildConfig.DEBUG ) { - m_ownerThread = Thread.currentThread(); - } - ++m_lockCount; - gotIt = true; - - if ( DEBUG_LOCKS ) { - StackTraceElement[] trace - = Thread.currentThread().getStackTrace(); - m_lockTrace = new StackTraceElement[trace.length]; - System.arraycopy( trace, 0, m_lockTrace, 0, trace.length ); - } - } else if ( this == owner && ! m_isForWrite ) { - if ( DEBUG_LOCKS ) { - Log.i( TAG, "tryLock(): incrementing lock count" ); - } - Assert.assertTrue( 0 == m_lockCount ); - ++m_lockCount; - gotIt = true; - owner = null; - } else if ( DEBUG_LOCKS ) { - Log.i( TAG, "tryLock(): rowid %d already held by lock %H", - m_rowid, owner ); + GameLock result = null; + synchronized ( sLockMap ) { + if ( sLockMap.containsKey( rowid ) ) { + result = sLockMap.get( rowid ).get(); + } + if ( null == result ) { + result = new GameLock( rowid ); + sLockMap.put( rowid, new WeakReference(result) ); } } - if ( DEBUG_LOCKS ) { - Log.i( TAG, "tryLock %H (rowid=%d) => %b", this, m_rowid, gotIt ); - } - return owner; + return result; } - // Wait forever (but may assert if too long) - public GameLock lock() + private GameLock( long rowid ) { m_rowid = rowid; } + + // We grant a lock IFF: + // * Count is 0 + // OR + // * existing locks are ReadOnly and this request is readOnly + // OR + // * the requesting thread already holds the lock + private GameLock tryLockImpl( boolean readOnly ) { - return this.lock( 0 ); + GameLock result = null; + synchronized ( mOwners ) { + if ( DEBUG_LOCKS ) { + Log.d( TAG, "%s.tryLockImpl(ro=%b)", this, readOnly ); + } + // Thread thisThread = Thread.currentThread(); + boolean grant = false; + if ( mOwners.empty() ) { + grant = true; + } else if ( mReadOnly && readOnly ) { + grant = true; + // } else if ( thisThread == mOwnerThread ) { + // grant = true; + } + + if ( grant ) { + mOwners.push( new Owner() ); + mReadOnly = readOnly; + result = this; + } + if ( DEBUG_LOCKS ) { + Log.d( TAG, "%s.tryLockImpl(ro=%b) => %s", this, readOnly, result ); + if ( null == result ) { + Log.d( TAG, "Unable to lock; cur owner: %s; would-be owner: %s", + mOwners.peek(), new Owner() ); + } + } + } + return result; + } + + // // This could be written to allow multiple read locks. Let's + // // see if not doing that causes problems. + public GameLock tryLock() + { + return tryLockImpl( false ); + } + + public GameLock tryLockRO() + { + return tryLockImpl( true ); + } + + private GameLock lockImpl( long timeoutMS, boolean readOnly ) throws InterruptedException + { + long startMS = System.currentTimeMillis(); + long endMS = startMS + timeoutMS; + synchronized ( mOwners ) { + while ( null == tryLockImpl( readOnly ) ) { + long now = System.currentTimeMillis(); + if ( now >= endMS ) { + throw new GameLockedException(); + } + mOwners.wait( endMS - now ); + } + } + + if ( DEBUG_LOCKS ) { + long tookMS = System.currentTimeMillis() - startMS; + Log.d( TAG, "%s.lockImpl() returning after %d ms", this, tookMS ); + } + return this; + } + + public GameLock lock() throws InterruptedException + { + if ( BuildConfig.DEBUG ) { + DbgUtils.assertOnUIThread( false ); + } + return lockImpl( Long.MAX_VALUE, false ); + } + + public GameLock lockRO() throws InterruptedException + { + if ( BuildConfig.DEBUG ) { + DbgUtils.assertOnUIThread( false ); + } + return lockImpl( Long.MAX_VALUE, true ); } // Version that's allowed to return null -- if maxMillis > 0 public GameLock lock( long maxMillis ) { - GameLock result = null; - Assert.assertTrue( maxMillis <= ASSERT_TIME ); Assert.assertTrue( maxMillis <= THROW_TIME ); - long sleptTime = 0; - + GameLock result = null; + try { + result = lockImpl( maxMillis, false ); + } catch (InterruptedException ex) { + Log.d( TAG, "lock(): got %s", ex.getMessage() ); + } if ( DEBUG_LOCKS ) { - Log.i( TAG, "lock %H (rowid:%d, maxMillis=%d)", this, m_rowid, - maxMillis ); + Log.d( TAG, "%s.lock(%d) => %s", this, maxMillis, result ); } - - for ( ; ; ) { - GameLock curOwner = tryLockImpl(); - if ( null == curOwner ) { - result = this; - break; - } - if ( DEBUG_LOCKS ) { - Log.i( TAG, "lock() %H failed; sleeping", this ); - if ( 0 == sleptTime || sleptTime + SLEEP_TIME >= ASSERT_TIME ) { - Log.i( TAG, "lock %H seeking stack:", curOwner ); - DbgUtils.printStack( TAG, curOwner.m_lockTrace ); - Log.i( TAG, "lock %H seeking stack:", this ); - DbgUtils.printStack( TAG ); - } - } - try { - Thread.sleep( SLEEP_TIME ); // milliseconds - sleptTime += SLEEP_TIME; - } catch( InterruptedException ie ) { - Log.ex( TAG, ie ); - break; - } - - if ( DEBUG_LOCKS ) { - Log.i( TAG, "lock() %H awake; " - + "sleptTime now %d millis", this, sleptTime ); - } - - if ( 0 < maxMillis && sleptTime >= maxMillis ) { - break; - } else if ( THROW_ON_LOCKED && sleptTime >= THROW_TIME ) { - throw new GameLockedException(); - } else if ( sleptTime >= ASSERT_TIME ) { - if ( DEBUG_LOCKS ) { - Log.i( TAG, "lock %H overlocked", this ); - } - Assert.fail(); - } - } - // DbgUtils.logf( "GameLock.lock(%s) done", m_path ); return result; } + public GameLock lockRO( long maxMillis ) + { + Assert.assertTrue( maxMillis <= THROW_TIME ); + GameLock lock = null; + try { + lock = lockImpl( maxMillis, true ); + } catch ( InterruptedException ex ) { + } + return lock; + } + public void unlock() { - // DbgUtils.logf( "GameLock.unlock(%s)", m_path ); - synchronized( s_locks ) { - Assert.assertTrue( this == s_locks.get(m_rowid) ); - // Need to get this working before can switch to using ReentrantLock - // if ( BuildConfig.DEBUG ) { - // Assert.assertTrue( Thread.currentThread().equals(m_ownerThread) ); - // } - if ( 1 == m_lockCount ) { - s_locks.remove( m_rowid ); - } else { - Assert.assertTrue( !m_isForWrite ); - } - --m_lockCount; - + synchronized ( mOwners ) { if ( DEBUG_LOCKS ) { - Log.i( TAG, "unlock: this: %H (rowid:%d) unlocked", this, m_rowid ); + Log.d( TAG, "%s.unlock()", this ); + } + Thread oldThread = mOwners.pop().mThread; + + // It's ok for different threads to hold the same RO lock + if ( !mReadOnly && oldThread != Thread.currentThread() ) { + Log.e( TAG, "unlock(): unequal threads: %s => %s", oldThread, + Thread.currentThread() ); + Assert.fail(); + } + + if ( mOwners.empty() ) { + mOwners.notifyAll(); + } + if ( DEBUG_LOCKS ) { + Log.d( TAG, "%s.unlock() DONE", this ); } } } + @Override + public void close() + { + unlock(); + } + public long getRowid() { return m_rowid; @@ -200,9 +240,9 @@ public class GameLock { // used only for asserts public boolean canWrite() { - boolean result = m_isForWrite && 1 == m_lockCount; + boolean result = !mReadOnly; // && 1 == mLockCount[0]; if ( !result ) { - Log.w( TAG, "canWrite(): %H, returning false", this ); + Log.w( TAG, "%s.canWrite(): => false", this ); } return result; } diff --git a/xwords4/android/app/src/main/java/org/eehouse/android/xw4/GameUtils.java b/xwords4/android/app/src/main/java/org/eehouse/android/xw4/GameUtils.java index 2df7fd7f9..1a0fad4ca 100644 --- a/xwords4/android/app/src/main/java/org/eehouse/android/xw4/GameUtils.java +++ b/xwords4/android/app/src/main/java/org/eehouse/android/xw4/GameUtils.java @@ -92,9 +92,12 @@ public class GameUtils { public static byte[] savedGame( Context context, long rowid ) { - GameLock lock = new GameLock( rowid, false ).lock(); - byte[] result = savedGame( context, lock ); - lock.unlock(); + byte[] result = null; + try (GameLock lock = GameLock.getFor( rowid ).tryLockRO() ) { + if ( null != lock ) { + result = savedGame( context, lock ); + } + } if ( null == result ) { throw new NoSuchGameException(); @@ -144,7 +147,7 @@ public class GameUtils { if ( null == lockDest ) { long groupID = DBUtils.getGroupForGame( context, lockSrc.getRowid() ); long rowid = saveNewGame( context, gamePtr, gi, groupID ); - lockDest = new GameLock( rowid, true ).lock(); + lockDest = GameLock.getFor( rowid ).tryLock(); } else { saveGame( context, gamePtr, gi, lockDest, true ); } @@ -157,16 +160,16 @@ public class GameUtils { public static boolean resetGame( Context context, long rowidIn ) { boolean success = false; - GameLock lock = new GameLock( rowidIn, true ).lock( 500 ); - if ( null != lock ) { - tellDied( context, lock, true ); - resetGame( context, lock, lock, false ); - lock.unlock(); + try ( GameLock lock = GameLock.getFor( rowidIn ).lock( 500 ) ) { + if ( null != lock ) { + tellDied( context, lock, true ); + resetGame( context, lock, lock, false ); - Utils.cancelNotification( context, (int)rowidIn ); - success = true; - } else { - Log.w( TAG, "resetGame: unable to open rowid %d", rowidIn ); + Utils.cancelNotification( context, (int)rowidIn ); + success = true; + } else { + Log.w( TAG, "resetGame: unable to open rowid %d", rowidIn ); + } } return success; } @@ -221,7 +224,7 @@ public class GameUtils { lock = thread.getLock(); } else { try { - lock = new GameLock( rowid, false ).lock( maxMillis ); + lock = GameLock.getFor( rowid ).lockRO( maxMillis ); } catch ( GameLock.GameLockedException gle ) { Log.ex( TAG, gle ); } @@ -248,11 +251,11 @@ public class GameUtils { long rowid = DBUtils.ROWID_NOTFOUND; GameLock lockSrc = null; - JNIThread thread = JNIThread.getRetained( rowidIn, false ); + JNIThread thread = JNIThread.getRetained( rowidIn ); if ( null != thread ) { lockSrc = thread.getLock(); } else { - lockSrc = new GameLock( rowidIn, false ).lock( 300 ); + lockSrc = GameLock.getFor( rowidIn ).lockRO( 300 ); } if ( null != lockSrc ) { @@ -288,14 +291,14 @@ public class GameUtils { { boolean success; // does this need to be synchronized? - GameLock lock = new GameLock( rowid, true ); - if ( lock.tryLock() ) { - deleteGame( context, lock, informNow ); - lock.unlock(); - success = true; - } else { - Log.w( TAG, "deleteGame: unable to delete rowid %d", rowid ); - success = false; + try ( GameLock lock = GameLock.getFor( rowid ).tryLock() ) { + if ( null != lock ) { + deleteGame( context, lock, informNow ); + success = true; + } else { + Log.w( TAG, "deleteGame: unable to delete rowid %d", rowid ); + success = false; + } } return success; } @@ -392,16 +395,16 @@ public class GameUtils { public static Bitmap loadMakeBitmap( Context context, long rowid ) { Bitmap thumb = null; - GameLock lock = new GameLock( rowid, false ); - if ( lock.tryLock() ) { - CurGameInfo gi = new CurGameInfo( context ); - GamePtr gamePtr = loadMakeGame( context, gi, lock ); - if ( null != gamePtr ) { - thumb = takeSnapshot( context, gamePtr, gi ); - gamePtr.release(); - DBUtils.saveThumbnail( context, lock, thumb ); + try ( GameLock lock = GameLock.getFor( rowid ).tryLockRO() ) { + if ( null != lock ) { + CurGameInfo gi = new CurGameInfo( context ); + GamePtr gamePtr = loadMakeGame( context, gi, lock ); + if ( null != gamePtr ) { + thumb = takeSnapshot( context, gamePtr, gi ); + gamePtr.release(); + DBUtils.saveThumbnail( context, lock, thumb ); + } } - lock.unlock(); } return thumb; } @@ -507,9 +510,10 @@ public class GameUtils { CurGameInfo gi, long groupID ) { byte[] stream = XwJNI.game_saveToStream( gamePtr, gi ); - GameLock lock = DBUtils.saveNewGame( context, stream, groupID, null ); - long rowid = lock.getRowid(); - lock.unlock(); + long rowid; + try ( GameLock lock = DBUtils.saveNewGame( context, stream, groupID, null ) ) { + rowid = lock.getRowid(); + } return rowid; } @@ -540,10 +544,10 @@ public class GameUtils { long rowid = DBUtils.ROWID_NOTFOUND; byte[] bytes = XwJNI.gi_to_stream( gi ); if ( null != bytes ) { - GameLock lock = DBUtils.saveNewGame( context, bytes, groupID, - gameName ); - rowid = lock.getRowid(); - lock.unlock(); + try ( GameLock lock = DBUtils.saveNewGame( context, bytes, groupID, + gameName ) ) { + rowid = lock.getRowid(); + } } return rowid; } @@ -641,9 +645,12 @@ public class GameUtils { } if ( DBUtils.ROWID_NOTFOUND != rowid ) { - GameLock lock = new GameLock( rowid, true ).lock(); - applyChanges( context, sink, gi, util, addr, null, lock, false ); - lock.unlock(); + // Use tryLock in case we're on UI thread. It's guaranteed to + // succeed because we just created the rowid. + try ( GameLock lock = GameLock.getFor( rowid ).tryLock() ) { + Assert.assertNotNull( lock ); + applyChanges( context, sink, gi, util, addr, null, lock, false ); + } } return rowid; @@ -868,7 +875,7 @@ public class GameUtils { return file.endsWith( XWConstants.GAME_EXTN ); } - public static Bundle makeLaunchExtras( long rowid, boolean invited ) + private static Bundle makeLaunchExtras( long rowid, boolean invited ) { Bundle bundle = new Bundle(); bundle.putLong( INTENT_KEY_ROWID, rowid ); @@ -952,54 +959,54 @@ public class GameUtils { // have the lock and we'll never get it. Better to drop // the message than fire the hung-lock assert. Messages // belong in local pre-delivery storage anyway. - GameLock lock = new GameLock( rowid, true ).lock( 150 ); - if ( null != lock ) { - CurGameInfo gi = new CurGameInfo( context ); - FeedUtilsImpl feedImpl = new FeedUtilsImpl( context, rowid ); - GamePtr gamePtr = loadMakeGame( context, gi, feedImpl, sink, lock ); - if ( null != gamePtr ) { - XwJNI.comms_resendAll( gamePtr, false, false ); + try ( GameLock lock = GameLock.getFor( rowid ).lock( 150 ) ) { + if ( null != lock ) { + CurGameInfo gi = new CurGameInfo( context ); + FeedUtilsImpl feedImpl = new FeedUtilsImpl( context, rowid ); + GamePtr gamePtr = loadMakeGame( context, gi, feedImpl, sink, lock ); + if ( null != gamePtr ) { + XwJNI.comms_resendAll( gamePtr, false, false ); - Assert.assertNotNull( ret ); - draw = XwJNI.game_receiveMessage( gamePtr, msg, ret ); - XwJNI.comms_ackAny( gamePtr ); + Assert.assertNotNull( ret ); + draw = XwJNI.game_receiveMessage( gamePtr, msg, ret ); + XwJNI.comms_ackAny( gamePtr ); - // update gi to reflect changes due to messages - XwJNI.game_getGi( gamePtr, gi ); + // update gi to reflect changes due to messages + XwJNI.game_getGi( gamePtr, gi ); - if ( draw && XWPrefs.getThumbEnabled( context ) ) { - Bitmap bitmap = takeSnapshot( context, gamePtr, gi ); - DBUtils.saveThumbnail( context, lock, bitmap ); - } + if ( draw && XWPrefs.getThumbEnabled( context ) ) { + Bitmap bitmap = takeSnapshot( context, gamePtr, gi ); + DBUtils.saveThumbnail( context, lock, bitmap ); + } - if ( null != bmr ) { - if ( null != feedImpl.m_chat ) { - bmr.m_chat = feedImpl.m_chat; - bmr.m_chatFrom = feedImpl.m_chatFrom; - bmr.m_chatTs = feedImpl.m_ts; - } else { - LastMoveInfo lmi = new LastMoveInfo(); - XwJNI.model_getPlayersLastScore( gamePtr, -1, lmi ); - bmr.m_lmi = lmi; + if ( null != bmr ) { + if ( null != feedImpl.m_chat ) { + bmr.m_chat = feedImpl.m_chat; + bmr.m_chatFrom = feedImpl.m_chatFrom; + bmr.m_chatTs = feedImpl.m_ts; + } else { + LastMoveInfo lmi = new LastMoveInfo(); + XwJNI.model_getPlayersLastScore( gamePtr, -1, lmi ); + bmr.m_lmi = lmi; + } + } + + saveGame( context, gamePtr, gi, lock, false ); + GameSummary summary = summarizeAndRelease( context, lock, + gamePtr, gi ); + if ( null != isLocalOut ) { + isLocalOut[0] = 0 <= summary.turn + && gi.players[summary.turn].isLocal; + } + + int flags = setFromFeedImpl( feedImpl ); + if ( GameSummary.MSG_FLAGS_NONE != flags ) { + draw = true; + int curFlags = DBUtils.getMsgFlags( context, rowid ); + DBUtils.setMsgFlags( rowid, flags | curFlags ); } } - - saveGame( context, gamePtr, gi, lock, false ); - GameSummary summary = summarizeAndRelease( context, lock, - gamePtr, gi ); - if ( null != isLocalOut ) { - isLocalOut[0] = 0 <= summary.turn - && gi.players[summary.turn].isLocal; - } - - int flags = setFromFeedImpl( feedImpl ); - if ( GameSummary.MSG_FLAGS_NONE != flags ) { - draw = true; - int curFlags = DBUtils.getMsgFlags( context, rowid ); - DBUtils.setMsgFlags( rowid, flags | curFlags ); - } } - lock.unlock(); } } return draw; @@ -1011,35 +1018,36 @@ public class GameUtils { public static boolean replaceDicts( Context context, long rowid, String oldDict, String newDict ) { - GameLock lock = new GameLock( rowid, true ).lock(300); - boolean success = null != lock; - if ( success ) { - byte[] stream = savedGame( context, lock ); - CurGameInfo gi = new CurGameInfo( context ); - XwJNI.gi_from_stream( gi, stream ); + boolean success; + try ( GameLock lock = GameLock.getFor( rowid ).lock(300) ) { + success = null != lock; + if ( success ) { + byte[] stream = savedGame( context, lock ); + CurGameInfo gi = new CurGameInfo( context ); + XwJNI.gi_from_stream( gi, stream ); - // first time required so dictNames() will work - gi.replaceDicts( context, newDict ); + // first time required so dictNames() will work + gi.replaceDicts( context, newDict ); - String[] dictNames = gi.dictNames(); - DictUtils.DictPairs pairs = DictUtils.openDicts( context, - dictNames ); + String[] dictNames = gi.dictNames(); + DictUtils.DictPairs pairs = DictUtils.openDicts( context, + dictNames ); - GamePtr gamePtr = - XwJNI.initFromStream( rowid, stream, gi, dictNames, - pairs.m_bytes, pairs.m_paths, - gi.langName( context ), null, - null, CommonPrefs.get( context ), null ); - // second time required as game_makeFromStream can overwrite - gi.replaceDicts( context, newDict ); + GamePtr gamePtr = + XwJNI.initFromStream( rowid, stream, gi, dictNames, + pairs.m_bytes, pairs.m_paths, + gi.langName( context ), null, + null, CommonPrefs.get( context ), null ); + // second time required as game_makeFromStream can overwrite + gi.replaceDicts( context, newDict ); - saveGame( context, gamePtr, gi, lock, false ); + saveGame( context, gamePtr, gi, lock, false ); - summarizeAndRelease( context, lock, gamePtr, gi ); + summarizeAndRelease( context, lock, gamePtr, gi ); - lock.unlock(); - } else { - Log.w( TAG, "replaceDicts: unable to open rowid %d", rowid ); + } else { + Log.w( TAG, "replaceDicts: unable to open rowid %d", rowid ); + } } return success; } // replaceDicts @@ -1295,32 +1303,32 @@ public class GameUtils { } } - GameLock lock = new GameLock( rowid, false ); - if ( lock.tryLock() ) { - CurGameInfo gi = new CurGameInfo( m_context ); - MultiMsgSink sink = new MultiMsgSink( m_context, rowid ); - GamePtr gamePtr = loadMakeGame( m_context, gi, sink, lock ); - if ( null != gamePtr ) { - int nSent = XwJNI.comms_resendAll( gamePtr, true, - m_filter, false ); - gamePtr.release(); - Log.d( TAG, "Resender.doInBackground(): sent %d " - + "messages for rowid %d", nSent, rowid ); - nSentTotal += sink.numSent(); + try ( GameLock lock = GameLock.getFor( rowid ).tryLockRO() ) { + if ( null != lock ) { + CurGameInfo gi = new CurGameInfo( m_context ); + MultiMsgSink sink = new MultiMsgSink( m_context, rowid ); + GamePtr gamePtr = loadMakeGame( m_context, gi, sink, lock ); + if ( null != gamePtr ) { + int nSent = XwJNI.comms_resendAll( gamePtr, true, + m_filter, false ); + gamePtr.release(); + Log.d( TAG, "Resender.doInBackground(): sent %d " + + "messages for rowid %d", nSent, rowid ); + nSentTotal += sink.numSent(); + } else { + Log.d( TAG, "Resender.doInBackground(): loadMakeGame()" + + " failed for rowid %d", rowid ); + } } else { - Log.d( TAG, "Resender.doInBackground(): loadMakeGame()" - + " failed for rowid %d", rowid ); - } - lock.unlock(); - } else { - JNIThread jniThread = JNIThread.getRetained( rowid, false ); - if ( null != jniThread ) { - jniThread.handle( JNIThread.JNICmd.CMD_RESEND, false, - false, false ); - jniThread.release(); - } else { - Log.w( TAG, "Resender.doInBackground: unable to unlock %d", - rowid ); + JNIThread jniThread = JNIThread.getRetained( rowid ); + if ( null != jniThread ) { + jniThread.handle( JNIThread.JNICmd.CMD_RESEND, false, + false, false ); + jniThread.release(); + } else { + Log.w( TAG, "Resender.doInBackground: unable to unlock %d", + rowid ); + } } } } diff --git a/xwords4/android/app/src/main/java/org/eehouse/android/xw4/GamesListDelegate.java b/xwords4/android/app/src/main/java/org/eehouse/android/xw4/GamesListDelegate.java index 5a874cd96..e221fade9 100644 --- a/xwords4/android/app/src/main/java/org/eehouse/android/xw4/GamesListDelegate.java +++ b/xwords4/android/app/src/main/java/org/eehouse/android/xw4/GamesListDelegate.java @@ -1829,11 +1829,11 @@ public class GamesListDelegate extends ListDelegateBase GameUtils.savedGame( self, selRowIDs[0] ); long groupID = XWPrefs .getDefaultNewGameGroup( self ); - GameLock lock = - GameUtils.saveNewGame( self, stream, groupID ); - DBUtils.saveSummary( self, lock, smry ); - m_mySIS.selGames.add( lock.getRowid() ); - lock.unlock(); + try ( GameLock lock = + GameUtils.saveNewGame( self, stream, groupID ) ) { + DBUtils.saveSummary( self, lock, smry ); + m_mySIS.selGames.add( lock.getRowid() ); + } mkListAdapter(); } }); @@ -2033,9 +2033,7 @@ public class GamesListDelegate extends ListDelegateBase try { hasDicts = GameUtils.gameDictsHere( m_activity, rowid, missingNames, missingLang ); - } catch ( GameUtils.NoSuchGameException nsge ) { - hasDicts = true; // irrelevant question - } catch ( GameLock.GameLockedException gle ) { + } catch ( GameLock.GameLockedException | GameUtils.NoSuchGameException ex ) { hasDicts = true; // irrelevant question } diff --git a/xwords4/android/app/src/main/java/org/eehouse/android/xw4/XWServiceHelper.java b/xwords4/android/app/src/main/java/org/eehouse/android/xw4/XWServiceHelper.java index e23a85349..3f076f1f8 100644 --- a/xwords4/android/app/src/main/java/org/eehouse/android/xw4/XWServiceHelper.java +++ b/xwords4/android/app/src/main/java/org/eehouse/android/xw4/XWServiceHelper.java @@ -75,7 +75,7 @@ abstract class XWServiceHelper { { boolean allConsumed = true; boolean[] isLocalP = new boolean[1]; - JNIThread jniThread = JNIThread.getRetained( rowid, false ); + JNIThread jniThread = JNIThread.getRetained( rowid ); boolean consumed = false; if ( null != jniThread ) { consumed = true; diff --git a/xwords4/android/app/src/main/java/org/eehouse/android/xw4/jni/JNIThread.java b/xwords4/android/app/src/main/java/org/eehouse/android/xw4/jni/JNIThread.java index 3c90325e1..0dd797b00 100644 --- a/xwords4/android/app/src/main/java/org/eehouse/android/xw4/jni/JNIThread.java +++ b/xwords4/android/app/src/main/java/org/eehouse/android/xw4/jni/JNIThread.java @@ -35,6 +35,7 @@ import org.eehouse.android.xw4.DbgUtils; import org.eehouse.android.xw4.DictUtils; import org.eehouse.android.xw4.GameLock; import org.eehouse.android.xw4.GameUtils; +import org.eehouse.android.xw4.Utils; import org.eehouse.android.xw4.Log; import org.eehouse.android.xw4.R; import org.eehouse.android.xw4.XWPrefs; @@ -832,8 +833,14 @@ public class JNIThread extends Thread { synchronized( s_instances ) { result = s_instances.get( rowid ); if ( null == result && makeNew ) { - result = new JNIThread( new GameLock( rowid, true ).lock() ); - s_instances.put( rowid, result ); + DbgUtils.assertOnUIThread(); // can't use GameLock.lock() + if ( true /*test done*/ || (0 != Utils.nextRandomInt() % 3) ) { + GameLock lock = GameLock.getFor( rowid ).tryLock(); + if ( lock != null ) { + result = new JNIThread( lock ); + s_instances.put( rowid, result ); + } + } } if ( null != result ) { result.retain_sync();