Skip to content

Commit 27f29c2

Browse files
alperozturk96alperozturk96
authored
Merge pull request #17392 from nextcloud/backport/17373/stable-34.1.x
[stable-34.1.x] fix(upload): prevent crash and duplicate retries when retrying failed uploads
2 parents 0a563f8 + 01f3668 commit 27f29c2

7 files changed

Lines changed: 231 additions & 42 deletions

File tree

‎app/src/main/java/com/nextcloud/client/account/UserAccountManager.java‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@
99
import android.accounts.Account;
1010
import android.accounts.AccountManager;
1111
import android.app.Activity;
12+
import android.content.Context;
1213
import android.content.Intent;
1314

1415
import com.owncloud.android.MainApp;
@@ -30,6 +31,9 @@ public interface UserAccountManager extends CurrentAccountProvider {
3031
String ACCOUNT_USES_STANDARD_PASSWORD = "ACCOUNT_USES_STANDARD_PASSWORD";
3132
String PENDING_FOR_REMOVAL = "PENDING_FOR_REMOVAL";
3233

34+
@Nullable
35+
Context getContext();
36+
3337
@Nullable
3438
OwnCloudAccount getCurrentOwnCloudAccount();
3539

‎app/src/main/java/com/nextcloud/client/account/UserAccountManagerImpl.java‎

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -55,7 +55,7 @@ public class UserAccountManagerImpl implements UserAccountManager {
5555
private static final String TAG = UserAccountManagerImpl.class.getSimpleName();
5656
private static final String PREF_SELECT_OC_ACCOUNT = "select_oc_account";
5757

58-
private Context context;
58+
private final Context context;
5959
private final AccountManager accountManager;
6060

6161
public static UserAccountManagerImpl fromContext(Context context) {
@@ -305,6 +305,12 @@ public User getAnonymousUser() {
305305
return AnonymousUser.fromContext(context);
306306
}
307307

308+
@Nullable
309+
@Override
310+
public Context getContext() {
311+
return context;
312+
}
313+
308314
@Override
309315
@Nullable
310316
public OwnCloudAccount getCurrentOwnCloudAccount() {

‎app/src/main/java/com/nextcloud/client/di/DispatcherModule.kt‎

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,17 +1,23 @@
11
/*
22
* Nextcloud - Android Client
33
*
4+
* SPDX-FileCopyrightText: 2026 Alper Ozturk <alper.ozturk@nextcloud.com>
45
* SPDX-FileCopyrightText: 2022 Álvaro Brey <alvaro@alvarobrey.com>
56
* SPDX-FileCopyrightText: 2022 Nextcloud GmbH
67
* SPDX-License-Identifier: AGPL-3.0-or-later OR GPL-2.0-only
78
*/
89
package com.nextcloud.client.di
910

11+
import com.owncloud.android.lib.common.utils.Log_OC
1012
import dagger.Module
1113
import dagger.Provides
1214
import kotlinx.coroutines.CoroutineDispatcher
15+
import kotlinx.coroutines.CoroutineExceptionHandler
16+
import kotlinx.coroutines.CoroutineScope
1317
import kotlinx.coroutines.Dispatchers
18+
import kotlinx.coroutines.SupervisorJob
1419
import javax.inject.Qualifier
20+
import javax.inject.Singleton
1521

1622
@Retention(AnnotationRetention.BINARY)
1723
@Qualifier
@@ -25,8 +31,15 @@ annotation class IoDispatcher
2531
@Qualifier
2632
annotation class MainDispatcher
2733

34+
@Retention(AnnotationRetention.BINARY)
35+
@Qualifier
36+
annotation class ApplicationScope
37+
2838
@Module
2939
object DispatcherModule {
40+
41+
private const val APPLICATION_SCOPE_TAG = "ApplicationScope"
42+
3043
@DefaultDispatcher
3144
@Provides
3245
fun provideDefaultDispatcher(): CoroutineDispatcher = Dispatchers.Default
@@ -38,4 +51,18 @@ object DispatcherModule {
3851
@MainDispatcher
3952
@Provides
4053
fun provideMainDispatcher(): CoroutineDispatcher = Dispatchers.Main
54+
55+
/**
56+
* A process-lifetime [CoroutineScope] for singletons that outlive any single Android component.
57+
*/
58+
@ApplicationScope
59+
@Provides
60+
@Singleton
61+
fun provideApplicationScope(@IoDispatcher dispatcher: CoroutineDispatcher): CoroutineScope = CoroutineScope(
62+
SupervisorJob() +
63+
dispatcher +
64+
CoroutineExceptionHandler { _, throwable ->
65+
Log_OC.e(APPLICATION_SCOPE_TAG, "Uncaught exception in application coroutine scope", throwable)
66+
}
67+
)
4168
}

‎app/src/main/java/com/nextcloud/client/jobs/upload/FileUploadHelper.kt‎

Lines changed: 26 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -19,13 +19,14 @@ import com.nextcloud.client.database.entity.toOCUpload
1919
import com.nextcloud.client.database.entity.toUploadEntity
2020
import com.nextcloud.client.device.BatteryStatus
2121
import com.nextcloud.client.device.PowerManagementService
22+
import com.nextcloud.client.di.ApplicationScope
2223
import com.nextcloud.client.jobs.BackgroundJobManager
2324
import com.nextcloud.client.network.Connectivity
2425
import com.nextcloud.client.network.ConnectivityService
2526
import com.nextcloud.client.notifications.AppWideNotificationManager
2627
import com.nextcloud.utils.extensions.checkWCFRestrictions
28+
import com.nextcloud.utils.extensions.createOwncloudClient
2729
import com.nextcloud.utils.extensions.getUploadIds
28-
import com.nextcloud.utils.extensions.isAnonymous
2930
import com.nextcloud.utils.extensions.isLastResultConflictError
3031
import com.nextcloud.utils.extensions.isSame
3132
import com.owncloud.android.MainApp
@@ -38,7 +39,6 @@ import com.owncloud.android.db.OCUpload
3839
import com.owncloud.android.db.UploadResult
3940
import com.owncloud.android.files.services.NameCollisionPolicy
4041
import com.owncloud.android.lib.common.OwnCloudClient
41-
import com.owncloud.android.lib.common.OwnCloudClientFactory
4242
import com.owncloud.android.lib.common.network.OnDatatransferProgressListener
4343
import com.owncloud.android.lib.common.operations.RemoteOperationResult
4444
import com.owncloud.android.lib.common.utils.Log_OC
@@ -56,7 +56,7 @@ import kotlinx.coroutines.Dispatchers
5656
import kotlinx.coroutines.launch
5757
import kotlinx.coroutines.withContext
5858
import java.io.File
59-
import java.util.concurrent.Semaphore
59+
import java.util.concurrent.atomic.AtomicBoolean
6060
import javax.inject.Inject
6161

6262
@Suppress("TooManyFunctions")
@@ -74,26 +74,28 @@ class FileUploadHelper {
7474
@Inject
7575
lateinit var fileStorageManager: FileDataStorageManager
7676

77-
private val ioScope = CoroutineScope(Dispatchers.IO)
77+
@Inject
78+
@ApplicationScope
79+
lateinit var appScope: CoroutineScope
7880

7981
init {
8082
MainApp.getAppComponent().inject(this)
8183
}
8284

85+
private val uploadActionHandler = UploadListAdapterActionHandler()
86+
8387
companion object {
8488
private val TAG = FileUploadWorker::class.java.simpleName
8589

8690
const val MAX_FILE_COUNT = 500
8791

8892
val mBoundListeners = HashMap<String, OnDatatransferProgressListener>()
8993

90-
private var instance: FileUploadHelper? = null
94+
private val retryInProgress = AtomicBoolean(false)
9195

92-
private val retryFailedUploadsSemaphore = Semaphore(1)
96+
private val sharedInstance: FileUploadHelper by lazy { FileUploadHelper() }
9397

94-
fun instance(): FileUploadHelper = instance ?: synchronized(this) {
95-
instance ?: FileUploadHelper().also { instance = it }
96-
}
98+
fun instance(): FileUploadHelper = sharedInstance
9799

98100
fun buildRemoteName(accountName: String, remotePath: String): String = accountName + remotePath
99101
}
@@ -122,21 +124,17 @@ class FileUploadHelper {
122124
connectivityService: ConnectivityService,
123125
accountManager: UserAccountManager,
124126
powerManagementService: PowerManagementService
125-
): Boolean {
126-
if (!retryFailedUploadsSemaphore.tryAcquire()) {
127+
) {
128+
if (!retryInProgress.compareAndSet(false, true)) {
127129
Log_OC.d(TAG, "skipping retryFailedUploads, already running")
128-
return true
130+
return
129131
}
130132

131-
var isUploadStarted = false
132133
val capability = fileStorageManager.getCapability(accountManager.user)
133134

134-
try {
135-
ioScope.launch {
135+
appScope.launch {
136+
try {
136137
val uploads = getUploadsByStatus(null, UploadStatus.UPLOAD_FAILED, capability)
137-
if (uploads.isNotEmpty()) {
138-
isUploadStarted = true
139-
}
140138

141139
retryUploads(
142140
uploadsStorageManager,
@@ -145,12 +143,12 @@ class FileUploadHelper {
145143
powerManagementService,
146144
uploads
147145
)
146+
} finally {
147+
// Reset only after retry processing has completely finished so the guard covers
148+
// coroutine execution, not just its launch. This keeps a single retry running at a time.
149+
retryInProgress.set(false)
148150
}
149-
} finally {
150-
retryFailedUploadsSemaphore.release()
151151
}
152-
153-
return isUploadStarted
154152
}
155153

156154
suspend fun retryCancelledUploads(
@@ -185,21 +183,13 @@ class FileUploadHelper {
185183
val batteryStatus = powerManagementService.battery
186184

187185
val uploadsToRetry = mutableListOf<Long>()
188-
189-
val currentAccount = accountManager.currentAccount
190-
val context = MainApp.getAppContext()
191-
var ownCloudClient: OwnCloudClient? = null
192-
if (!currentAccount.isAnonymous(context)) {
193-
ownCloudClient =
194-
OwnCloudClientFactory.createOwnCloudClient(accountManager.currentAccount, MainApp.getAppContext())
195-
}
196-
val uploadActionHandler = UploadListAdapterActionHandler()
186+
val client = accountManager.createOwncloudClient()
197187

198188
for (upload in uploads) {
199189
if (upload.isLastResultConflictError()) {
200-
ownCloudClient?.let {
190+
client?.let {
201191
conflictHandlingResult =
202-
uploadActionHandler.handleConflict(upload, ownCloudClient, uploadsStorageManager)
192+
uploadActionHandler.handleConflict(upload, client = it, uploadsStorageManager)
203193
}
204194
continue
205195
}
@@ -214,7 +204,7 @@ class FileUploadHelper {
214204

215205
if (uploadResult != UploadResult.UPLOADED) {
216206
if (upload.lastResult != uploadResult) {
217-
// Setting Upload status else cancelled uploads will behave wrong, when retrying
207+
// Setting Upload status else canceled uploads will behave wrong, when retrying
218208
// Needs to happen first since lastResult wil be overwritten by setter
219209
upload.uploadStatus = UploadStatus.UPLOAD_FAILED
220210

@@ -345,7 +335,7 @@ class FileUploadHelper {
345335
status: UploadStatus,
346336
onCompleted: () -> Unit = {}
347337
) {
348-
ioScope.launch {
338+
appScope.launch {
349339
uploadsStorageManager.uploadDao.updateStatus(remotePath, accountName, status.value)
350340
onCompleted()
351341
}
@@ -531,7 +521,7 @@ class FileUploadHelper {
531521
* @param user Needed for creating client
532522
*/
533523
fun removeDuplicatedFile(duplicatedFile: OCFile, client: OwnCloudClient, user: User, onCompleted: () -> Unit) {
534-
ioScope.launch {
524+
appScope.launch {
535525
val removeFileOperation = RemoveFileOperation(
536526
duplicatedFile,
537527
false,

‎app/src/main/java/com/nextcloud/utils/extensions/UserAccountManagerExtensions.kt‎

Lines changed: 46 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,46 @@
1+
/*
2+
* Nextcloud - Android Client
3+
*
4+
* SPDX-FileCopyrightText: 2026 Alper Ozturk <alper.ozturk@nextcloud.com>
5+
* SPDX-License-Identifier: AGPL-3.0-or-later
6+
*/
7+
8+
package com.nextcloud.utils.extensions
9+
10+
import com.nextcloud.client.account.UserAccountManager
11+
import com.owncloud.android.MainApp
12+
import com.owncloud.android.lib.common.OwnCloudClient
13+
import com.owncloud.android.lib.common.OwnCloudClientFactory
14+
import com.owncloud.android.lib.common.accounts.AccountUtils
15+
import com.owncloud.android.lib.common.utils.Log_OC
16+
17+
private const val TAG = "UserAccountManagerExtensions"
18+
19+
fun UserAccountManager.createOwncloudClient(): OwnCloudClient? = createOwncloudClient(currentAccount.name)
20+
21+
@Suppress("TooGenericExceptionCaught", "ReturnCount", "DEPRECATION")
22+
fun UserAccountManager.createOwncloudClient(accountName: String): OwnCloudClient? {
23+
val context = context ?: MainApp.getAppContext()
24+
if (context == null) {
25+
Log_OC.e(TAG, "app context is null, cannot create client")
26+
return null
27+
}
28+
29+
val user = getUser(accountName).orElse(null)
30+
if (user == null || user.isAnonymous) {
31+
Log_OC.e(TAG, "account is not registered, cannot create client for: $accountName")
32+
return null
33+
}
34+
35+
return try {
36+
val result = OwnCloudClientFactory.createOwnCloudClient(user.toPlatformAccount(), context)
37+
Log_OC.i(TAG, "client created")
38+
result
39+
} catch (e: AccountUtils.AccountNotFoundException) {
40+
Log_OC.e(TAG, "account removed while creating client for: $accountName", e)
41+
null
42+
} catch (e: Exception) {
43+
Log_OC.e(TAG, "cannot create client: ", e)
44+
null
45+
}
46+
}

‎app/src/main/java/com/owncloud/android/ui/activity/UploadListActivity.kt‎

Lines changed: 1 addition & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -168,16 +168,12 @@ class UploadListActivity :
168168
}
169169

170170
private fun refresh() {
171-
val isUploadStarted = FileUploadHelper.instance().retryFailedUploads(
171+
FileUploadHelper.instance().retryFailedUploads(
172172
uploadsStorageManager,
173173
connectivityService,
174174
accountManager,
175175
powerManagementService
176176
)
177-
178-
if (!isUploadStarted) {
179-
uploadListAdapter.loadUploadItemsFromDb { swipeListRefreshLayout?.isRefreshing = false }
180-
}
181177
}
182178

183179
override fun onStart() {

0 commit comments

Comments
 (0)