Merge "Merge cherrypicks of ['android-review.googlesource.com/3018365', 'android-review.googlesource.com/3017333', 'android-review.googlesource.com/3056542'] into androidx-datastore-release." into androidx-datastore-releaseandroidx-datastore-release

4 files changed, 147 insertions, 3 deletions

diff --git a/datastore/datastore-core/src/androidInstrumentedTest/kotlin/androidx/datastore/core/multiprocess/MultiProcessDataStoreIpcTest.kt b/datastore/datastore-core/src/androidInstrumentedTest/kotlin/androidx/datastore/core/multiprocess/MultiProcessDataStoreIpcTest.kt
index c3351b393d2..0da13913fb5 100644
--- a/datastore/datastore-core/src/androidInstrumentedTest/kotlin/androidx/datastore/core/multiprocess/MultiProcessDataStoreIpcTest.kt
+++ b/datastore/datastore-core/src/androidInstrumentedTest/kotlin/androidx/datastore/core/multiprocess/MultiProcessDataStoreIpcTest.kt
@@ -470,4 +470,118 @@ class MultiProcessDataStoreIpcTest {
             "remoteValue"
         )
     }
+
+    @Test
+    fun testConcurrentUpdateNoDeadlock_file() = testConcurrentUpdateNoDeadlock(StorageVariant.FILE)
+
+    @Test
+    fun testConcurrentUpdateNoDeadlock_okio() = testConcurrentUpdateNoDeadlock(StorageVariant.OKIO)
+
+    /**
+     * Reproduce the false alarm on deadlock by Linux. It happens in the case where:<br>
+     * 1. process A holds file lock 1;<br>
+     * 2. process B holds file lock 2;<br>
+     * 3. process B (could be another thread than 2.) waits to hold file lock 1 (still held by A);<br>
+     * 4. process A (could be another thread than 1.) waits to hold file lock 2 (still held by B) -
+     * exception "Resource deadlock would occur" is thrown - caught and retried with exponential
+     * backoff.
+     */
+    private fun testConcurrentUpdateNoDeadlock(
+        storageVariant: StorageVariant
+    ) = multiProcessRule.runTest {
+        val connection = multiProcessRule.createConnection()
+        val subject1 = connection.createSubject(
+            multiProcessRule.datastoreScope
+        )
+        val subject2 = connection.createSubject(
+            multiProcessRule.datastoreScope
+        )
+
+        val file1 = tmpFolder.newFile()
+        val file2 = tmpFolder.newFile()
+        val datastore1 = createMultiProcessTestDatastore(
+            filePath = file1.canonicalPath,
+            storageVariant = storageVariant,
+            hostDatastoreScope = multiProcessRule.datastoreScope,
+            subjects = arrayOf(subject1)
+        )
+        val datastore2 = createMultiProcessTestDatastore(
+            filePath = file2.canonicalPath,
+            storageVariant = storageVariant,
+            hostDatastoreScope = multiProcessRule.datastoreScope,
+            subjects = arrayOf(subject2)
+        )
+
+        // setup real data and lock file
+        datastore1.updateData {
+            it.toBuilder().setText("hostData").build()
+        }
+        datastore2.updateData {
+            it.toBuilder().setText("hostData").build()
+        }
+
+        // process A holds file lock 1
+        val blockWrite = CompletableDeferred<Unit>()
+        val startedWrite = CompletableDeferred<Unit>()
+
+        val localUpdate1 = async {
+            datastore1.updateData {
+                startedWrite.complete(Unit)
+                blockWrite.await()
+                it.toBuilder().setInteger(3).build()
+            }
+        }
+        startedWrite.await()
+
+        // process B holds file lock 2
+        val commitWriteLatch1 = InterProcessCompletable<IpcUnit>()
+        val writeStartedLatch1 = InterProcessCompletable<IpcUnit>()
+        val setTextAction1 = async {
+            subject2.invokeInRemoteProcess(
+                SetTextAction(
+                    value = "remoteValue",
+                    commitTransactionLatch = commitWriteLatch1,
+                    transactionStartedLatch = writeStartedLatch1
+                )
+            )
+        }
+        writeStartedLatch1.await(subject2)
+
+        // process B (could be another thread than 2.) waits to hold file lock 1
+        val commitWriteLatch2 = InterProcessCompletable<IpcUnit>()
+        val actionStartedLatch = InterProcessCompletable<IpcUnit>()
+        val setTextAction2 = async {
+            subject1.invokeInRemoteProcess(
+                SetTextAction(
+                    value = "remoteValue",
+                    commitTransactionLatch = commitWriteLatch2,
+                    actionStartedLatch = actionStartedLatch
+                )
+            )
+        }
+        actionStartedLatch.await(subject1)
+        // wait a bit to let the other process get into updateData, might be flaky
+        delay(100)
+
+        // process A (could be another thread than 1.) waits to hold file lock 2 (still held by B)
+        val localUpdate2 = async {
+            datastore2.updateData {
+                it.toBuilder().setInteger(4).build()
+            }
+        }
+
+        blockWrite.complete(Unit)
+        commitWriteLatch1.complete(subject2, IpcUnit)
+        commitWriteLatch2.complete(subject1, IpcUnit)
+
+        setTextAction1.await()
+        setTextAction2.await()
+        localUpdate1.await()
+        localUpdate2.await()
+
+        assertThat(datastore1.data.first().text).isEqualTo("remoteValue")
+        assertThat(datastore1.data.first().integer).isEqualTo(3)
+        assertThat(datastore2.data.first().text).isEqualTo("remoteValue")
+        assertThat(datastore2.data.first().integer).isEqualTo(4)
+    }
 }
diff --git a/datastore/datastore-core/src/androidInstrumentedTest/kotlin/androidx/datastore/core/multiprocess/ipcActions/SetTextAction.kt b/datastore/datastore-core/src/androidInstrumentedTest/kotlin/androidx/datastore/core/multiprocess/ipcActions/SetTextAction.kt
index f442d6cabbb..54440d2d080 100644
--- a/datastore/datastore-core/src/androidInstrumentedTest/kotlin/androidx/datastore/core/multiprocess/ipcActions/SetTextAction.kt
+++ b/datastore/datastore-core/src/androidInstrumentedTest/kotlin/androidx/datastore/core/multiprocess/ipcActions/SetTextAction.kt
@@ -28,10 +28,12 @@ internal class SetTextAction(
     private val value: String,
     private val transactionStartedLatch: InterProcessCompletable<IpcUnit>? = null,
     private val commitTransactionLatch: InterProcessCompletable<IpcUnit>? = null,
+    private val actionStartedLatch: InterProcessCompletable<IpcUnit>? = null,
 ) : IpcAction<IpcUnit>(), Parcelable {
     override suspend fun invokeInRemoteProcess(
         subject: TwoWayIpcSubject
     ): IpcUnit {
+        actionStartedLatch?.complete(subject, IpcUnit)
         subject.datastore.updateData {
             transactionStartedLatch?.complete(subject, IpcUnit)
             commitTransactionLatch?.await(subject)
diff --git a/datastore/datastore-core/src/androidMain/kotlin/androidx/datastore/core/MultiProcessCoordinator.android.kt b/datastore/datastore-core/src/androidMain/kotlin/androidx/datastore/core/MultiProcessCoordinator.android.kt
index 3d3edffbf0a..e596d66eada 100644
--- a/datastore/datastore-core/src/androidMain/kotlin/androidx/datastore/core/MultiProcessCoordinator.android.kt
+++ b/datastore/datastore-core/src/androidMain/kotlin/androidx/datastore/core/MultiProcessCoordinator.android.kt
@@ -25,6 +25,7 @@ import java.io.IOException
 import java.nio.channels.FileLock
 import kotlin.contracts.ExperimentalContracts
 import kotlin.coroutines.CoroutineContext
+import kotlinx.coroutines.delay
 import kotlinx.coroutines.flow.Flow
 import kotlinx.coroutines.sync.Mutex
 import kotlinx.coroutines.sync.withLock
@@ -43,7 +44,7 @@ internal class MultiProcessCoordinator(
             FileOutputStream(lockFile).use { lockFileStream ->
                 var lock: FileLock? = null
                 try {
-                    lock = lockFileStream.getChannel().lock(0L, Long.MAX_VALUE, /* shared= */ false)
+                    lock = getExclusiveFileLockWithRetryIfDeadlock(lockFileStream)
                     return block()
                 } finally {
                     lock?.release()
@@ -78,7 +79,8 @@ internal class MultiProcessCoordinator(
                         // will throw an IOException with EAGAIN error, instead of returning null as
                         // specified in {@link FileChannel#tryLock}. We only continue if the error
                         // message is EAGAIN, otherwise just throw it.
-                        if (ex.message?.startsWith(LOCK_ERROR_MESSAGE) != true) {
+                        if ((ex.message?.startsWith(LOCK_ERROR_MESSAGE) != true) &&
+                            (ex.message?.startsWith(DEADLOCK_ERROR_MESSAGE) != true)) {
                             throw ex
                         }
                     }
@@ -162,6 +164,32 @@ internal class MultiProcessCoordinator(
             }
         }
     }
+
+    companion object {
+        // Retry with exponential backoff to get file lock if it hits "Resource deadlock would
+        // occur" error until the backoff reaches [MAX_WAIT_MILLIS].
+        private suspend fun getExclusiveFileLockWithRetryIfDeadlock(
+            lockFileStream: FileOutputStream
+        ): FileLock {
+            var backoff = INITIAL_WAIT_MILLIS
+            while (backoff <= MAX_WAIT_MILLIS) {
+                try {
+                    return lockFileStream.getChannel().lock(0L, Long.MAX_VALUE, /* shared= */ false)
+                } catch (ex: IOException) {
+                    if (ex.message?.contains(DEADLOCK_ERROR_MESSAGE) != true) {
+                        throw ex
+                    }
+                    delay(backoff)
+                    backoff *= 2
+                }
+            }
+            return lockFileStream.getChannel().lock(0L, Long.MAX_VALUE, /* shared= */ false)
+        }
+
+        private val DEADLOCK_ERROR_MESSAGE = "Resource deadlock would occur"
+        private val INITIAL_WAIT_MILLIS: Long = 10
+        private val MAX_WAIT_MILLIS: Long = 60000
+    }
 }
 
 /**
diff --git a/libraryversions.toml b/libraryversions.toml
index 7004c7ce8b4..94fcf988973 100644
--- a/libraryversions.toml
+++ b/libraryversions.toml
@@ -53,7 +53,7 @@ CREDENTIALS_FIDO_QUARANTINE = "1.0.0-alpha02"
 CURSORADAPTER = "1.1.0-alpha01"
 CUSTOMVIEW = "1.2.0-alpha03"
 CUSTOMVIEW_POOLINGCONTAINER = "1.1.0-alpha01"
-DATASTORE = "1.1.0"
+DATASTORE = "1.1.1"
 DOCUMENTFILE = "1.1.0-alpha02"
 DRAGANDDROP = "1.1.0-alpha01"
 DRAWERLAYOUT = "1.3.0-alpha01"