From 8eea1177a373bf00d485fdbe36ee54980af377ec Mon Sep 17 00:00:00 2001 From: Nowko Date: Fri, 16 Oct 2026 11:03:46 +0200 Subject: [PATCH] fix: resume pets after a killed server when no deletion was lost A server killed mid-shutdown left pets-recovery-required behind, and every later start disabled CompanionPets until an operator removed it by hand. The session now also arms pets-session-intact, which is withdrawn as soon as a death or release cannot reach pet-deletions.log, a migration is held, or the shutdown save fails. When both files are found at startup, every terminal transition is already in the log, so the store logs a warning and resumes from the last saved pets.yml. A guard without the proof, including one left by an older release, still requires reconciliation. Co-Authored-By: Claude Opus 5.5 --- .../companionpets/store/PetStore.java | 52 +++++++++++++++++-- .../PetsPluginLifecycleTest.java | 13 +++++ .../companionpets/store/PetStoreTest.java | 42 ++++++++++++++- 3 files changed, 102 insertions(+), 5 deletions(-) diff --git a/src/main/java/net/tfminecraft/companionpets/store/PetStore.java b/src/main/java/net/tfminecraft/companionpets/store/PetStore.java index cae6680..1358997 100644 --- a/src/main/java/net/tfminecraft/companionpets/store/PetStore.java +++ b/src/main/java/net/tfminecraft/companionpets/store/PetStore.java @@ -51,6 +51,8 @@ public final class PetStore { private final File backup; private final File deletions; private final File session; + /** Present while every death and release of the active session is in pet-deletions.log. */ + private final File intact; private final Set deleted = new LinkedHashSet<>(); private final Map pets = new LinkedHashMap<>(); private record CachedRow(long revision, Map values) { } @@ -82,6 +84,7 @@ public PetStore(JavaPlugin plugin) { this.backup = new File(file.getParentFile(), file.getName() + ".bak"); this.deletions = new File(file.getParentFile(), "pet-deletions.log"); this.session = new File(file.getParentFile(), "pets-recovery-required"); + this.intact = new File(file.getParentFile(), "pets-session-intact"); this.logger = logger; } @@ -90,9 +93,21 @@ public boolean load() { loaded = false; ownershipReady = false; deletionLogHealthy = false; - if (session.exists()) { - logger.severe("Previous pet session did not commit a clean shutdown. Stop the server and reconcile pets.yml " - + "with pet-deletions.log and actual deaths before removing pets-recovery-required; saving is disabled"); + if (!session.exists()) { + discard(intact); + } else if (intact.exists()) { + // A killed process only loses unsaved positions and needs: every death and release + // reached pet-deletions.log before its body went away, so the saved data is consistent. + logger.warning("Previous pet session stopped without a clean shutdown; no deaths or releases were left " + + "uncommitted, so pets resume from the last saved pets.yml"); + if (!discard(session) || !discard(intact)) { + logger.severe("Could not clear pets-recovery-required; saving is disabled"); + return false; + } + } else { + logger.severe("Previous pet session did not commit a clean shutdown, and a pet save or deletion may have failed. " + + "Stop the server and reconcile pets.yml with pet-deletions.log and actual deaths before removing " + + "pets-recovery-required; saving is disabled"); return false; } try { @@ -410,6 +425,11 @@ public boolean beginSession() { if (!loaded || !deletionLogHealthy || sessionStarted) return false; try { Files.createDirectories(session.toPath().getParent()); + // The proof goes first, so a crash between the two writes leaves only a stale proof. + try (FileChannel channel = FileChannel.open(intact.toPath(), StandardOpenOption.CREATE, + StandardOpenOption.WRITE, StandardOpenOption.TRUNCATE_EXISTING)) { + writeAndForce(channel, "Every pet death and release of the active session is in pet-deletions.log.\n"); + } try (FileChannel channel = FileChannel.open(session.toPath(), StandardOpenOption.CREATE_NEW, StandardOpenOption.WRITE)) { writeAndForce(channel, "Pet session active; reconcile terminal transitions if shutdown does not complete.\n"); } @@ -417,6 +437,7 @@ public boolean beginSession() { return true; } catch (IOException ex) { loaded = false; + discard(intact); logger.log(Level.SEVERE, "Could not arm pet recovery protection; saving is disabled", ex); return false; } @@ -432,10 +453,14 @@ public boolean close() { Thread.currentThread().interrupt(); } } - if (!saved) return false; + if (!saved) { + breakIntact(); + return false; + } if (sessionStarted) { try { Files.delete(session.toPath()); + Files.deleteIfExists(intact.toPath()); sessionStarted = false; } catch (IOException ex) { logger.log(Level.SEVERE, "Could not confirm clean pet shutdown", ex); @@ -445,6 +470,23 @@ public boolean close() { return true; } + /** + * Withdraws the proof that a killed session may resume unattended. A delete needs no free + * space, so it still works after the write that failed. + */ + private void breakIntact() { + if (!discard(intact)) logger.severe("Could not remove pets-session-intact; remove it before the next start"); + } + + private static boolean discard(File target) { + try { + Files.deleteIfExists(target.toPath()); + return true; + } catch (IOException | SecurityException ex) { + return false; + } + } + static void writeAndForce(FileChannel channel, String value) throws IOException { ByteBuffer bytes = StandardCharsets.UTF_8.encode(value); while (bytes.hasRemaining()) channel.write(bytes); @@ -643,6 +685,7 @@ public boolean remove(UUID id) { } } catch (IOException ex) { deletionLogHealthy = false; + breakIntact(); logger.log(Level.SEVERE, "Could not commit deletion for pet " + id + "; body recovery and saving are disabled. The session guard requires operator reconciliation", ex); return false; @@ -744,6 +787,7 @@ public boolean migrate() { } catch (IOException ex) { logger.log(Level.SEVERE, "Pet ownership migration is held; saving disabled", ex); loaded = false; + breakIntact(); return false; } } diff --git a/src/test/java/net/tfminecraft/companionpets/PetsPluginLifecycleTest.java b/src/test/java/net/tfminecraft/companionpets/PetsPluginLifecycleTest.java index 8ed2287..9dcb678 100644 --- a/src/test/java/net/tfminecraft/companionpets/PetsPluginLifecycleTest.java +++ b/src/test/java/net/tfminecraft/companionpets/PetsPluginLifecycleTest.java @@ -142,6 +142,19 @@ private WolfMock body(Pet pet, Location at) { assertTrue(messages.stream().anyMatch(record -> record.getMessage().contains("did not commit a clean shutdown"))); } + @Test void killedServerWithAnIntactSessionStartsPetsAgain() throws Exception { + prepare(); var owner = server.addPlayer(); var pet = pet(owner.getUniqueId()); pet.stored(true); saveBeforeEnable(pet); + Files.writeString(file("pets-session-intact"), "intact\n"); + Files.writeString(file("pets-recovery-required"), "session active\n"); + enable(); + assertTrue(plugin.isEnabled()); assertNotNull(runtime().store().get(pet.id())); + assertTrue(messages.stream().anyMatch(record -> record.getMessage().contains("pets resume from the last saved pets.yml"))); + assertTrue(Files.exists(file("pets-recovery-required")), "The new session arms its own guard"); + server.getPluginManager().disablePlugin(plugin); + assertFalse(Files.exists(file("pets-recovery-required"))); + assertFalse(Files.exists(file("pets-session-intact"))); + } + @Test void corruptSavedPetsDisableStartupWithoutOverwritingTheEvidence() throws Exception { prepare(); Files.writeString(file("pets.yml"), "pets: [broken\n"); server.getPluginManager().enablePlugin(plugin); diff --git a/src/test/java/net/tfminecraft/companionpets/store/PetStoreTest.java b/src/test/java/net/tfminecraft/companionpets/store/PetStoreTest.java index 358cd96..c97260e 100644 --- a/src/test/java/net/tfminecraft/companionpets/store/PetStoreTest.java +++ b/src/test/java/net/tfminecraft/companionpets/store/PetStoreTest.java @@ -637,6 +637,7 @@ void deletionLogFailureRefusesReleaseAndBlocksRecoveryAfterRestart() throws Exce assertFalse(current.canRestoreBodies()); assertFalse(current.save()); assertFalse(current.close()); + assertFalse(Files.exists(directory.resolve("pets-session-intact"))); PetStore restarted = store(); assertFalse(restarted.load()); assertNull(restarted.get(id)); @@ -644,12 +645,39 @@ void deletionLogFailureRefusesReleaseAndBlocksRecoveryAfterRestart() throws Exce } @Test - void interruptedSessionRequiresReconciliationEvenIfSaveDataIsValid() throws Exception { + void killedHealthySessionResumesFromTheLastSaveAndKeepsCommittedDeaths() throws Exception { + UUID kept = UUID.randomUUID(), died = UUID.randomUUID(); PetStore current = store(); assertTrue(current.load()); assertTrue(current.beginSession()); + current.add(net.tfminecraft.companionpets.testutil.OwnershipFixture.pet(kept, UUID.randomUUID(), "wolf", "Rex", PetSex.MALE)); + current.add(net.tfminecraft.companionpets.testutil.OwnershipFixture.pet(died, UUID.randomUUID(), "wolf", "Ash", PetSex.MALE)); + assertTrue(current.save()); + assertTrue(current.remove(died)); + current.get(kept).name("Unsaved rename"); + // The process dies here: no close(), and pets.yml still lists the pet that died. + assertTrue(Files.readString(directory.resolve("pets.yml")).contains(died.toString())); + + PetStore restarted = store(); + assertTrue(restarted.load()); + assertEquals("Rex", restarted.get(kept).name()); + assertNull(restarted.get(died)); + assertTrue(restarted.isDeleted(died)); + assertFalse(Files.exists(directory.resolve("pets-recovery-required"))); + assertFalse(Files.exists(directory.resolve("pets-session-intact"))); + assertTrue(restarted.beginSession()); + assertTrue(restarted.close()); + assertFalse(Files.exists(directory.resolve("pets-recovery-required"))); + assertFalse(Files.exists(directory.resolve("pets-session-intact"))); + } + + @Test + void legacyRecoveryGuardWithoutIntactProofStillRequiresReconciliation() throws Exception { + PetStore current = store(); + assertTrue(current.load()); assertTrue(current.save()); String primary = Files.readString(directory.resolve("pets.yml")); + Files.writeString(directory.resolve("pets-recovery-required"), "guard from an older release\n"); PetStore restarted = store(); assertFalse(restarted.load()); @@ -659,6 +687,17 @@ void interruptedSessionRequiresReconciliationEvenIfSaveDataIsValid() throws Exce assertTrue(Files.exists(directory.resolve("pets-recovery-required"))); } + @Test + void staleIntactProofWithoutGuardIsDiscarded() throws Exception { + Files.writeString(directory.resolve("pets-session-intact"), "left by a crash while arming\n"); + PetStore current = store(); + assertTrue(current.load()); + assertFalse(Files.exists(directory.resolve("pets-session-intact"))); + assertTrue(current.beginSession()); + assertTrue(Files.exists(directory.resolve("pets-session-intact"))); + assertTrue(current.close()); + } + @Test void cleanSessionShutdownAllowsRepeatedRestarts() { UUID id = UUID.randomUUID(); @@ -696,6 +735,7 @@ void cannotEnableRuntimeUnlessSessionGuardIsDurable() throws Exception { assertTrue(current.load()); Files.createDirectory(directory.resolve("pets-recovery-required")); assertFalse(current.beginSession()); + assertFalse(Files.exists(directory.resolve("pets-session-intact"))); assertFalse(current.save()); assertFalse(current.canRestoreBodies()); assertFalse(current.close());