diff --git a/kstore-file/api/desktop/kstore-file.api b/kstore-file/api/desktop/kstore-file.api index 62b9766..c33dafd 100644 --- a/kstore-file/api/desktop/kstore-file.api +++ b/kstore-file/api/desktop/kstore-file.api @@ -4,6 +4,10 @@ public final class io/github/xxfast/kstore/file/FileCodec : io/github/xxfast/kst public fun encode (Ljava/lang/Object;Lkotlin/coroutines/Continuation;)Ljava/lang/Object; } +public final class io/github/xxfast/kstore/file/FileCodecKt { + public static final fun uniqueTempFile (Lkotlinx/io/files/Path;)Lkotlinx/io/files/Path; +} + public final class io/github/xxfast/kstore/file/extensions/KVersionedStoreKt { public static final fun DefaultMigration (Ljava/lang/Object;)Lkotlin/jvm/functions/Function2; } diff --git a/kstore-file/src/commonMain/kotlin/io/github/xxfast/kstore/file/FileCodec.kt b/kstore-file/src/commonMain/kotlin/io/github/xxfast/kstore/file/FileCodec.kt index 3aff99e..a24baa5 100644 --- a/kstore-file/src/commonMain/kotlin/io/github/xxfast/kstore/file/FileCodec.kt +++ b/kstore-file/src/commonMain/kotlin/io/github/xxfast/kstore/file/FileCodec.kt @@ -3,6 +3,7 @@ package io.github.xxfast.kstore.file import io.github.xxfast.kstore.Codec import io.github.xxfast.kstore.DefaultJson +import kotlin.uuid.Uuid import kotlinx.io.buffered import kotlinx.io.files.FileNotFoundException import kotlinx.io.files.Path @@ -19,12 +20,14 @@ import kotlinx.serialization.json.io.encodeToSink as encode /** * Creates a store with [FileCodec] with json serializer * @param file path to the file that is managed by this store + * @param tempFile staging file this codec writes through. Defaults to a path unique to this codec, + * so that codecs sharing a [file] never stage into the same buffer - see [uniqueTempFile] * @param json JSON Serializer to use. defaults to [DefaultJson] * @return store that contains a value of type [T] */ public inline fun FileCodec( file: Path, - tempFile: Path = Path("$file.temp"), + tempFile: Path = uniqueTempFile(file), json: Json = DefaultJson, ): FileCodec = FileCodec( file = file, @@ -79,6 +82,30 @@ public class FileCodec( } } +/** + * A staging path unique to a single codec, of the form `..temp`. + * + * Stores pointing at the same [file] each get their own staging file. Sharing one lets concurrent + * writes interleave into a single buffer before either is moved into place, and whichever moves + * last publishes the mixture, which is how a store ends up corrupt on disk (see issue #85). With a + * staging file each, the move stays atomic and the worst case is last write wins. + * + * Note this is one staging file per codec, not per write, so a process killed mid-write leaves at + * most one behind per store. The next write is unaffected either way. + * + * The suffix comes from [Uuid.random] rather than [kotlin.random.Random] on purpose. Random.Default + * is a PRNG seeded from the clock on some targets, so two processes cold starting in the same tick + * can draw the same sequence and land on the same staging path, which is the one case this is meant + * to prevent. Uuid.random draws from the platform's secure source and has no shared seed. + * + * @param file path to the file being staged for + * @return a staging path that no other codec will pick + */ +public fun uniqueTempFile(file: Path): Path { + val suffix: String = Uuid.random().toHexString() + return Path("$file.$suffix.temp") +} + /** * Moves [source] onto [destination] atomically via [atomicMove]. * diff --git a/kstore-file/src/commonMain/kotlin/io/github/xxfast/kstore/file/extensions/KVersionedStore.kt b/kstore-file/src/commonMain/kotlin/io/github/xxfast/kstore/file/extensions/KVersionedStore.kt index 10c1450..6f4f8ba 100644 --- a/kstore-file/src/commonMain/kotlin/io/github/xxfast/kstore/file/extensions/KVersionedStore.kt +++ b/kstore-file/src/commonMain/kotlin/io/github/xxfast/kstore/file/extensions/KVersionedStore.kt @@ -4,6 +4,7 @@ import io.github.xxfast.kstore.Codec import io.github.xxfast.kstore.DefaultJson import io.github.xxfast.kstore.KStore import io.github.xxfast.kstore.file.moveOrCopy +import io.github.xxfast.kstore.file.uniqueTempFile import io.github.xxfast.kstore.storeOf import kotlinx.io.buffered import kotlinx.io.files.FileNotFoundException @@ -61,8 +62,8 @@ public class VersionedCodec( private val serializer: KSerializer, private val migration: Migration, private val versionPath: Path = Path("$file.version"), // TODO: Save to file metadata instead - private val tempPath: Path = Path("$file.temp"), - private val tempVersionPath: Path = Path("$versionPath.temp"), + private val tempPath: Path = uniqueTempFile(file), + private val tempVersionPath: Path = uniqueTempFile(versionPath), ) : Codec { /** diff --git a/kstore-file/src/commonTest/kotlin/io/github/xxfast/kstore/file/FileCodecTests.kt b/kstore-file/src/commonTest/kotlin/io/github/xxfast/kstore/file/FileCodecTests.kt index e376b33..0a2bf17 100644 --- a/kstore-file/src/commonTest/kotlin/io/github/xxfast/kstore/file/FileCodecTests.kt +++ b/kstore-file/src/commonTest/kotlin/io/github/xxfast/kstore/file/FileCodecTests.kt @@ -35,10 +35,16 @@ class FileCodecTests { .use { DefaultJson.encodeToSink(value, it) } } + /** Staging files left beside the store, which should be none once a write settles */ + private fun stagingFiles(): List = + SystemFileSystem.list(Path(FILE_PATH).parent ?: Path(".")) + .filter { it.name.startsWith(Path(FILE_PATH).name) && it.name.endsWith(".temp") } + @AfterTest fun cleanUp() { SystemFileSystem.delete(Path(FILE_PATH), false) SystemFileSystem.delete(Path("$FILE_PATH.temp"), false) + stagingFiles().forEach { SystemFileSystem.delete(it, false) } } @Test @@ -83,6 +89,61 @@ class FileCodecTests { assertFailsWith { codec.decode() } } + // Codecs sharing a file must not share a staging file, or concurrent writes interleave into one + // buffer and whichever moves last publishes the mixture - see issue #85 + + @Test + fun testUniqueTempFileDiffersEveryCall() { + val file = Path(FILE_PATH) + val staging: Set = List(100) { uniqueTempFile(file) }.toSet() + assertEquals(100, staging.size) + } + + @Test + fun testUniqueTempFileStagesBesideTheTargetFile() { + // atomicMove is only atomic within one filesystem, so staging has to sit next to the target + val file = Path("some/dir/pets.json") + assertEquals(Path("some/dir").toString(), uniqueTempFile(file).parent.toString()) + } + + @Test + fun testCodecDoesNotStageThroughTheSharedPath() = runTest { + // The path every codec used to stage through. Stand in for another codec's in-flight write. + val shared = Path("$FILE_PATH.temp") + SystemFileSystem.sink(shared).buffered().use { it.writeString("another codec's staging") } + + codec.encode(listOf(MYLO)) + + // This codec staged somewhere of its own, so it neither read nor consumed the other write + assertEquals(listOf(MYLO), codec.decode()) + assertEquals(true, SystemFileSystem.exists(shared)) + } + + @Test + fun testSeparateCodecsOnSameFileDoNotShareStaging() = runTest { + val other: FileCodec> = FileCodec(file = Path(FILE_PATH)) + + codec.encode(listOf(MYLO)) + other.encode(listOf(OREO)) + + // Last write wins, and it is a whole document rather than a mixture of both + assertEquals(listOf(OREO), codec.decode()) + assertEquals(emptyList(), stagingFiles()) + } + + @Test + fun testFailedEncodeLeavesNoStagingBehind() = runTest { + val other: FileCodec> = FileCodec(file = Path(FILE_PATH)) + codec.encode(listOf(MYLO)) + + // KAT's serializer throws part way through, so this codec fails mid-write + assertFailsWith { other.encode(listOf(MYLO, KAT)) } + + // The failure cleans up after itself and leaves the other codec's file intact + assertEquals(listOf(MYLO), codec.decode()) + assertEquals(emptyList(), stagingFiles()) + } + @Test fun testMoveFallsBackToCopyWhenAtomicMoveUnsupported() = runTest { val tempFile = Path("$FILE_PATH.temp") diff --git a/kstore-file/src/commonTest/kotlin/io/github/xxfast/kstore/file/extensions/KVersionedStoreTests.kt b/kstore-file/src/commonTest/kotlin/io/github/xxfast/kstore/file/extensions/KVersionedStoreTests.kt index f29310f..7d57737 100644 --- a/kstore-file/src/commonTest/kotlin/io/github/xxfast/kstore/file/extensions/KVersionedStoreTests.kt +++ b/kstore-file/src/commonTest/kotlin/io/github/xxfast/kstore/file/extensions/KVersionedStoreTests.kt @@ -210,6 +210,21 @@ class KVersionedStoreTests { assertEquals(null, storeV2.get()) } + @Test + fun testCodecDoesNotStageThroughTheSharedPaths() = runTest { + // The paths every versioned codec used to stage through - see issue #85 + val sharedData = Path("$file.temp") + val sharedVersion = Path("$versionFile.temp") + write(sharedData, "another codec's staging") + write(sharedVersion, "another codec's staging") + + storeV2.set(MYLO_V2) + + assertEquals(MYLO_V2, storeV2.get()) + assertEquals(true, SystemFileSystem.exists(sharedData)) + assertEquals(true, SystemFileSystem.exists(sharedVersion)) + } + @Test fun testTransactionalEncode() = runTest { assertFailsWith { storeV41.set(MYLO_V41) }