From d94217cc064ce84f07866d72efba3e8c2ff0cd49 Mon Sep 17 00:00:00 2001 From: Eric House Date: Wed, 16 Jan 2019 21:45:39 -0800 Subject: [PATCH] Make Board wait a bit for lock on opening Was seeing frequent failure to open games as JNIThread.getRetained() was unable to get a lock without waiting. Which it can't do on UI thread. So added a method to GameLock that takes a callback to be called when the lock's obtained, with the actual waiting done on a local thread. Then fixed BoardDelegate a bit to not crash while waiting for the callback. --- .../eehouse/android/xw4/BoardDelegate.java | 45 ++++++++++------ .../org/eehouse/android/xw4/ChatDelegate.java | 2 +- .../android/xw4/GameConfigDelegate.java | 2 +- .../org/eehouse/android/xw4/GameLock.java | 54 +++++++++++++++++++ .../org/eehouse/android/xw4/GameUtils.java | 6 +-- .../eehouse/android/xw4/XWServiceHelper.java | 4 +- .../eehouse/android/xw4/jni/JNIThread.java | 25 ++++----- 7 files changed, 101 insertions(+), 37 deletions(-) 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 ae4dc6135..b932c91fa 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 @@ -566,35 +566,48 @@ 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_activity, 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(); + GameLock.callWithLock( m_rowid, 100L, new Handler(), + new GameLock.LockProc() { + @Override + public void gotLock( GameLock lock ) { + if ( null == lock ) { + finish(); + } else { + m_jniThreadRef = JNIThread.getRetained( lock ); - NFCUtils.register( m_activity, this ); // Don't seem to need to unregister... + // see http://stackoverflow.com/questions/680180/where-to-stop- \ + // destroy-threads-in-android-service-class + m_jniThreadRef.setDaemonOnce( true ); + m_jniThreadRef.startOnce(); - setBackgroundColor(); - setKeepScreenOn(); - } + // Don't seem to need to unregister... + NFCUtils.register( m_activity, BoardDelegate.this ); + + setBackgroundColor(); + setKeepScreenOn(); + + doResume( true ); + } + } + } ); } // init @Override protected void onStart() { super.onStart(); - doResume( true ); + if ( null != m_jniThreadRef ) { + doResume( true ); + } } @Override protected void onResume() { super.onResume(); - doResume( false ); + if ( null != m_jniThreadRef ) { + doResume( false ); + } } protected void onPause() @@ -2747,7 +2760,7 @@ public class BoardDelegate extends DelegateBase GamePtr gamePtr = null; GameSummary summary = null; CurGameInfo gi = null; - JNIThread thread = JNIThread.getRetained( activity, rowID ); + JNIThread thread = JNIThread.getRetained( rowID ); if ( null != thread ) { gamePtr = thread.getGamePtr().retain(); summary = thread.getSummary(); diff --git a/xwords4/android/app/src/main/java/org/eehouse/android/xw4/ChatDelegate.java b/xwords4/android/app/src/main/java/org/eehouse/android/xw4/ChatDelegate.java index b72506648..f3239e590 100644 --- a/xwords4/android/app/src/main/java/org/eehouse/android/xw4/ChatDelegate.java +++ b/xwords4/android/app/src/main/java/org/eehouse/android/xw4/ChatDelegate.java @@ -130,7 +130,7 @@ public class ChatDelegate extends DelegateBase { protected void onResume() { super.onResume(); - m_jniThreadRef = JNIThread.getRetained( m_activity, m_rowid ); + m_jniThreadRef = JNIThread.getRetained( m_rowid ); if ( null == m_jniThreadRef ) { Log.w( TAG, "onResume(): m_jniThreadRef null; exiting" ); finish(); 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 847964d62..8cae19c7b 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 @@ -483,7 +483,7 @@ public class GameConfigDelegate extends DelegateBase @Override protected void onResume() { - m_jniThread = JNIThread.getRetained( m_activity, m_rowid ); + m_jniThread = JNIThread.getRetained( m_rowid ); super.onResume(); loadGame(); } 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 3f048bd4c..71f8502c3 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,6 +20,8 @@ package org.eehouse.android.xw4; +import android.os.Handler; + import java.io.Serializable; import java.lang.ref.WeakReference; import java.util.Formatter; @@ -232,6 +234,58 @@ public class GameLock implements AutoCloseable { return m_rowid; } + private void setOwner( Owner owner ) + { + synchronized ( mOwners ) { + mOwners.pop(); + mOwners.push( owner ); + } + } + + public interface LockProc { + public void gotLock( GameLock lock ); + } + + // Meant to be called from UI thread, returning immediately, but when it + // gets the lock, or time runs out, calls the callback (using the Handler + // passed in) with the lock or null. + public static void callWithLock( final long rowid, + final long maxMillis, + final Handler handler, + final LockProc proc ) + { + // capture caller thread and stack + final Owner owner = new Owner(); + + new Thread( new Runnable() { + @Override + public void run() { + GameLock lock = null; + if ( false && 0 == Utils.nextRandomInt() % 5 ) { + Log.d( TAG, "testing return-null case" ); + try { + Thread.sleep( maxMillis ); + } catch ( Exception ex) {} + } else { + try { + lock = GameLock + .getFor( rowid ) + .lockImpl( maxMillis, false ); + lock.setOwner( owner ); + } catch ( GameLockedException | InterruptedException gle ) {} + } + + final GameLock fLock = lock; + handler.post( new Runnable() { + @Override + public void run() { + proc.gotLock( fLock ); + } + } ); + } + } ).start(); + } + // used only for asserts public boolean canWrite() { 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 a656c8149..84dcbe6a4 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 @@ -219,7 +219,7 @@ public class GameUtils { long maxMillis ) { GameSummary result = null; - JNIThread thread = JNIThread.getRetained( context, rowid ); + JNIThread thread = JNIThread.getRetained( rowid ); GameLock lock = null; if ( null != thread ) { lock = thread.getLock(); @@ -252,7 +252,7 @@ public class GameUtils { long rowid = DBUtils.ROWID_NOTFOUND; GameLock lockSrc = null; - JNIThread thread = JNIThread.getRetained( context, rowidIn ); + JNIThread thread = JNIThread.getRetained( rowidIn ); if ( null != thread ) { lockSrc = thread.getLock(); } else { @@ -1275,7 +1275,7 @@ public class GameUtils { + " failed for rowid %d", rowid ); } } else { - JNIThread jniThread = JNIThread.getRetained( m_context, rowid ); + JNIThread jniThread = JNIThread.getRetained( rowid ); if ( null != jniThread ) { jniThread.handle( JNIThread.JNICmd.CMD_RESEND, false, false, false ); 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 5f6a3310e..1d8974f9f 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 @@ -76,7 +76,7 @@ abstract class XWServiceHelper { { boolean allConsumed = true; boolean[] isLocalP = new boolean[1]; - JNIThread jniThread = JNIThread.getRetained( context, rowid ); + JNIThread jniThread = JNIThread.getRetained( rowid ); boolean consumed = false; if ( null != jniThread ) { consumed = true; @@ -146,7 +146,7 @@ abstract class XWServiceHelper { if ( null == gi ) { // locked. Maybe it's open? - JNIThread thread = JNIThread.getRetained( mService, rowid ); + JNIThread thread = JNIThread.getRetained( rowid ); if ( null != thread ) { gi = thread.getGI(); thread.release( false ); 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 9b15d6ac7..66ce7e955 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 @@ -823,27 +823,24 @@ public class JNIThread extends Thread { } } - public static JNIThread getRetained( Context context, long rowid ) + public static JNIThread getRetained( long rowid ) { - return getRetained( context, rowid, false ); + return getRetained( rowid, null ); } - public static JNIThread getRetained( Context context, long rowid, boolean makeNew ) + public static JNIThread getRetained( GameLock lock ) + { + return getRetained( lock.getRowid(), lock ); + } + + private static JNIThread getRetained( long rowid, GameLock lock ) { JNIThread result = null; synchronized( s_instances ) { result = s_instances.get( rowid ); - if ( null == result && makeNew ) { - 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 ); - } else { - DbgUtils.toastNoLock( TAG, context, "getRetained(%d)", rowid ); - } - } + if ( null == result && null != lock ) { + result = new JNIThread( lock ); + s_instances.put( rowid, result ); } if ( null != result ) { result.retain_sync();