diff options
author | Android Build Coastguard Worker <android-build-coastguard-worker@google.com> | 2024-04-23 22:56:20 +0000 |
---|---|---|
committer | Gerrit Code Review <noreply-gerritcodereview@google.com> | 2024-04-23 22:56:20 +0000 |
commit | 4e91af5f5597ac6275559f2ce4bc2f5cc852bddb (patch) | |
tree | 0abb540b2f8d878a5378997f9f44d3cf4514cb6b | |
parent | 8cb4f17bcd16bd758f3d93838da28bb9294a0fbf (diff) | |
parent | 69e7c0e40ec3b0a2073ab5b80d398ab23a670f20 (diff) | |
download | support-androidx-datastore-release.tar.gz |
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" |