fix: Private sync hanging when there is not space available (#96)

Closes #83
relaycorp · May 27, 2020 · 0953cfb · 0953cfb
1 parent eadf005
commit 0953cfb
Show file tree

Hide file tree

Showing 9 changed files with 59 additions and 37 deletions.
diff --git a/app/src/main/AndroidManifest.xml b/app/src/main/AndroidManifest.xml
@@ -29,12 +29,14 @@
         <activity
             android:name=".ui.sync.internet.InternetSyncActivity"
             android:label="@string/sync_internet"
+            android:launchMode="singleTask"
             android:screenOrientation="portrait"
             android:theme="@style/Theme.Courier.Sync" />
 
         <activity
             android:name=".ui.sync.people.PeopleSyncActivity"
             android:label="@string/sync_people"
+            android:launchMode="singleTask"
             android:screenOrientation="portrait"
             android:theme="@style/Theme.Courier.Sync" />
 

diff --git a/app/src/main/java/tech/relaycorp/cogrpc/server/CogRPCConnectionService.kt b/app/src/main/java/tech/relaycorp/cogrpc/server/CogRPCConnectionService.kt
@@ -1,5 +1,7 @@
 package tech.relaycorp.cogrpc.server
 
+import io.grpc.Status
+import io.grpc.StatusRuntimeException
 import io.grpc.stub.StreamObserver
 import kotlinx.coroutines.CoroutineScope
 import kotlinx.coroutines.launch
@@ -26,6 +28,10 @@ class CogRPCConnectionService(
                     if (result) {
                         logger.info("deliverCargo next ack ${cargoDelivery.id}")
                         responseObserver.onNext(cargoDelivery.toAck())
+                    } else {
+                        logger.info("deliverCargo no space available for ${cargoDelivery.id}")
+                        logger.info("deliverCargo closing with error")
+                        responseObserver.onError(StatusRuntimeException(Status.RESOURCE_EXHAUSTED))
                     }
                 }
             }

diff --git a/app/src/main/java/tech/relaycorp/cogrpc/server/CogRPCServer.kt b/app/src/main/java/tech/relaycorp/cogrpc/server/CogRPCServer.kt
@@ -81,7 +81,9 @@ internal constructor(
     }
 
     fun stop() {
-        server?.shutdown()
+        server
+            ?.shutdown()
+            ?.awaitTermination(TERMINATION_TIMEOUT.inSeconds.toLong(), TimeUnit.SECONDS)
         server = null
 
         job.cancel()
@@ -107,6 +109,7 @@ internal constructor(
         private val MAX_CONNECTION_AGE = 15.minutes
         private val MAX_CONNECTION_AGE_GRACE = 30.seconds
         private val MAX_CONNECTION_IDLE = 10.seconds
+        private val TERMINATION_TIMEOUT = 5.seconds
     }
 
     interface Service {

diff --git a/app/src/main/java/tech/relaycorp/courier/domain/PrivateSync.kt b/app/src/main/java/tech/relaycorp/courier/domain/PrivateSync.kt
@@ -21,10 +21,11 @@ class PrivateSync
     suspend fun startSync() {
         if (cogRPCServer.isStarted) throw IllegalStateException("Sync already started")
 
-        state.send(State.Syncing)
+        state.send(State.Starting)
         cogRPCServer.start(service) {
             state.sendBlocking(State.Error)
         }
+        if (state.value == State.Starting) state.send(State.Syncing)
     }
 
     fun stopSync() {
@@ -33,6 +34,6 @@ class PrivateSync
     }
 
     enum class State {
-        Syncing, Stopped, Error
+        Starting, Syncing, Stopped, Error
     }
 }
diff --git a/app/src/main/java/tech/relaycorp/courier/ui/sync/people/PeopleSyncActivity.kt b/app/src/main/java/tech/relaycorp/courier/ui/sync/people/PeopleSyncActivity.kt
@@ -4,6 +4,7 @@ import android.content.Context
 import android.content.Intent
 import android.os.Bundle
 import androidx.appcompat.app.AlertDialog
+import androidx.core.view.isInvisible
 import androidx.core.view.isVisible
 import androidx.lifecycle.ViewModelProvider
 import androidx.lifecycle.lifecycleScope
@@ -54,8 +55,8 @@ class PeopleSyncActivity : BaseActivity() {
                     state is PeopleSyncViewModel.State.Syncing.HadFirstClient
                 clientsConnected.text = state.clientsConnectedValue.toString()
 
+                stop.isInvisible = state !is PeopleSyncViewModel.State.Syncing
                 val isDone = state == PeopleSyncViewModel.State.Error
-                stop.isVisible = !isDone
                 close.isVisible = isDone
                 if (!isDone) {
                     animation.startLoopingAvd(R.drawable.ic_sync_animation)
@@ -112,6 +113,7 @@ class PeopleSyncActivity : BaseActivity() {
 
     private fun PeopleSyncViewModel.State.toSyncMessageRes() =
         when (this) {
+            PeopleSyncViewModel.State.Starting -> R.string.sync_people_starting
             PeopleSyncViewModel.State.Syncing.WaitingFirstClient -> R.string.sync_people_syncing
             is PeopleSyncViewModel.State.Syncing.HadFirstClient -> R.string.sync_people_syncing_some
             is PeopleSyncViewModel.State.Error -> R.string.sync_error

diff --git a/app/src/main/java/tech/relaycorp/courier/ui/sync/people/PeopleSyncViewModel.kt b/app/src/main/java/tech/relaycorp/courier/ui/sync/people/PeopleSyncViewModel.kt
@@ -5,7 +5,6 @@ import kotlinx.coroutines.flow.asFlow
 import kotlinx.coroutines.flow.combine
 import kotlinx.coroutines.flow.drop
 import kotlinx.coroutines.flow.filter
-import kotlinx.coroutines.flow.first
 import kotlinx.coroutines.flow.launchIn
 import kotlinx.coroutines.flow.onEach
 import kotlinx.coroutines.flow.take
@@ -22,7 +21,7 @@ import javax.inject.Inject
 class PeopleSyncViewModel
 @Inject constructor(
     private val privateSync: PrivateSync,
-    private val wifiHotspotStateReceiver: WifiHotspotStateReceiver
+    wifiHotspotStateReceiver: WifiHotspotStateReceiver
 ) : BaseViewModel() {
 
     // Inputs
@@ -82,29 +81,30 @@ class PeopleSyncViewModel
             .onEach { hadFirstClient = true }
             .launchIn(ioScope)
 
-        privateSync
-            .state()
-            .combine(privateSync.clientsConnected()) { syncState, clientsConnected ->
-                if (syncState == PrivateSync.State.Stopped) {
-                    finish.send(Finish)
-                } else {
-                    state.send(
-                        when (syncState) {
-                            PrivateSync.State.Syncing -> if (hadFirstClient) {
-                                State.Syncing.HadFirstClient(clientsConnected)
-                            } else {
-                                State.Syncing.WaitingFirstClient
-                            }
-                            else -> State.Error
-                        }
-                    )
-                }
+        combine(
+            privateSync.state(),
+            privateSync.clientsConnected()
+        ) { syncState, clientsConnected ->
+            when (syncState) {
+                PrivateSync.State.Starting -> state.send(State.Starting)
+                PrivateSync.State.Syncing -> state.send(
+                    if (hadFirstClient) {
+                        State.Syncing.HadFirstClient(clientsConnected)
+                    } else {
+                        State.Syncing.WaitingFirstClient
+                    }
+                )
+                PrivateSync.State.Stopped -> finish.send(Finish)
+                PrivateSync.State.Error -> state.send(State.Error)
             }
+        }
             .launchIn(ioScope)
 
         stopClicks
             .asFlow()
             .onEach {
+                if (state.value == State.Starting) return@onEach
+
                 val clientsConnected =
                     (state.value as? State.Syncing.HadFirstClient)?.clientsConnected ?: 0
                 if (clientsConnected > 0) {
@@ -126,9 +126,8 @@ class PeopleSyncViewModel
         super.onCleared()
     }
 
-    private suspend fun getHotspotState() = wifiHotspotStateReceiver.state().first()
-
     sealed class State {
+        object Starting : State()
         sealed class Syncing : State() {
             object WaitingFirstClient : Syncing()
             data class HadFirstClient(val clientsConnected: Int) : Syncing()

diff --git a/app/src/main/res/layout/activity_people_sync.xml b/app/src/main/res/layout/activity_people_sync.xml
@@ -4,6 +4,7 @@
     xmlns:tools="http://schemas.android.com/tools"
     android:layout_width="match_parent"
     android:layout_height="match_parent"
+    android:animateLayoutChanges="true"
     tools:context=".ui.sync.people.PeopleSyncActivity">
 
     <include layout="@layout/common_app_bar" />
@@ -67,20 +68,24 @@
             </LinearLayout>
         </FrameLayout>
 
-        <com.google.android.material.button.MaterialButton
-            android:id="@+id/stop"
-            style="?materialButtonOutlinedStyle"
+        <FrameLayout
             android:layout_width="wrap_content"
-            android:layout_height="wrap_content"
-            android:text="@string/stop" />
+            android:layout_height="wrap_content">
 
-        <com.google.android.material.button.MaterialButton
-            android:id="@+id/close"
-            android:layout_width="wrap_content"
-            android:layout_height="wrap_content"
-            android:text="@string/close"
-            android:visibility="gone"
-            tools:visibility="visible" />
+            <com.google.android.material.button.MaterialButton
+                android:id="@+id/stop"
+                style="?materialButtonOutlinedStyle"
+                android:layout_width="wrap_content"
+                android:layout_height="wrap_content"
+                android:text="@string/stop" />
 
+            <com.google.android.material.button.MaterialButton
+                android:id="@+id/close"
+                android:layout_width="wrap_content"
+                android:layout_height="wrap_content"
+                android:text="@string/close"
+                android:visibility="gone"
+                tools:visibility="visible" />
+        </FrameLayout>
     </LinearLayout>
 </androidx.coordinatorlayout.widget.CoordinatorLayout>
diff --git a/app/src/main/res/values/strings.xml b/app/src/main/res/values/strings.xml
@@ -58,6 +58,7 @@
     <string name="sync_internet_stop_confirm_message">The sync hasn\'t finished yet.</string>
 
     <!-- Private / People Sync -->
+    <string name="sync_people_starting">Setting up</string>
     <string name="sync_people_syncing">Syncing</string>
     <string name="sync_people_waiting_first_client">Waiting for the first person to connect</string>
     <string name="sync_people_syncing_some">Syncing with</string>

diff --git a/app/src/test/java/tech/relaycorp/cogrpc/server/CogRPCServerDeliveryCargoTest.kt b/app/src/test/java/tech/relaycorp/cogrpc/server/CogRPCServerDeliveryCargoTest.kt
@@ -1,5 +1,7 @@
 package tech.relaycorp.cogrpc.server
 
+import io.grpc.Status
+import io.grpc.StatusRuntimeException
 import io.grpc.internal.testing.StreamRecorder
 import kotlinx.coroutines.test.runBlockingTest
 import org.junit.jupiter.api.Assertions.assertEquals
@@ -64,6 +66,7 @@ internal class CogRPCServerDeliveryCargoTest {
                 .toString(Charset.defaultCharset())
         )
         assertTrue(ackRecorder.values.isEmpty())
+        assertEquals(Status.RESOURCE_EXHAUSTED, (ackRecorder.error as StatusRuntimeException).status)
 
         testServer.stop()
     }