Skip to content

Commit

Permalink
Catch some attempts to access UI inside withCol, and col within UI th…
Browse files Browse the repository at this point in the history
…read

If detected, shows an IDE lint.

- Catches calls to col.* inside AnkiActivity contexts.
- Catches calls to methods of classes derived from AnkiActivity, eg

```
withCol {
  someDeckPickerMethod()
}
```

- UI classes that don't inherit from AnkiActivity are not caught unless
they are also annotated with @UiThread. Eg SetNotetypeListener in
NoteEditor references col, but is not caught without an extra annotation.

It might be best to wait until 2.16 is out (and any quick follow-up
releases to address the inevitable regressions) before we start addressing
these issues in bulk; for now the code might be better left as-is unless
it's to address a known bug someone has reported.

Related: #13944, #13942, #13936
  • Loading branch information
dae authored and BrayanDSO committed Jun 4, 2023
1 parent 52a3ea9 commit 043ded7
Show file tree
Hide file tree
Showing 13 changed files with 27 additions and 8 deletions.
2 changes: 2 additions & 0 deletions AnkiDroid/src/main/java/com/ichi2/anki/AnkiActivity.kt
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ import android.widget.ProgressBar
import androidx.activity.result.ActivityResultLauncher
import androidx.annotation.LayoutRes
import androidx.annotation.StringRes
import androidx.annotation.UiThread
import androidx.appcompat.app.ActionBar
import androidx.appcompat.app.AppCompatActivity
import androidx.appcompat.app.AppCompatDelegate
Expand Down Expand Up @@ -59,6 +60,7 @@ import com.ichi2.utils.KotlinCleanup
import com.ichi2.utils.SyncStatus
import timber.log.Timber

@UiThread
open class AnkiActivity : AppCompatActivity, SimpleMessageDialogListener, CollectionGetter {

/** The name of the parent class (example: 'Reviewer') */
Expand Down
7 changes: 4 additions & 3 deletions AnkiDroid/src/main/java/com/ichi2/anki/CollectionManager.kt
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ package com.ichi2.anki

import android.annotation.SuppressLint
import androidx.annotation.VisibleForTesting
import androidx.annotation.WorkerThread
import anki.backend.backendError
import com.ichi2.libanki.Collection
import com.ichi2.libanki.CollectionV16
Expand Down Expand Up @@ -78,7 +79,7 @@ object CollectionManager {
*
* context(Queue) suspend fun canOnlyBeRunInWithQueue()
*/
private suspend fun<T> withQueue(block: CollectionManager.() -> T): T {
private suspend fun<T> withQueue(@WorkerThread block: CollectionManager.() -> T): T {
return withContext(queue) {
this@CollectionManager.block()
}
Expand All @@ -91,7 +92,7 @@ object CollectionManager {
* sure the collection won't be closed or modified by another thread. This guarantee
* does not hold if legacy code calls [getColUnsafe].
*/
suspend fun <T> withCol(block: Collection.() -> T): T {
suspend fun <T> withCol(@WorkerThread block: Collection.() -> T): T {
return withQueue {
ensureOpenInner()
block(collection!!)
Expand All @@ -105,7 +106,7 @@ object CollectionManager {
* these two cases, it should wrap the return value of the block in a class (eg Optional),
* instead of returning a nullable object.
*/
suspend fun<T> withOpenColOrNull(block: Collection.() -> T): T? {
suspend fun<T> withOpenColOrNull(@WorkerThread block: Collection.() -> T): T? {
return withQueue {
if (collection != null && !collection!!.dbClosed) {
block(collection!!)
Expand Down
5 changes: 1 addition & 4 deletions AnkiDroid/src/main/java/com/ichi2/anki/NoteEditor.kt
Original file line number Diff line number Diff line change
Expand Up @@ -36,10 +36,7 @@ import android.view.View.OnFocusChangeListener
import android.view.ViewGroup.MarginLayoutParams
import android.widget.*
import android.widget.AdapterView.OnItemSelectedListener
import androidx.annotation.CheckResult
import androidx.annotation.RequiresApi
import androidx.annotation.StringRes
import androidx.annotation.VisibleForTesting
import androidx.annotation.*
import androidx.appcompat.app.AlertDialog
import androidx.appcompat.view.menu.MenuBuilder
import androidx.appcompat.widget.AppCompatButton
Expand Down
2 changes: 2 additions & 0 deletions AnkiDroid/src/main/java/com/ichi2/libanki/Collection.kt
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ import android.content.res.Resources
import android.database.sqlite.SQLiteDatabaseLockedException
import androidx.annotation.CheckResult
import androidx.annotation.VisibleForTesting
import androidx.annotation.WorkerThread
import anki.search.SearchNode
import anki.search.SearchNodeKt
import anki.search.searchNode
Expand Down Expand Up @@ -80,6 +81,7 @@ import kotlin.random.Random
@KotlinCleanup("TextUtils -> Kotlin isNotEmpty()")
@KotlinCleanup("inline function in init { } so we don't need to init `crt` etc... at the definition")
@KotlinCleanup("ids.size != 0")
@WorkerThread
open class Collection(
/**
* @return The context that created this Collection.
Expand Down
2 changes: 2 additions & 0 deletions AnkiDroid/src/main/java/com/ichi2/libanki/ConfigManager.kt
Original file line number Diff line number Diff line change
Expand Up @@ -17,9 +17,11 @@
package com.ichi2.libanki

import androidx.annotation.CheckResult
import androidx.annotation.WorkerThread
import org.json.JSONArray
import org.json.JSONObject

@WorkerThread
abstract class ConfigManager {
@CheckResult abstract fun has(key: String): Boolean

Expand Down
2 changes: 2 additions & 0 deletions AnkiDroid/src/main/java/com/ichi2/libanki/DB.kt
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ import android.content.Context
import android.database.Cursor
import android.database.SQLException
import android.database.sqlite.SQLiteDatabase
import androidx.annotation.WorkerThread
import androidx.sqlite.db.SupportSQLiteDatabase
import com.ichi2.anki.BuildConfig
import com.ichi2.anki.CollectionHelper
Expand All @@ -41,6 +42,7 @@ import timber.log.Timber
* or the Android framework), and provides some helpers on top.
*/
@KotlinCleanup("Improve documentation")
@WorkerThread
class DB(db: SupportSQLiteDatabase) {
/**
* The collection, which is actually an SQLite database.
Expand Down
2 changes: 2 additions & 0 deletions AnkiDroid/src/main/java/com/ichi2/libanki/DeckManager.kt
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ package com.ichi2.libanki

import androidx.annotation.CheckResult
import androidx.annotation.VisibleForTesting
import androidx.annotation.WorkerThread
import com.ichi2.anki.exception.ConfirmModSchemaException
import com.ichi2.libanki.backend.exception.DeckRenameException
import com.ichi2.utils.DeckComparator
Expand All @@ -27,6 +28,7 @@ import net.ankiweb.rsdroid.RustCleanup
import org.intellij.lang.annotations.Language
import java.util.*

@WorkerThread
abstract class DeckManager {

/*
Expand Down
2 changes: 2 additions & 0 deletions AnkiDroid/src/main/java/com/ichi2/libanki/Decks.kt
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ package com.ichi2.libanki
import android.content.ContentValues
import androidx.annotation.CheckResult
import androidx.annotation.VisibleForTesting
import androidx.annotation.WorkerThread
import com.ichi2.anki.CrashReportService
import com.ichi2.anki.exception.ConfirmModSchemaException
import com.ichi2.libanki.Consts.DECK_STD
Expand All @@ -47,6 +48,7 @@ import java.util.regex.Pattern
@KotlinCleanup("remove unused functions")
@KotlinCleanup("where ever possible replace ArrayList() with mutableListOf()")
@KotlinCleanup("nullability")
@WorkerThread
class Decks(private val col: Collection) : DeckManager() {
@get:RustCleanup("This exists in Rust as DecksDictProxy, but its usage is warned against")
@KotlinCleanup("lateinit")
Expand Down
2 changes: 2 additions & 0 deletions AnkiDroid/src/main/java/com/ichi2/libanki/Media.kt
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ import android.database.Cursor
import android.database.SQLException
import android.net.Uri
import androidx.annotation.VisibleForTesting
import androidx.annotation.WorkerThread
import com.ichi2.anki.CrashReportService
import com.ichi2.libanki.exception.EmptyMediaException
import com.ichi2.libanki.template.TemplateFilters
Expand Down Expand Up @@ -58,6 +59,7 @@ import kotlin.math.min
* E.g: new File(dir(), "filename.jpg")
*/
@KotlinCleanup("IDE Lint")
@WorkerThread
open class Media(private val col: Collection, server: Boolean) {
private var mDir: String?

Expand Down
2 changes: 2 additions & 0 deletions AnkiDroid/src/main/java/com/ichi2/libanki/ModelManager.kt
Original file line number Diff line number Diff line change
Expand Up @@ -16,13 +16,15 @@

package com.ichi2.libanki

import androidx.annotation.WorkerThread
import com.ichi2.anki.CrashReportService
import com.ichi2.anki.exception.ConfirmModSchemaException
import com.ichi2.utils.Assert
import net.ankiweb.rsdroid.RustCleanup
import org.json.JSONObject
import timber.log.Timber

@WorkerThread
abstract class ModelManager(protected val col: Collection) {

/*
Expand Down
2 changes: 2 additions & 0 deletions AnkiDroid/src/main/java/com/ichi2/libanki/Tags.kt
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
package com.ichi2.libanki

import android.content.ContentValues
import androidx.annotation.WorkerThread
import com.ichi2.libanki.backend.model.TagUsnTuple
import com.ichi2.libanki.utils.TimeManager
import org.json.JSONObject
Expand All @@ -35,6 +36,7 @@ import java.util.regex.Pattern
* instead of a JSONObject. It is much more convenient to work with a TreeMap in Java, but there
* may be a performance penalty in doing so (on startup and shutdown).
*/
@WorkerThread
class Tags
/**
* Registry save/load
Expand Down
3 changes: 3 additions & 0 deletions AnkiDroid/src/main/java/com/ichi2/libanki/sched/BaseSched.kt
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ import android.graphics.Typeface
import android.text.SpannableStringBuilder
import android.text.style.StyleSpan
import androidx.annotation.VisibleForTesting
import androidx.annotation.WorkerThread
import anki.ankidroid.schedTimingTodayLegacyRequest
import anki.decks.DeckTreeNode
import anki.scheduler.*
Expand All @@ -45,6 +46,8 @@ import net.ankiweb.rsdroid.RustCleanup
* BackendFactory.defaultLegacySchema is false, so for now, SchedV2
* will need to (conditionally) override them.
*/

@WorkerThread
abstract class BaseSched(val col: Collection) {
/** Update a V1 scheduler collection to V2. Requires full sync. */
fun upgradeToV2() {
Expand Down
2 changes: 1 addition & 1 deletion lint-release.xml
Original file line number Diff line number Diff line change
Expand Up @@ -330,7 +330,7 @@
<issue id="Range" severity="ignore" />
<issue id="ResourceType" severity="ignore" />
<issue id="RestrictedApi" severity="ignore" />
<issue id="WrongThread" severity="ignore" />
<issue id="WrongThread" severity="informational" />
<issue id="VisibleForTests" severity="ignore" />
<issue id="ProtectedPermissions" severity="ignore" />
<issue id="TextViewEdits" severity="ignore" />
Expand Down

0 comments on commit 043ded7

Please sign in to comment.