Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
23 changes: 16 additions & 7 deletions rsdroid/src/main/java/net/ankiweb/rsdroid/Backend.kt
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,9 @@ import org.json.JSONObject
import timber.log.Timber
import java.io.Closeable
import java.io.File
import java.util.concurrent.locks.ReentrantReadWriteLock
import kotlin.concurrent.read
import kotlin.concurrent.write

open class Backend(
langs: Iterable<String> = listOf("en"),
Expand All @@ -42,6 +45,11 @@ open class Backend(
// Set on init; unset on .close(). Access via withBackend()
private var backendPointer: Long? = null

/**
* AnkiDroid#21455): Ensures [close] cannot free the backend during a [runMethodRaw] call.
*/
private val backendLock = ReentrantReadWriteLock()

val tr: Translations by lazy {
Translations(this)
}
Expand Down Expand Up @@ -93,8 +101,10 @@ open class Backend(
override fun close() {
checkMainThreadOp()
Timber.d("Closing rust backend")
NativeMethods.closeBackend(backendPointer!!)
backendPointer = null
backendLock.write {
NativeMethods.closeBackend(backendPointer!!)
backendPointer = null
}
}

/**
Expand Down Expand Up @@ -138,12 +148,11 @@ open class Backend(
* Run the provided closure with access to the backend.
* @throws BackendException if backend closed.
*/
private fun <T> withBackend(fn: (ptr: Long) -> T): T {
if (backendPointer == null) {
throw BackendException("Backend has been closed")
private fun <T> withBackend(fn: (ptr: Long) -> T): T =
backendLock.read {
val pointer = backendPointer ?: throw BackendException("Backend has been closed")
fn(pointer)
}
return fn(backendPointer!!)
}

// other DB methods

Expand Down
73 changes: 73 additions & 0 deletions rsdroid/src/test/java/net/ankiweb/BackendCloseRaceTest.kt
Original file line number Diff line number Diff line change
@@ -0,0 +1,73 @@
// SPDX-License-Identifier: GPL-3.0-or-later
package net.ankiweb

import android.annotation.SuppressLint
import androidx.test.ext.junit.runners.AndroidJUnit4
import net.ankiweb.rsdroid.Backend
import net.ankiweb.rsdroid.BackendException
import net.ankiweb.rsdroid.BackendFactory.getBackend
import net.ankiweb.rsdroid.testing.RustBackendLoader.ensureSetup
import org.junit.Assert.assertTrue
import org.junit.Before
import org.junit.Test
import org.junit.runner.RunWith
import kotlin.concurrent.thread
import kotlin.time.Duration
import kotlin.time.Duration.Companion.milliseconds
import kotlin.time.Duration.Companion.minutes

/**
* Ensures [Backend.close] waits for in-flight calls.
*
* Caused a use-after-free which SIGSEGVs/SIGABRTed the JVM (exit code 134).
*
* https://github.com/ankidroid/Anki-Android/issues/21455
*/
@RunWith(AndroidJUnit4::class)
class BackendCloseRaceTest {
@Before
fun loadLibrary() {
ensureSetup()
}

@Test
fun closeDoesNotInterruptInFlightCalls() {
val backend = getBackend()
backend.openCollection(":memory:")
@SuppressLint("CheckResult")
backend.fullQuery(longQuery(rows = 1_000), null) // warm up the query path

var queryError: Exception? = null
val queryThread =
thread(name = "backend-slow-query") {
try {
// keeps the backend busy inside a single native call for over a second
backend.fullQuery(longQuery(rows = 50_000_000), null)
} catch (e: Exception) {
queryError = e
}
}

sleep(500.milliseconds) // let the query enter native code
backend.close()
queryThread.join(1.minutes)

// Acceptable outcomes:
// * close() waited for the in-flight call: the query succeeds.
// * the call lost the race and was cleanly rejected.
if (queryError != null) {
assertTrue("unexpected query error: $queryError", queryError is BackendException)
}
}

/**
* SQLite has no sleep(), so counting a generated series is a simple busy-loop.
*/
private fun longQuery(rows: Int) =
"WITH RECURSIVE c(x) AS (VALUES(1) UNION ALL SELECT x+1 FROM c WHERE x < $rows) " +
"SELECT count(*) FROM c"
}

private fun sleep(duration: Duration) = Thread.sleep(duration.inWholeMilliseconds)

private fun Thread.join(timeout: Duration) = join(timeout.inWholeMilliseconds)
Loading