rewrite GameLock to use wait()/notifyAll()

I've learned a bit about java since writing that class. Should be faster
now without the sleep/polling. Also making it illegal to use blocking
lock() call on UI thread. There may be some assertions to fix in the
next few days.

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- \
+            NFCUtils.register( m_activity, this ); // Don't seem to need to unregister...
-        // 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...
+            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 );
+            try ( GameLock lock = GameLock.getFor( rowID ).tryLockRO() ) {
-            if ( lock.tryLock() ) {
+                if ( null != lock ) {
-                summary = DBUtils.getSummary( activity, lock );
+                    summary = DBUtils.getSummary( activity, lock );
-                gi = new CurGameInfo( activity );
+                    gi = new CurGameInfo( activity );
-                gamePtr = GameUtils.loadMakeGame( activity, gi, lock );
+                    gamePtr = GameUtils.loadMakeGame( activity, gi, lock );
-                lock.unlock();
+                }
             }
         }
 

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();
+        try ( GameLock lock = GameLock.getFor( rowid ).tryLock() ) {
-        GameSummary summary = getSummary( context, lock );
+            Assert.assertNotNull( lock );
-        if ( null != btAddr ) {
+            if ( null != lock ) {
-            summary.putStringExtra( GameSummary.EXTRA_REMATCH_BTADDR, btAddr );
+                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 );
+        try ( GameLock lock = GameLock.getFor( rowid ).lock( 300 ) ) {
-        if ( null != lock ) {
+            if ( null != lock ) {
-            deleteGame( context, lock );
+                deleteGame( context, lock );
-            lock.unlock();
+            } else {
-        } else {
+                Log.e( TAG, "deleteGame: unable to lock rowid %d", rowid );
-            Log.e( TAG, "deleteGame: unable to lock rowid %d", rowid );
+                if ( BuildConfig.DEBUG ) {
-            if ( BuildConfig.DEBUG ) {
+                    Assert.fail();
-                Assert.fail();
+                }
             }
         }
     }

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
+            try {
-                ? new GameLock( m_rowid, true ).lock()
+                GameLock gameLock = m_jniThread == null
-                : m_jniThread.getLock();
+                    ? GameLock.getFor( m_rowid ).lock()
-            GameUtils.applyChanges( m_activity, m_gi, m_car, m_disabMap,
+                    : m_jniThread.getLock();
-                                    gameLock, forceNew );
+                GameUtils.applyChanges( m_activity, m_gi, m_car, m_disabMap,
-            DBUtils.saveThumbnail( m_activity, gameLock, null ); // clear it
+                                        gameLock, forceNew );
-            if ( null == m_jniThread ) {
+                DBUtils.saveThumbnail( m_activity, gameLock, null ); // clear it
-                gameLock.unlock();
+                if ( null == m_jniThread ) {
+                    gameLock.unlock();
+                }
+            } catch (InterruptedException iex) {
+                Assert.assertFalse( BuildConfig.DEBUG );
             }
         }
     }

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 long ASSERT_TIME = 2000;
-    private static final int SLEEP_TIME = 100;
-    private static final long ASSERT_TIME = 2000;
     private static final long THROW_TIME = 1000;
     private long m_rowid;
-    private boolean m_isForWrite;
+    private Stack<Owner> mOwners = new Stack<>();
-    private int m_lockCount;
+    private boolean mReadOnly;
-    private Thread m_ownerThread;
-    private StackTraceElement[] m_lockTrace;
 
-    static {
+    private static class Owner {
-        Assert.assertTrue( THROW_TIME <= ASSERT_TIME );
+        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<Long, GameLock>
+    private static Map<Long, WeakReference<GameLock>> sLockMap = new HashMap<>();
-        s_locks = new HashMap<Long,GameLock>();
+    public static GameLock getFor( long rowid )
-
-    public GameLock( long rowid, boolean isForWrite )
     {
-        Assert.assertTrue( DBUtils.ROWID_NOTFOUND != rowid );
+        GameLock result = null;
-        m_rowid = rowid;
+        synchronized ( sLockMap ) {
-        m_isForWrite = isForWrite;
+            if ( sLockMap.containsKey( rowid ) ) {
-        m_lockCount = 0;
+                result = sLockMap.get( rowid ).get();
-        if ( DEBUG_LOCKS ) {
+            }
-            Log.i( TAG, "GameLock(rowid:%d,isForWrite:%b)=>this: %H",
+            if ( null == result ) {
-                   rowid, isForWrite, this );
+                result = new GameLock( rowid );
-            DbgUtils.printStack( TAG );
+                sLockMap.put( rowid, new WeakReference(result) );
-        }
-    }
-
-    // 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 );
             }
         }
-        if ( DEBUG_LOCKS ) {
+        return result;
-            Log.i( TAG, "tryLock %H (rowid=%d) => %b", this, m_rowid, gotIt );
-        }
-        return owner;
     }
 
-    // Wait forever (but may assert if too long)
+    private GameLock( long rowid ) { m_rowid = rowid; }
-    public GameLock lock()
+
+    // 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,
+            Log.d( TAG, "%s.lock(%d) => %s", this, maxMillis, result );
-                   maxMillis );
         }
-
-        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 ( mOwners ) {
-        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;
-
             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;
     }

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 = null;
-        byte[] result = savedGame( context, lock );
+        try (GameLock lock = GameLock.getFor( rowid ).tryLockRO() ) {
-        lock.unlock();
+            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 );
+        try ( GameLock lock = GameLock.getFor( rowidIn ).lock( 500 ) ) {
-        if ( null != lock ) {
+            if ( null != lock ) {
-            tellDied( context, lock, true );
+                tellDied( context, lock, true );
-            resetGame( context, lock, lock, false );
+                resetGame( context, lock, lock, false );
-            lock.unlock();
 
-            Utils.cancelNotification( context, (int)rowidIn );
+                Utils.cancelNotification( context, (int)rowidIn );
-            success = true;
+                success = true;
-        } else {
+            } else {
-            Log.w( TAG, "resetGame: unable to open rowid %d", rowidIn );
+                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 );
+        try ( GameLock lock = GameLock.getFor( rowid ).tryLock() ) {
-        if ( lock.tryLock() ) {
+            if ( null != lock ) {
-            deleteGame( context, lock, informNow );
+                deleteGame( context, lock, informNow );
-            lock.unlock();
+                success = true;
-            success = true;
+            } else {
-        } else {
+                Log.w( TAG, "deleteGame: unable to delete rowid %d", rowid );
-            Log.w( TAG, "deleteGame: unable to delete rowid %d", rowid );
+                success = false;
-            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 );
+        try ( GameLock lock = GameLock.getFor( rowid ).tryLockRO() ) {
-        if ( lock.tryLock() ) {
+            if ( null != lock ) {
-            CurGameInfo gi = new CurGameInfo( context );
+                CurGameInfo gi = new CurGameInfo( context );
-            GamePtr gamePtr = loadMakeGame( context, gi, lock );
+                GamePtr gamePtr = loadMakeGame( context, gi, lock );
-            if ( null != gamePtr ) {
+                if ( null != gamePtr ) {
-                thumb = takeSnapshot( context, gamePtr, gi );
+                    thumb = takeSnapshot( context, gamePtr, gi );
-                gamePtr.release();
+                    gamePtr.release();
-                DBUtils.saveThumbnail( context, lock, thumb );
+                    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;
-        long rowid = lock.getRowid();
+        try ( GameLock lock = DBUtils.saveNewGame( context, stream, groupID, null ) ) {
-        lock.unlock();
+            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,
+            try ( GameLock lock = DBUtils.saveNewGame( context, bytes, groupID,
-                                                 gameName );
+                                                       gameName ) ) {
-            rowid = lock.getRowid();
+                rowid = lock.getRowid();
-            lock.unlock();
+            }
         }
         return rowid;
     }
@@ -641,9 +645,12 @@ public class GameUtils {
         }
 
         if ( DBUtils.ROWID_NOTFOUND != rowid ) {
-            GameLock lock = new GameLock( rowid, true ).lock();
+            // Use tryLock in case we're on UI thread. It's guaranteed to
-            applyChanges( context, sink, gi, util, addr, null, lock, false );
+            // succeed because we just created the rowid.
-            lock.unlock();
+            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 );
+            try ( GameLock lock = GameLock.getFor( rowid ).lock( 150 ) ) {
-            if ( null != lock ) {
+                if ( null != lock ) {
-                CurGameInfo gi = new CurGameInfo( context );
+                    CurGameInfo gi = new CurGameInfo( context );
-                FeedUtilsImpl feedImpl = new FeedUtilsImpl( context, rowid );
+                    FeedUtilsImpl feedImpl = new FeedUtilsImpl( context, rowid );
-                GamePtr gamePtr = loadMakeGame( context, gi, feedImpl, sink, lock );
+                    GamePtr gamePtr = loadMakeGame( context, gi, feedImpl, sink, lock );
-                if ( null != gamePtr ) {
+                    if ( null != gamePtr ) {
-                    XwJNI.comms_resendAll( gamePtr, false, false );
+                        XwJNI.comms_resendAll( gamePtr, false, false );
 
-                    Assert.assertNotNull( ret );
+                        Assert.assertNotNull( ret );
-                    draw = XwJNI.game_receiveMessage( gamePtr, msg, ret );
+                        draw = XwJNI.game_receiveMessage( gamePtr, msg, ret );
-                    XwJNI.comms_ackAny( gamePtr );
+                        XwJNI.comms_ackAny( gamePtr );
 
-                    // update gi to reflect changes due to messages
+                        // update gi to reflect changes due to messages
-                    XwJNI.game_getGi( gamePtr, gi );
+                        XwJNI.game_getGi( gamePtr, gi );
 
-                    if ( draw && XWPrefs.getThumbEnabled( context ) ) {
+                        if ( draw && XWPrefs.getThumbEnabled( context ) ) {
-                        Bitmap bitmap = takeSnapshot( context, gamePtr, gi );
+                            Bitmap bitmap = takeSnapshot( context, gamePtr, gi );
-                        DBUtils.saveThumbnail( context, lock, bitmap );
+                            DBUtils.saveThumbnail( context, lock, bitmap );
-                    }
+                        }
 
-                    if ( null != bmr ) {
+                        if ( null != bmr ) {
-                        if ( null != feedImpl.m_chat ) {
+                            if ( null != feedImpl.m_chat ) {
-                            bmr.m_chat = feedImpl.m_chat;
+                                bmr.m_chat = feedImpl.m_chat;
-                            bmr.m_chatFrom = feedImpl.m_chatFrom;
+                                bmr.m_chatFrom = feedImpl.m_chatFrom;
-                            bmr.m_chatTs = feedImpl.m_ts;
+                                bmr.m_chatTs = feedImpl.m_ts;
-                        } else {
+                            } else {
-                            LastMoveInfo lmi = new LastMoveInfo();
+                                LastMoveInfo lmi = new LastMoveInfo();
-                            XwJNI.model_getPlayersLastScore( gamePtr, -1, lmi );
+                                XwJNI.model_getPlayersLastScore( gamePtr, -1, lmi );
-                            bmr.m_lmi = 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;
-        boolean success = null != lock;
+        try ( GameLock lock = GameLock.getFor( rowid ).lock(300) ) {
-        if ( success ) {
+            success = null != lock;
-            byte[] stream = savedGame( context, lock );
+            if ( success ) {
-            CurGameInfo gi = new CurGameInfo( context );
+                byte[] stream = savedGame( context, lock );
-            XwJNI.gi_from_stream( gi, stream );
+                CurGameInfo gi = new CurGameInfo( context );
+                XwJNI.gi_from_stream( gi, stream );
 
-            // first time required so dictNames() will work
+                // first time required so dictNames() will work
-            gi.replaceDicts( context, newDict );
+                gi.replaceDicts( context, newDict );
 
-            String[] dictNames = gi.dictNames();
+                String[] dictNames = gi.dictNames();
-            DictUtils.DictPairs pairs = DictUtils.openDicts( context,
+                DictUtils.DictPairs pairs = DictUtils.openDicts( context,
-                                                             dictNames );
+                                                                 dictNames );
 
-            GamePtr gamePtr =
+                GamePtr gamePtr =
-                XwJNI.initFromStream( rowid, stream, gi, dictNames,
+                    XwJNI.initFromStream( rowid, stream, gi, dictNames,
-                                      pairs.m_bytes, pairs.m_paths,
+                                          pairs.m_bytes, pairs.m_paths,
-                                      gi.langName( context ), null,
+                                          gi.langName( context ), null,
-                                      null, CommonPrefs.get( context ), null );
+                                          null, CommonPrefs.get( context ), null );
-            // second time required as game_makeFromStream can overwrite
+                // second time required as game_makeFromStream can overwrite
-            gi.replaceDicts( context, newDict );
+                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 {
-        } else {
+                Log.w( TAG, "replaceDicts: unable to open rowid %d", rowid );
-            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 );
+                try ( GameLock lock = GameLock.getFor( rowid ).tryLockRO() ) {
-                if ( lock.tryLock() ) {
+                    if ( null != lock ) {
-                    CurGameInfo gi = new CurGameInfo( m_context );
+                        CurGameInfo gi = new CurGameInfo( m_context );
-                    MultiMsgSink sink = new MultiMsgSink( m_context, rowid );
+                        MultiMsgSink sink = new MultiMsgSink( m_context, rowid );
-                    GamePtr gamePtr = loadMakeGame( m_context, gi, sink, lock );
+                        GamePtr gamePtr = loadMakeGame( m_context, gi, sink, lock );
-                    if ( null != gamePtr ) {
+                        if ( null != gamePtr ) {
-                        int nSent = XwJNI.comms_resendAll( gamePtr, true,
+                            int nSent = XwJNI.comms_resendAll( gamePtr, true,
-                                                           m_filter, false );
+                                                               m_filter, false );
-                        gamePtr.release();
+                            gamePtr.release();
-                        Log.d( TAG, "Resender.doInBackground(): sent %d "
+                            Log.d( TAG, "Resender.doInBackground(): sent %d "
-                               + "messages for rowid %d", nSent, rowid );
+                                   + "messages for rowid %d", nSent, rowid );
-                        nSentTotal += sink.numSent();
+                            nSentTotal += sink.numSent();
+                        } else {
+                            Log.d( TAG, "Resender.doInBackground(): loadMakeGame()"
+                                   + " failed for rowid %d", rowid );
+                        }
                     } else {
-                        Log.d( TAG, "Resender.doInBackground(): loadMakeGame()"
+                        JNIThread jniThread = JNIThread.getRetained( rowid );
-                               + " failed for rowid %d", rowid );
+                        if ( null != jniThread ) {
-                    }
+                            jniThread.handle( JNIThread.JNICmd.CMD_RESEND, false,
-                    lock.unlock();
+                                              false, false );
-                } else {
+                            jniThread.release();
-                    JNIThread jniThread = JNIThread.getRetained( rowid, false );
+                        } else {
-                    if ( null != jniThread ) {
+                            Log.w( TAG, "Resender.doInBackground: unable to unlock %d",
-                        jniThread.handle( JNIThread.JNICmd.CMD_RESEND, false,
+                                   rowid );
-                                          false, false );
+                        }
-                        jniThread.release();
-                    } else {
-                        Log.w( TAG, "Resender.doInBackground: unable to unlock %d",
-                               rowid );
                     }
                 }
             }

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 =
+                            try ( GameLock lock =
-                                GameUtils.saveNewGame( self, stream, groupID );
+                                  GameUtils.saveNewGame( self, stream, groupID ) ) {
-                            DBUtils.saveSummary( self, lock, smry );
+                                DBUtils.saveSummary( self, lock, smry );
-                            m_mySIS.selGames.add( lock.getRowid() );
+                                m_mySIS.selGames.add( lock.getRowid() );
-                            lock.unlock();
+                            }
                             mkListAdapter();
                         }
                     });
@@ -2033,9 +2033,7 @@ public class GamesListDelegate extends ListDelegateBase
         try {
             hasDicts = GameUtils.gameDictsHere( m_activity, rowid, missingNames,
                                                 missingLang );
-        } catch ( GameUtils.NoSuchGameException nsge ) {
+        } catch ( GameLock.GameLockedException | GameUtils.NoSuchGameException ex ) {
-            hasDicts = true;    // irrelevant question
-        } catch ( GameLock.GameLockedException gle ) {
             hasDicts = true;    // irrelevant question
         }
 

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;

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() );
+                DbgUtils.assertOnUIThread(); // can't use GameLock.lock()
-                s_instances.put( rowid, result );
+                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();