diff options
author | Roman / Linnea Gräf <roman.graef@gmail.com> | 2023-02-18 15:24:52 +0100 |
---|---|---|
committer | GitHub <noreply@github.com> | 2023-02-18 15:24:52 +0100 |
commit | 0d281d483909d71272783033b2aba8f33dcbce36 (patch) | |
tree | e59f05fe4b22f989b5a600e66d7b8c9aaecda37c /src/main/kotlin/io | |
parent | b12da67a08b6808363c0ad130fe8c3bc6f00f7b2 (diff) | |
download | NotEnoughUpdates-0d281d483909d71272783033b2aba8f33dcbce36.tar.gz NotEnoughUpdates-0d281d483909d71272783033b2aba8f33dcbce36.tar.bz2 NotEnoughUpdates-0d281d483909d71272783033b2aba8f33dcbce36.zip |
ApiCache: Fix apparent NullPointerException (#619)
`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.
Diffstat (limited to 'src/main/kotlin/io')
-rw-r--r-- | src/main/kotlin/io/github/moulberry/notenoughupdates/util/ApiCache.kt | 52 | ||||
-rw-r--r-- | src/main/kotlin/io/github/moulberry/notenoughupdates/util/kotlin/completablefuture.kt | 33 |
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 +} + |