Skip to content

Commit a69dd89

Browse files
dconeybedconeybe
committed
get rid of thread-safe state setting code, and just require the main thread
1 parent d760569 commit a69dd89

File tree

1 file changed

+79
-170
lines changed

1 file changed

+79
-170
lines changed

‎firebase-dataconnect/demo/src/main/kotlin/com/google/firebase/dataconnect/minimaldemo/MainActivityViewModel.kt

Lines changed: 79 additions & 170 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@
1616
package com.google.firebase.dataconnect.minimaldemo
1717

1818
import android.util.Log
19+
import androidx.annotation.MainThread
1920
import androidx.lifecycle.ViewModel
2021
import androidx.lifecycle.ViewModelProvider
2122
import androidx.lifecycle.ViewModelProvider.AndroidViewModelFactory.Companion.APPLICATION_KEY
@@ -30,18 +31,21 @@ import io.kotest.property.Arb
3031
import io.kotest.property.RandomSource
3132
import io.kotest.property.arbitrary.next
3233
import java.util.Objects
33-
import kotlinx.coroutines.CoroutineStart
34+
import kotlinx.coroutines.CancellationException
3435
import kotlinx.coroutines.Deferred
3536
import kotlinx.coroutines.ExperimentalCoroutinesApi
36-
import kotlinx.coroutines.Job
3737
import kotlinx.coroutines.async
3838
import kotlinx.coroutines.flow.MutableStateFlow
3939
import kotlinx.coroutines.flow.StateFlow
4040
import kotlinx.coroutines.flow.asStateFlow
41+
import kotlinx.coroutines.launch
4142
import kotlinx.serialization.Serializable
4243

4344
class MainActivityViewModel(private val app: MyApplication) : ViewModel() {
4445

46+
// Threading Note: _state may be _read_ by any thread, but _MUST ONLY_ be written to by the
47+
// main thread. To support writing on other threads, special concurrency controls must be put
48+
// in place to address the resulting race condition.
4549
private val _state =
4650
MutableStateFlow(
4751
State(
@@ -56,62 +60,37 @@ class MainActivityViewModel(private val app: MyApplication) : ViewModel() {
5660

5761
private val rs = RandomSource.default()
5862

63+
@OptIn(ExperimentalCoroutinesApi::class)
64+
@MainThread
5965
fun insertItem() {
60-
while (true) {
61-
if (tryInsertItem()) {
62-
break
63-
}
64-
}
65-
}
66-
67-
private fun tryInsertItem(): Boolean {
6866
val arb = Arb.insertItemVariables()
6967
val variables = if (rs.random.nextFloat() < 0.333f) arb.edgecase(rs)!! else arb.next(rs)
7068

71-
val oldState = _state.value
69+
val originalState = _state.value
7270

7371
// If there is already an "insert" in progress, then just return and let the in-progress
7472
// operation finish.
75-
when (oldState.getItem) {
76-
is State.OperationState.InProgress -> return true
73+
when (originalState.getItem) {
74+
is State.OperationState.InProgress -> return
7775
is State.OperationState.New,
7876
is State.OperationState.Completed -> Unit
7977
}
8078

81-
// Create a new coroutine to perform the "insert" operation, but don't start it yet by
82-
// specifying start=CoroutineStart.LAZY because we won't start it until the state is
83-
// successfully set.
84-
val newInsertJob: Deferred<Zwda6x9zyyKey> =
85-
viewModelScope.async(start = CoroutineStart.LAZY) {
86-
app.getConnector().insertItem.ref(variables).execute().data.key
87-
}
88-
89-
// Update the state and start the coroutine if it is successfully set.
90-
val insertItemOperationInProgressState =
91-
State.OperationState.InProgress(oldState.nextSequenceNumber, variables, newInsertJob)
92-
val newState = oldState.withInsertInProgress(insertItemOperationInProgressState)
93-
if (!_state.compareAndSet(oldState, newState)) {
94-
return false
95-
}
96-
97-
// Actually start the coroutine now that the state has been set.
79+
// Start a new coroutine to perform the "insert" operation.
9880
Log.i(TAG, "Inserting item: $variables")
99-
newState.startInsert(insertItemOperationInProgressState)
100-
return true
101-
}
102-
103-
@OptIn(ExperimentalCoroutinesApi::class)
104-
private fun State.startInsert(
105-
insertItemOperationInProgressState:
106-
State.OperationState.InProgress<InsertItemMutation.Variables, Zwda6x9zyyKey>
107-
) {
108-
require(insertItemOperationInProgressState === insertItem)
109-
val job: Deferred<Zwda6x9zyyKey> = insertItemOperationInProgressState.job
110-
val variables: InsertItemMutation.Variables = insertItemOperationInProgressState.variables
111-
112-
job.start()
81+
val job: Deferred<Zwda6x9zyyKey> =
82+
viewModelScope.async { app.getConnector().insertItem.ref(variables).execute().data.key }
83+
val inProgressOperationState =
84+
State.OperationState.InProgress(originalState.nextSequenceNumber, variables, job)
85+
_state.value = originalState.withInsertInProgress(inProgressOperationState)
11386

87+
// Update the internal state once the "insert" operation has completed.
11488
job.invokeOnCompletion { exception ->
89+
// Don't log CancellationException, as document by invokeOnCompletion().
90+
if (exception is CancellationException) {
91+
return@invokeOnCompletion
92+
}
93+
11594
val result =
11695
if (exception !== null) {
11796
Log.w(TAG, "WARNING: Inserting item FAILED: $exception (variables=$variables)", exception)
@@ -122,187 +101,117 @@ class MainActivityViewModel(private val app: MyApplication) : ViewModel() {
122101
Result.success(key)
123102
}
124103

125-
while (true) {
104+
viewModelScope.launch {
126105
val oldState = _state.value
127-
if (oldState.insertItem !== insertItemOperationInProgressState) {
128-
break
129-
}
130-
131-
val insertItemOperationCompletedState =
132-
State.OperationState.Completed(oldState.nextSequenceNumber, variables, result)
133-
val newState = oldState.withInsertCompleted(insertItemOperationCompletedState)
134-
if (_state.compareAndSet(oldState, newState)) {
135-
break
106+
if (oldState.insertItem === inProgressOperationState) {
107+
_state.value =
108+
oldState.withInsertCompleted(
109+
State.OperationState.Completed(oldState.nextSequenceNumber, variables, result)
110+
)
136111
}
137112
}
138113
}
139114
}
140115

116+
@OptIn(ExperimentalCoroutinesApi::class)
141117
fun getItem() {
142-
while (true) {
143-
if (tryGetItem()) {
144-
break
145-
}
146-
}
147-
}
148-
149-
private fun tryGetItem(): Boolean {
150-
val oldState = _state.value
118+
val originalState = _state.value
151119

152120
// If there is no previous successful "insert" operation, then we don't know any ID's to get,
153121
// so just do nothing.
154-
val key: Zwda6x9zyyKey = oldState.lastInsertedKey ?: return true
122+
val key: Zwda6x9zyyKey = originalState.lastInsertedKey ?: return
155123

156124
// If there is already a "get" in progress, then just return and let the in-progress operation
157125
// finish.
158-
when (oldState.getItem) {
159-
is State.OperationState.InProgress -> return true
126+
when (originalState.getItem) {
127+
is State.OperationState.InProgress -> return
160128
is State.OperationState.New,
161129
is State.OperationState.Completed -> Unit
162130
}
163131

164-
// Create a new coroutine to perform the "get" operation, but don't start it yet by specifying
165-
// start=CoroutineStart.LAZY because we won't start it until the state is successfully set.
166-
val newGetJob: Deferred<GetItemByKeyQuery.Data.Item?> =
167-
viewModelScope.async(start = CoroutineStart.LAZY) {
168-
app.getConnector().getItemByKey.execute(key).data.item
169-
}
170-
171-
// Update the state and start the coroutine if it is successfully set.
172-
val getItemOperationInProgressState =
173-
State.OperationState.InProgress(oldState.nextSequenceNumber, key, newGetJob)
174-
val newState = oldState.withGetInProgress(getItemOperationInProgressState)
175-
if (!_state.compareAndSet(oldState, newState)) {
176-
return false
177-
}
178-
179-
// Actually start the coroutine now that the state has been set.
180-
Log.i(TAG, "Getting item with key: $key")
181-
newState.startGet(getItemOperationInProgressState)
182-
return true
183-
}
184-
185-
@OptIn(ExperimentalCoroutinesApi::class)
186-
private fun State.startGet(
187-
getItemOperationInProgressState:
188-
State.OperationState.InProgress<Zwda6x9zyyKey, GetItemByKeyQuery.Data.Item?>
189-
) {
190-
require(getItemOperationInProgressState === getItem)
191-
val job: Deferred<GetItemByKeyQuery.Data.Item?> = getItemOperationInProgressState.job
192-
val key: Zwda6x9zyyKey = getItemOperationInProgressState.variables
193-
194-
job.start()
132+
// Start a new coroutine to perform the "get" operation.
133+
Log.i(TAG, "Retrieving item with key: $key")
134+
val job: Deferred<GetItemByKeyQuery.Data.Item?> =
135+
viewModelScope.async { app.getConnector().getItemByKey.execute(key).data.item }
136+
val inProgressOperationState =
137+
State.OperationState.InProgress(originalState.nextSequenceNumber, key, job)
138+
_state.value = originalState.withGetInProgress(inProgressOperationState)
195139

140+
// Update the internal state once the "get" operation has completed.
196141
job.invokeOnCompletion { exception ->
142+
// Don't log CancellationException, as document by invokeOnCompletion().
143+
if (exception is CancellationException) {
144+
return@invokeOnCompletion
145+
}
146+
197147
val result =
198148
if (exception !== null) {
199-
Log.w(TAG, "WARNING: Getting item with key $key FAILED: $exception", exception)
149+
Log.w(TAG, "WARNING: Retrieving item with key=$key FAILED: $exception", exception)
200150
Result.failure(exception)
201151
} else {
202152
val item = job.getCompleted()
203-
Log.i(TAG, "Got item with key $key: $item")
153+
Log.i(TAG, "Retrieved item with key: $key (item=${item})")
204154
Result.success(item)
205155
}
206156

207-
while (true) {
157+
viewModelScope.launch {
208158
val oldState = _state.value
209-
if (oldState.getItem !== getItemOperationInProgressState) {
210-
break
211-
}
212-
213-
val getItemOperationCompletedState =
214-
State.OperationState.Completed(
215-
oldState.nextSequenceNumber,
216-
getItemOperationInProgressState.variables,
217-
result,
218-
)
219-
val newState = oldState.withGetCompleted(getItemOperationCompletedState)
220-
if (_state.compareAndSet(oldState, newState)) {
221-
break
159+
if (oldState.getItem === inProgressOperationState) {
160+
_state.value =
161+
oldState.withGetCompleted(
162+
State.OperationState.Completed(oldState.nextSequenceNumber, key, result)
163+
)
222164
}
223165
}
224166
}
225167
}
226168

227169
fun deleteItem() {
228-
while (true) {
229-
if (tryDeleteItem()) {
230-
break
231-
}
232-
}
233-
}
234-
235-
private fun tryDeleteItem(): Boolean {
236-
val oldState = _state.value
170+
val originalState = _state.value
237171

238172
// If there is no previous successful "insert" operation, then we don't know any ID's to delete,
239173
// so just do nothing.
240-
val key: Zwda6x9zyyKey = oldState.lastInsertedKey ?: return true
174+
val key: Zwda6x9zyyKey = originalState.lastInsertedKey ?: return
241175

242176
// If there is already a "delete" in progress, then just return and let the in-progress
243177
// operation finish.
244-
when (oldState.deleteItem) {
245-
is State.OperationState.InProgress -> return true
178+
when (originalState.getItem) {
179+
is State.OperationState.InProgress -> return
246180
is State.OperationState.New,
247181
is State.OperationState.Completed -> Unit
248182
}
249183

250-
// Create a new coroutine to perform the "delete" operation, but don't start it yet by
251-
// specifying start=CoroutineStart.LAZY because we won't start it until the state is
252-
// successfully set.
253-
val newDeleteJob: Deferred<Unit> =
254-
viewModelScope.async(start = CoroutineStart.LAZY) {
255-
app.getConnector().deleteItemByKey.execute(key)
256-
}
257-
258-
// Update the state and start the coroutine if it is successfully set.
259-
val deleteItemOperationInProgressState =
260-
State.OperationState.InProgress(oldState.nextSequenceNumber, key, newDeleteJob)
261-
val newState = oldState.withDeleteInProgress(deleteItemOperationInProgressState)
262-
if (!_state.compareAndSet(oldState, newState)) {
263-
return false
264-
}
265-
266-
// Actually start the coroutine now that the state has been set.
184+
// Start a new coroutine to perform the "delete" operation.
267185
Log.i(TAG, "Deleting item with key: $key")
268-
newState.startDelete(deleteItemOperationInProgressState)
269-
return true
270-
}
271-
272-
private fun State.startDelete(
273-
deleteItemOperationInProgressState: State.OperationState.InProgress<Zwda6x9zyyKey, Unit>
274-
) {
275-
require(deleteItemOperationInProgressState === deleteItem)
276-
val job: Job = deleteItemOperationInProgressState.job
277-
val key: Zwda6x9zyyKey = deleteItemOperationInProgressState.variables
278-
279-
job.start()
186+
val job: Deferred<Unit> =
187+
viewModelScope.async { app.getConnector().deleteItemByKey.execute(key) }
188+
val inProgressOperationState =
189+
State.OperationState.InProgress(originalState.nextSequenceNumber, key, job)
190+
_state.value = originalState.withDeleteInProgress(inProgressOperationState)
280191

192+
// Update the internal state once the "delete" operation has completed.
281193
job.invokeOnCompletion { exception ->
194+
// Don't log CancellationException, as document by invokeOnCompletion().
195+
if (exception is CancellationException) {
196+
return@invokeOnCompletion
197+
}
198+
282199
val result =
283200
if (exception !== null) {
284-
Log.w(TAG, "WARNING: Deleting item with key $key FAILED: $exception", exception)
201+
Log.w(TAG, "WARNING: Deleting item with key=$key FAILED: $exception", exception)
285202
Result.failure(exception)
286203
} else {
287-
Log.i(TAG, "Deleted item with key $key")
204+
Log.i(TAG, "Deleted item with key: $key")
288205
Result.success(Unit)
289206
}
290207

291-
while (true) {
208+
viewModelScope.launch {
292209
val oldState = _state.value
293-
if (oldState.deleteItem !== deleteItemOperationInProgressState) {
294-
break
295-
}
296-
297-
val deleteItemOperationCompletedState =
298-
State.OperationState.Completed(
299-
oldState.nextSequenceNumber,
300-
deleteItemOperationInProgressState.variables,
301-
result,
302-
)
303-
val newState = oldState.withDeleteCompleted(deleteItemOperationCompletedState)
304-
if (_state.compareAndSet(oldState, newState)) {
305-
break
210+
if (oldState.deleteItem === inProgressOperationState) {
211+
_state.value =
212+
oldState.withDeleteCompleted(
213+
State.OperationState.Completed(oldState.nextSequenceNumber, key, result)
214+
)
306215
}
307216
}
308217
}

0 commit comments

Comments
 (0)