Fix crash in UploadStarter on Samsung devices with Android 5

wordpress-mobile · Nov 29, 2019 · d5b9948 · d5b9948
1 parent c2a864e
commit d5b9948
Showing 1 changed file with 33 additions and 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
@@ -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) {