Skip to content

Commit

Permalink
Fix full download crashing in new schema case
Browse files Browse the repository at this point in the history
This worked in my initial testing, but with a smaller collection, the
UI is now racing with the download on my machine:

- getSyncStatus() was assuming col.db would still be open by the time
it gets around to calling methods on it
- onCreateOptionsMenu() likewise assumes that if colIsOpen() returns true,
it is free to call methods on it

A better way way to fix this would be to wrap all collection access in a mutex,
so that the collection is never touched without the lock being acquired.
Code holding the mutex could then be guaranteed that the collection is open
and valid for the duration of its run. Changing all that legacy code is
a big task however.

I've added a lock to the upload case as well, as it's possible it's
likewise buggy and just not triggering on my machine.
  • Loading branch information
dae authored and mikehardy committed Jul 22, 2022
1 parent b2b0ad5 commit 2d5370f
Show file tree
Hide file tree
Showing 3 changed files with 18 additions and 11 deletions.
2 changes: 1 addition & 1 deletion AnkiDroid/src/main/java/com/ichi2/anki/DeckPicker.kt
Original file line number Diff line number Diff line change
Expand Up @@ -619,7 +619,7 @@ open class DeckPicker :
return true
}
})
if (colIsOpen()) {
if (colIsOpen() && !CollectionHelper.getInstance().isCollectionLocked) {
displaySyncBadge(menu)

// Show / hide undo
Expand Down
18 changes: 12 additions & 6 deletions AnkiDroid/src/main/java/com/ichi2/anki/Sync.kt
Original file line number Diff line number Diff line change
Expand Up @@ -166,16 +166,19 @@ private suspend fun handleDownload(
extractProgress = fullDownloadProgress(col.tr.syncDownloadingFromAnkiweb()),
onCancel = ::cancelSync
) {
col.createBackup(
BackupManager.getBackupDirectoryFromCollection(col.path),
force = true,
waitForCompletion = true
)
col.close(save = true, downgrade = false, forFullSync = true)
val helper = CollectionHelper.getInstance()
helper.lockCollection()
try {
col.createBackup(
BackupManager.getBackupDirectoryFromCollection(col.path),
force = true,
waitForCompletion = true
)
col.close(save = true, downgrade = false, forFullSync = true)
col.fullDownload(auth)
} finally {
col.reopen(afterFullSync = true)
helper.unlockCollection()
}
}

Expand All @@ -193,11 +196,14 @@ private suspend fun handleUpload(
extractProgress = fullDownloadProgress(col.tr.syncUploadingToAnkiweb()),
onCancel = ::cancelSync
) {
val helper = CollectionHelper.getInstance()
helper.lockCollection()
col.close(save = true, downgrade = false, forFullSync = true)
try {
col.fullUpload(auth)
} finally {
col.reopen(afterFullSync = true)
helper.unlockCollection()
}
}
Timber.i("Full Upload Completed")
Expand Down
9 changes: 5 additions & 4 deletions AnkiDroid/src/main/java/com/ichi2/utils/SyncStatus.kt
Original file line number Diff line number Diff line change
Expand Up @@ -30,14 +30,15 @@ enum class SyncStatus {

@JvmStatic
fun getSyncStatus(getCol: Supplier<Collection>): SyncStatus {
val col: Collection
col = try {
getCol.get()
return try {
val col = getCol.get()
// may fail when the collection is closed for a full sync,
// as col.db is null
getSyncStatus(col)
} catch (e: Exception) {
Timber.w(e)
return INCONCLUSIVE
}
return getSyncStatus(col)
}

@JvmStatic
Expand Down

0 comments on commit 2d5370f

Please sign in to comment.