aboutsummaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authornea <nea@nea.moe>2023-02-16 23:12:04 +0100
committernea <nea@nea.moe>2023-02-16 23:38:54 +0100
commitd7f875a0dc1f46402abc3462c2fbe357876773bb (patch)
tree39e9f6f4e5075ae1a70af6f20f04e8d658eff065
parent1d75ac44c20fafd9f834dc7c01066a85a74f89e7 (diff)
downloadNotEnoughUpdates-d7f875a0dc1f46402abc3462c2fbe357876773bb.tar.gz
NotEnoughUpdates-d7f875a0dc1f46402abc3462c2fbe357876773bb.tar.bz2
NotEnoughUpdates-d7f875a0dc1f46402abc3462c2fbe357876773bb.zip
ApiCache: Fix apparent NullPointerExceptionfix/apicache-nullpointer
`CacheResult` should in theory have either a `file != null` a `future != null` or be `disposed`. Apparently this invariant of `CacheResult` is either being violated somewhere, or the `synchronized` blocks arent as synchronized as id hoped they were. In fact, `dispose()` does not even delete the file, so i can really only see this happening because the first `synchronized` block that writes the file and the second `synchronized` block that reads from the file hold the same lock. I have no idea how this would happen, but hopefully this fixes it (since the dispose didn't have a threading issue reported so far, i feel more confident leaving the .deleteOnExit in there, but I'm also wrapping any potential IOExceptions during access, because I am just so confused how the internal state was broken.
-rw-r--r--src/main/kotlin/io/github/moulberry/notenoughupdates/util/ApiCache.kt52
-rw-r--r--src/main/kotlin/io/github/moulberry/notenoughupdates/util/kotlin/completablefuture.kt33
2 files changed, 60 insertions, 25 deletions
diff --git a/src/main/kotlin/io/github/moulberry/notenoughupdates/util/ApiCache.kt b/src/main/kotlin/io/github/moulberry/notenoughupdates/util/ApiCache.kt
index c14df425..59fc2dd5 100644
--- a/src/main/kotlin/io/github/moulberry/notenoughupdates/util/ApiCache.kt
+++ b/src/main/kotlin/io/github/moulberry/notenoughupdates/util/ApiCache.kt
@@ -22,6 +22,7 @@ package io.github.moulberry.notenoughupdates.util
import io.github.moulberry.notenoughupdates.NotEnoughUpdates
import io.github.moulberry.notenoughupdates.options.customtypes.NEUDebugFlag
import io.github.moulberry.notenoughupdates.util.ApiUtil.Request
+import io.github.moulberry.notenoughupdates.util.kotlin.supplyImmediate
import org.apache.http.NameValuePair
import java.nio.file.Files
import java.nio.file.Path
@@ -47,43 +48,45 @@ object ApiCache {
val shouldGunzip: Boolean,
)
- data class CacheResult(
- private var future: CompletableFuture<String>?,
+ data class CacheResult internal constructor(
+ var cacheState: CacheState,
val firedAt: TimeSource.Monotonic.ValueTimeMark,
- private var file: Path? = null,
- private var disposed: Boolean = false,
) {
- init {
- future!!.thenAcceptAsync { text ->
+ constructor(future: CompletableFuture<String>, firedAt: TimeSource.Monotonic.ValueTimeMark) : this(
+ CacheState.WaitingForFuture(future),
+ firedAt
+ ) {
+ future.thenAccept { text ->
synchronized(this) {
- if (disposed) {
- return@synchronized
- }
- future = null
val f = Files.createTempFile(cacheBaseDir, "api-cache", ".bin")
log("Writing cache to disk: $f")
f.toFile().deleteOnExit()
f.writeText(text)
- file = f
+ cacheState = CacheState.FileCached(f)
}
}
}
- val isAvailable get() = file != null && !disposed
+ sealed interface CacheState {
+ object Disposed : CacheState
+ data class WaitingForFuture(val future: CompletableFuture<String>) : CacheState
+ data class FileCached(val file: Path) : CacheState
+ }
+
+ val isAvailable get() = cacheState is CacheState.FileCached
fun getCachedFuture(): CompletableFuture<String> {
synchronized(this) {
- if (disposed) {
- return CompletableFuture.supplyAsync {
- throw IllegalStateException("Attempting to read from a disposed future at $file. Most likely caused by non synchronized access to ApiCache.cachedRequests")
+ return when (val cs = cacheState) {
+ CacheState.Disposed -> supplyImmediate {
+ throw IllegalStateException("Attempting to read from a disposed future. Most likely caused by non synchronized access to ApiCache.cachedRequests")
}
- }
- val fut = future
- if (fut != null) {
- return fut
- } else {
- val text = file!!.readText()
- return CompletableFuture.completedFuture(text)
+
+ is CacheState.FileCached -> supplyImmediate {
+ cs.file.readText()
+ }
+
+ is CacheState.WaitingForFuture -> cs.future
}
}
}
@@ -96,11 +99,10 @@ object ApiCache {
*/
internal fun dispose() {
synchronized(this) {
- if (disposed) return
+ val file = (cacheState as? CacheState.FileCached)?.file
log("Disposing cache for $file")
- disposed = true
+ cacheState = CacheState.Disposed
file?.deleteIfExists()
- future = null
}
}
}
diff --git a/src/main/kotlin/io/github/moulberry/notenoughupdates/util/kotlin/completablefuture.kt b/src/main/kotlin/io/github/moulberry/notenoughupdates/util/kotlin/completablefuture.kt
new file mode 100644
index 00000000..de45c1e3
--- /dev/null
+++ b/src/main/kotlin/io/github/moulberry/notenoughupdates/util/kotlin/completablefuture.kt
@@ -0,0 +1,33 @@
+/*
+ * Copyright (C) 2023 NotEnoughUpdates contributors
+ *
+ * This file is part of NotEnoughUpdates.
+ *
+ * NotEnoughUpdates is free software: you can redistribute it
+ * and/or modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation, either
+ * version 3 of the License, or (at your option) any later version.
+ *
+ * NotEnoughUpdates is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public License
+ * along with NotEnoughUpdates. If not, see <https://www.gnu.org/licenses/>.
+ */
+
+package io.github.moulberry.notenoughupdates.util.kotlin
+
+import java.util.concurrent.CompletableFuture
+
+inline fun <R> supplyImmediate(block: () -> R): CompletableFuture<R> {
+ val cf = CompletableFuture<R>()
+ try {
+ cf.complete(block())
+ } catch (t: Throwable) {
+ cf.completeExceptionally(t)
+ }
+ return cf
+}
+