From d5b994850d80de50edbcf3c8237aa9daf3ffedd0 Mon Sep 17 00:00:00 2001 From: malinajirka Date: Fri, 29 Nov 2019 15:31:14 +0100 Subject: [PATCH] Fix crash in UploadStarter on Samsung devices with Android 5 --- .../android/ui/uploads/UploadStarter.kt | 73 +++++++++---------- 1 file changed, 33 insertions(+), 40 deletions(-) diff --git a/WordPress/src/main/java/org/wordpress/android/ui/uploads/UploadStarter.kt b/WordPress/src/main/java/org/wordpress/android/ui/uploads/UploadStarter.kt index 86fc119af485..05cfa69a1032 100644 --- a/WordPress/src/main/java/org/wordpress/android/ui/uploads/UploadStarter.kt +++ b/WordPress/src/main/java/org/wordpress/android/ui/uploads/UploadStarter.kt @@ -12,7 +12,6 @@ import kotlinx.coroutines.CoroutineScope import kotlinx.coroutines.Job import kotlinx.coroutines.coroutineScope import kotlinx.coroutines.launch -import kotlinx.coroutines.sync.Mutex import org.wordpress.android.analytics.AnalyticsTracker.Stat import org.wordpress.android.fluxc.Dispatcher import org.wordpress.android.fluxc.generated.UploadActionBuilder @@ -63,13 +62,6 @@ class UploadStarter @Inject constructor( ) : CoroutineScope { private val job = Job() - /** - * When the app comes to foreground both `queueUploadFromAllSites` and `queueUploadFromSite` are invoked. - * The problem is that they can run in parallel and `uploadServiceFacade.isPostUploadingOrQueued(it)` might return - * out-of-date result and a same post is added twice. - */ - private val mutex = Mutex() - override val coroutineContext: CoroutineContext get() = job + bgDispatcher /** @@ -141,40 +133,41 @@ class UploadStarter @Inject constructor( /** * This is meant to be used by [checkConnectionAndUpload] only. + * + * The method needs to be synchronized from the following reasons. When the app comes to foreground both + * `queueUploadFromAllSites` and `queueUploadFromSite` are invoked. The problem is that they can run in parallel + * and `uploadServiceFacade.isPostUploadingOrQueued(it)` might return out-of-date result and a same post is added + * twice. */ + @Synchronized private suspend fun upload(site: SiteModel) = coroutineScope { - try { - mutex.lock() - postStore.getPostsWithLocalChanges(site) - .asSequence() - .map { post -> - val action = uploadActionUseCase.getAutoUploadAction(post, site) - Pair(post, action) - } - .filter { (_, action) -> - action != DO_NOTHING - } - .toList() - .forEach { (post, action) -> - trackAutoUploadAction(action, post.status) - AppLog.d( - AppLog.T.POSTS, - "UploadStarter for post title: ${post.title}, action: $action" - ) - dispatcher.dispatch( - UploadActionBuilder.newIncrementNumberOfAutoUploadAttemptsAction( - post - ) - ) - uploadServiceFacade.uploadPost( - context = context, - post = post, - trackAnalytics = false - ) - } - } finally { - mutex.unlock() - } + postStore.getPostsWithLocalChanges(site) + .asSequence() + .map { post -> + val action = uploadActionUseCase.getAutoUploadAction(post, site) + Pair(post, action) + } + .filter { (_, action) -> + action != DO_NOTHING + } + .toList() + .forEach { (post, action) -> + trackAutoUploadAction(action, post.status) + AppLog.d( + AppLog.T.POSTS, + "UploadStarter for post title: ${post.title}, action: $action" + ) + dispatcher.dispatch( + UploadActionBuilder.newIncrementNumberOfAutoUploadAttemptsAction( + post + ) + ) + uploadServiceFacade.uploadPost( + context = context, + post = post, + trackAnalytics = false + ) + } } private fun trackAutoUploadAction(action: UploadAction, status: String) {