Skip to content

Commit 43949f1

Browse files
committed
Analysis: Resolve synchronized on suspend coroutines warning
Warning Messages: "@synchronized annotation is not applicable to suspend functions and lambdas" It turns out that this '@synchronized' solution were already replacing the previous (d5b9948) 'Mutex' solution, and this, due to the fact that the existing (back then) 'Mutex' solution were causing crashes on Samsung devices running Android 5 (API Level 21). However, this repo, and thus both, the WordPress and Jetpack apps, are no longer supporting these devices as the current 'minSdkVersion' is '24' (Android 7). Thus, this '@synchronized' annotation can be now safely removed and the 'Mutex' solution reverting back. For more info see: - Fix crash in UploadStarter on Samsung devices with Android 5 #10877 (Introduced In): https://github.com/wordpress-mobile/WordPress-Android/ pull/10877/ - ExceptionInInitializerError #10827 (Crash Report): #10827 - NoSuchFieldException with AtomicReferenceFieldUpdater on Samsung Android 5.0.x devices #490 (Coroutines Issue): Kotlin/kotlinx.coroutines#490
1 parent 2fd0fc8 commit 43949f1

File tree

1 file changed

+43
-36
lines changed

1 file changed

+43
-36
lines changed

WordPress/src/main/java/org/wordpress/android/ui/uploads/UploadStarter.kt

+43-36
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@ import kotlinx.coroutines.Job
1313
import kotlinx.coroutines.async
1414
import kotlinx.coroutines.coroutineScope
1515
import kotlinx.coroutines.launch
16+
import kotlinx.coroutines.sync.Mutex
1617
import org.wordpress.android.analytics.AnalyticsTracker.Stat
1718
import org.wordpress.android.fluxc.Dispatcher
1819
import org.wordpress.android.fluxc.generated.UploadActionBuilder
@@ -65,6 +66,13 @@ class UploadStarter @Inject constructor(
6566
) : CoroutineScope {
6667
private val job = Job()
6768

69+
/**
70+
* When the app comes to foreground both `queueUploadFromAllSites` and `queueUploadFromSite` are invoked.
71+
* The problem is that they can run in parallel and `uploadServiceFacade.isPostUploadingOrQueued(it)` might return
72+
* out-of-date result and a same post is added twice.
73+
*/
74+
private val mutex = Mutex()
75+
6876
override val coroutineContext: CoroutineContext get() = job + bgDispatcher
6977

7078
/**
@@ -136,45 +144,44 @@ class UploadStarter @Inject constructor(
136144

137145
/**
138146
* This is meant to be used by [checkConnectionAndUpload] only.
139-
*
140-
* The method needs to be synchronized from the following reasons. When the app comes to foreground both
141-
* `queueUploadFromAllSites` and `queueUploadFromSite` are invoked. The problem is that they can run in parallel
142-
* and `uploadServiceFacade.isPostUploadingOrQueued(it)` might return out-of-date result and a same post is added
143-
* twice.
144147
*/
145-
@Suppress("SYNCHRONIZED_ON_SUSPEND")
146-
@Synchronized
147148
private suspend fun upload(site: SiteModel) = coroutineScope {
148-
val posts = async { postStore.getPostsWithLocalChanges(site) }
149-
val pages = async { pageStore.getPagesWithLocalChanges(site) }
150-
val list = posts.await() + pages.await()
149+
try {
150+
mutex.lock()
151+
152+
val posts = async { postStore.getPostsWithLocalChanges(site) }
153+
val pages = async { pageStore.getPagesWithLocalChanges(site) }
154+
val list = posts.await() + pages.await()
151155

152-
list.asSequence()
153-
.map { post ->
154-
val action = uploadActionUseCase.getAutoUploadAction(post, site)
155-
Pair(post, action)
156-
}
157-
.filter { (_, action) ->
158-
action != DO_NOTHING
159-
}
160-
.toList()
161-
.forEach { (post, action) ->
162-
trackAutoUploadAction(action, post.status, post.isPage)
163-
AppLog.d(
164-
AppLog.T.POSTS,
165-
"UploadStarter for post (isPage: ${post.isPage}) title: ${post.title}, action: $action"
166-
)
167-
dispatcher.dispatch(
168-
UploadActionBuilder.newIncrementNumberOfAutoUploadAttemptsAction(
169-
post
170-
)
171-
)
172-
uploadServiceFacade.uploadPost(
173-
context = context,
174-
post = post,
175-
trackAnalytics = false
176-
)
177-
}
156+
list.asSequence()
157+
.map { post ->
158+
val action = uploadActionUseCase.getAutoUploadAction(post, site)
159+
Pair(post, action)
160+
}
161+
.filter { (_, action) ->
162+
action != DO_NOTHING
163+
}
164+
.toList()
165+
.forEach { (post, action) ->
166+
trackAutoUploadAction(action, post.status, post.isPage)
167+
AppLog.d(
168+
AppLog.T.POSTS,
169+
"UploadStarter for post (isPage: ${post.isPage}) title: ${post.title}, action: $action"
170+
)
171+
dispatcher.dispatch(
172+
UploadActionBuilder.newIncrementNumberOfAutoUploadAttemptsAction(
173+
post
174+
)
175+
)
176+
uploadServiceFacade.uploadPost(
177+
context = context,
178+
post = post,
179+
trackAnalytics = false
180+
)
181+
}
182+
} finally {
183+
mutex.unlock()
184+
}
178185
}
179186

180187
private fun trackAutoUploadAction(

0 commit comments

Comments
 (0)