From 2e33ea1a577105890f4dcc494777c32657a358f9 Mon Sep 17 00:00:00 2001 From: Pelotrio <45769595+Pelotrio@users.noreply.github.com> Date: Wed, 30 Sep 2026 14:13:35 +0200 Subject: [PATCH 1/4] Files others write are read by one owner on one serialized path docs/SYSTEMS.md, sections 1 and 2, with the rule agreed after PR 1: each state owner updates its state through one serialized path and publishes immutable snapshots; external changes are rechecked before writes. - Workers: the shared threads, a bounded pool for file work and one timer that only hands work on. - Strand: an owner's tasks one at a time and in order, on those threads. - FileWatch: the one watcher of the folders Companion follows. A folder that does not exist yet is watched through its nearest existing ancestor, one removed while watched is watched again, and one that cannot be watched is read on retries after 1 and 5 seconds and every 30 seconds after. Events are filtered by name. - FileReading: the owner of a value read from such a file. Every read and every change of the value runs on its strand, whoever asks; a write is read once it settled, a failed read is tried again, and changed() fires only when a read found another value. - KeyAssignments is a FileReading of options.txt; its own watcher, scheduler and counters go, and with them the races the last reviews found between a page's read and its own. The order of the remaining PRs puts the files before the remaining owners. Co-Authored-By: Claude Opus 5.5 (1M context) --- .../catalog/KeyAssignments.java | 204 ++---------------- .../totalDebugCompanion/util/FileReading.java | 157 ++++++++++++++ .../totalDebugCompanion/util/FileWatch.java | 180 ++++++++++++++++ .../totalDebugCompanion/util/Strand.java | 52 +++++ .../totalDebugCompanion/util/Workers.java | 43 ++++ .../totalDebugCompanion/SystemsRulesTest.java | 9 +- .../catalog/KeyAssignmentsTest.java | 4 +- .../util/FileReadingTest.java | 170 +++++++++++++++ docs/SYSTEMS.md | 56 ++--- 9 files changed, 660 insertions(+), 215 deletions(-) create mode 100644 companion/src/main/java/com/github/minecraft_ta/totalDebugCompanion/util/FileReading.java create mode 100644 companion/src/main/java/com/github/minecraft_ta/totalDebugCompanion/util/FileWatch.java create mode 100644 companion/src/main/java/com/github/minecraft_ta/totalDebugCompanion/util/Strand.java create mode 100644 companion/src/main/java/com/github/minecraft_ta/totalDebugCompanion/util/Workers.java create mode 100644 companion/src/test/java/com/github/minecraft_ta/totalDebugCompanion/util/FileReadingTest.java diff --git a/companion/src/main/java/com/github/minecraft_ta/totalDebugCompanion/catalog/KeyAssignments.java b/companion/src/main/java/com/github/minecraft_ta/totalDebugCompanion/catalog/KeyAssignments.java index 929d2071c..483231087 100644 --- a/companion/src/main/java/com/github/minecraft_ta/totalDebugCompanion/catalog/KeyAssignments.java +++ b/companion/src/main/java/com/github/minecraft_ta/totalDebugCompanion/catalog/KeyAssignments.java @@ -1,210 +1,44 @@ package com.github.minecraft_ta.totalDebugCompanion.catalog; +import com.github.minecraft_ta.totalDebugCompanion.util.FileReading; import com.github.minecraft_ta.totalDebugCompanion.util.Signal; + import java.io.IOException; -import java.nio.file.ClosedWatchServiceException; -import java.nio.file.FileSystems; import java.nio.file.Path; -import java.nio.file.StandardWatchEventKinds; -import java.nio.file.WatchEvent; -import java.nio.file.WatchKey; -import java.nio.file.WatchService; -import java.util.List; +import java.time.Duration; import java.util.Map; -import java.util.Objects; -import java.util.concurrent.CopyOnWriteArrayList; -import java.util.concurrent.Executors; -import java.util.concurrent.ScheduledExecutorService; -import java.util.concurrent.ScheduledFuture; -import java.util.concurrent.TimeUnit; /** - * The keys {@code options.txt} assigns, watched: whoever writes the file, the game when a key is rebound in its controls - * screen, Companion or an editor, the listeners hear of it once the assignments differ from those read before. Another - * option written, such as the volume, tells nobody. The file's folder is watched, so a file replaced by a rename, as the - * game and Companion write it, is seen; a deleted file assigns nothing. A file that cannot be read keeps the assignments - * read before, and is read again after 1, 5 and 30 seconds and at its next change. Companion's own writes are read at once - * ({@link #readNow()}), so a page shows them without waiting for the watch. + * The keys {@code options.txt} assigns, whoever writes the file: the game when a key is rebound in its controls screen, + * Companion or an editor. Followers hear of it once the assignments differ from those read before; another option + * written, such as the volume, tells nobody, and a deleted file assigns nothing. The file is followed as a + * {@link FileReading}, which reads it once it settled, tries again after a failed read and watches the game's folder + * also before it exists. Companion's own writes are read at once ({@link #readNow()}). */ public final class KeyAssignments implements AutoCloseable { - private static final long SETTLE_MILLIS = 300; - private static final List RETRY_MILLIS = List.of(1_000L, 5_000L, 30_000L); - private static final System.Logger LOGGER = System.getLogger(KeyAssignments.class.getName()); - - private final Path options; - private final Signal changed = new Signal(); - private final ScheduledExecutorService timer = Executors.newSingleThreadScheduledExecutor(task -> Thread.ofPlatform() - .daemon() - .name("Companion options.txt") - .unstarted(task)); - /** Null only where the system has no file watching at all. */ - private final WatchService watcher; - /** - * Whether the game's folder is watched now: not before it exists, or after it was removed; then its watch is tried - * again after 1 and 5 seconds and every 30 seconds after. - */ - private volatile boolean watching; - private ScheduledFuture pending; - /** Counts writes seen; only a read for the latest may tell, so a retry of an earlier one cannot read a write in parts. */ - private long generation; - /** The assignments read last, or null before the first read. */ - private Map read; - /** Whether the last read failed: a page may show that failure, so the next read that succeeds is told. */ - private boolean unreadable; + private final FileReading> reading; - /** Watches {@code options}, whose folder, the game's, may not exist yet: it is watched once it does. */ public KeyAssignments(Path options) { - this.options = Objects.requireNonNull(options, "options").toAbsolutePath().normalize(); - this.watcher = watchService(); - if (this.watcher != null) { - Thread.ofPlatform().daemon().name("Companion options.txt watcher").start(this::watch); - if (!register()) registerLater(0); - } - this.timer.execute(() -> read(0, 0)); - } - - /** - * The keys the file assigns: as last read while its folder is watched, and read now where it has not been read yet - * or is not watched, since then a change would go unseen. The first read, whoever makes it, is what the next change - * is told against, so a follower that read before this owner did still hears of a later change; after it, only - * this owner's own reads are, so every follower hears of a change it has not seen. Blocking where it reads. - */ - public Map assignments() throws IOException { - synchronized (this) { - if (this.read != null && this.watching) return this.read; - } - Map now = KeyBindings.readOptions(this.options); - synchronized (this) { - if (this.read == null) this.read = now; - } - return now; - } - - private static WatchService watchService() { - try { - return FileSystems.getDefault().newWatchService(); - } catch (IOException | RuntimeException unavailable) { - LOGGER.log(System.Logger.Level.WARNING, "Key assignments are read when asked, not watched: " + unavailable.getMessage()); - return null; - } - } - - /** Watches the game's folder; false where it cannot be, as before it exists. */ - private boolean register() { - try { - this.options.getParent().register(this.watcher, StandardWatchEventKinds.ENTRY_CREATE, - StandardWatchEventKinds.ENTRY_MODIFY, StandardWatchEventKinds.ENTRY_DELETE); - this.watching = true; - return true; - } catch (IOException | RuntimeException unwatchable) { - return false; - } + this.reading = new FileReading<>(options, KeyBindings::readOptions, Duration.ofMillis(300)); } - /** - * Tries to watch the game's folder again after a while, and reads what the file assigns by then either way: while it - * cannot be watched, a key rebound in the game is found by these reads. - */ - private void registerLater(int attempt) { - long delay = RETRY_MILLIS.get(Math.min(attempt, RETRY_MILLIS.size() - 1)); - try { - this.timer.schedule(() -> { - boolean watchingNow = register(); - written(); - if (!watchingNow) registerLater(attempt + 1); - }, delay, TimeUnit.MILLISECONDS); - } catch (RuntimeException closed) { - // Closed with the project. - } - } - - /** Fires on a Companion thread after the assignments changed. */ + /** Fires after the assignments changed. */ public Signal changed() { - return this.changed; + return this.reading.changed(); } - /** Reads the file now rather than once a watch saw it settle, as after Companion wrote it. */ - public synchronized void readNow() { - long generation = ++this.generation; - schedule(() -> read(0, generation), 0); - } - - private void watch() { - try { - while (true) { - WatchKey key = this.watcher.take(); - boolean ours = false; - for (WatchEvent event : key.pollEvents()) { - // Lost events may have been about the file too. - ours |= event.kind() == StandardWatchEventKinds.OVERFLOW - || event.context() instanceof Path name && name.equals(this.options.getFileName()); - } - if (ours) written(); - if (!key.reset()) { - // The folder was removed: what the file assigns is read now, and the folder watched again once it is back. - this.watching = false; - written(); - registerLater(0); - } - } - } catch (InterruptedException | ClosedWatchServiceException closed) { - // Closed with the project. - } - } - - /** Reads the file once it has not been written for {@value #SETTLE_MILLIS} ms, as a write can come in parts. */ - private synchronized void written() { - long generation = ++this.generation; - schedule(() -> read(0, generation), SETTLE_MILLIS); - } - - /** Replaces the read waiting, if any, with {@code read} in {@code millis}. */ - private synchronized void schedule(Runnable read, long millis) { - if (this.pending != null) this.pending.cancel(false); - try { - this.pending = this.timer.schedule(read, millis, TimeUnit.MILLISECONDS); - } catch (RuntimeException closed) { - // Closed with the project. - } + /** The keys the file assigns, as read last; read now where none were read yet. Blocking then. */ + public Map assignments() throws IOException { + return this.reading.value(); } - /** Reads the file for write {@code generation}; a newer write makes it tell nothing and try no more. */ - private void read(int attempt, long generation) { - Map now; - try { - now = KeyBindings.readOptions(this.options); - } catch (IOException | RuntimeException unreadable) { - synchronized (this) { - if (generation == this.generation) this.unreadable = true; - if (generation == this.generation && attempt < RETRY_MILLIS.size()) { - schedule(() -> read(attempt + 1, generation), RETRY_MILLIS.get(attempt)); - } - } - return; - } - boolean changed; - synchronized (this) { - // A newer write is read after this one; what this one read is still what the next compares with. - if (generation != this.generation) { - if (this.read == null) this.read = now; - return; - } - changed = this.unreadable || this.read != null && !this.read.equals(now); - this.read = now; - this.unreadable = false; - } - if (changed) this.changed.fire(); + /** Reads the file now, as after Companion wrote it. */ + public void readNow() { + this.reading.readNow(); } - /** Stops watching. It never fails, so a project's shutdown goes on to its queued writes after it. */ @Override public void close() { - this.timer.shutdownNow(); - if (this.watcher == null) return; - try { - this.watcher.close(); - } catch (IOException failure) { - LOGGER.log(System.Logger.Level.WARNING, "The watcher of " + this.options + " did not close: " + failure.getMessage()); - } + this.reading.close(); } } diff --git a/companion/src/main/java/com/github/minecraft_ta/totalDebugCompanion/util/FileReading.java b/companion/src/main/java/com/github/minecraft_ta/totalDebugCompanion/util/FileReading.java new file mode 100644 index 000000000..e72b644ba --- /dev/null +++ b/companion/src/main/java/com/github/minecraft_ta/totalDebugCompanion/util/FileReading.java @@ -0,0 +1,157 @@ +package com.github.minecraft_ta.totalDebugCompanion.util; + +import java.io.IOException; +import java.io.InterruptedIOException; +import java.nio.file.Path; +import java.time.Duration; +import java.util.List; +import java.util.Objects; +import java.util.concurrent.CompletableFuture; +import java.util.concurrent.ExecutionException; +import java.util.concurrent.ScheduledFuture; + +/** + * The owner of a value read from a file others write, such as the keys {@code options.txt} assigns (docs/SYSTEMS.md, + * section 2). Every read of the file and every change of the value runs on this reading's strand, in order; the value is + * published as it was read, and {@link #changed()} fires only when a read found another value than the one before. + * + *

A write is read once it has settled, as a file can be written in parts. A read that fails keeps the value read + * before, and is tried again after 1, 5 and 30 seconds and at the next write; the next read that succeeds is told, since a + * page may show the failure. The file's folder is followed through {@link FileWatch}, also before it exists.

+ */ +public final class FileReading implements AutoCloseable { + /** Reads the value a file holds; a missing file is a value too, such as nothing assigned. Blocking. */ + @FunctionalInterface + public interface Reader { + T read(Path file) throws IOException; + } + + private static final List RETRY_MILLIS = List.of(1_000L, 5_000L, 30_000L); + + /** A value as read, so a value that is null can be told from none read yet. */ + private record Read(T value) { + } + + private final Reader reader; + private final long settleMillis; + private final Signal changed = new Signal(); + private final Strand strand = Workers.strand(); + /** The value read last, or null before the first read: published, so it is read without the strand. */ + private volatile Read last; + // Changed on the strand only. + private Path file; + private Runnable unwatch = () -> { }; + private ScheduledFuture pending; + /** Counts the reads asked for; one waiting for a write to settle gives way to a later one. */ + private long generation; + private boolean unreadable; + private boolean closed; + + /** Reads {@code file} with {@code reader}, and again after each write that stopped for {@code settle}. */ + public FileReading(Path file, Reader reader, Duration settle) { + this.reader = Objects.requireNonNull(reader, "reader"); + this.settleMillis = settle.toMillis(); + Path followed = Objects.requireNonNull(file, "file").toAbsolutePath().normalize(); + this.strand.execute(() -> follow(followed)); + } + + /** Fires on this reading's strand after a read found another value than the one before. */ + public Signal changed() { + return this.changed; + } + + /** + * The value read last; where none was read yet, it is read now, on this reading's strand, so it is the one later + * changes are told against. Blocking then. + */ + public T value() throws IOException { + Read held = this.last; + if (held != null) return held.value(); + CompletableFuture read = new CompletableFuture<>(); + this.strand.execute(() -> { + try { + Read now = this.last; + read.complete(now != null ? now.value() : readHere()); + } catch (IOException | RuntimeException failure) { + read.completeExceptionally(failure); + } + }); + try { + return read.get(); + } catch (ExecutionException failed) { + if (failed.getCause() instanceof IOException unreadable) throw unreadable; + throw new IOException(failed.getCause()); + } catch (InterruptedException interrupted) { + Thread.currentThread().interrupt(); + throw new InterruptedIOException("Stopped while reading the file"); + } + } + + /** Reads the file now rather than once a write settled, as after Companion wrote it. */ + public void readNow() { + this.strand.execute(() -> read(0, ++this.generation)); + } + + /** Follows {@code file} from now on, as the current world after the game opened another; its value is read now. */ + public void moveTo(Path file) { + Path followed = Objects.requireNonNull(file, "file").toAbsolutePath().normalize(); + this.strand.execute(() -> follow(followed)); + } + + /** Stops following the file; a read under way is not told. */ + @Override + public void close() { + this.strand.execute(() -> { + this.closed = true; + this.unwatch.run(); + if (this.pending != null) this.pending.cancel(false); + }); + } + + private void follow(Path followed) { + if (this.closed) return; + this.unwatch.run(); + this.file = followed; + Path name = followed.getFileName(); + this.unwatch = FileWatch.shared().watch(followed.getParent(), name::equals, this::written); + read(0, ++this.generation); + } + + /** A write of the file was seen: it is read once no other came for the settle time. Any thread. */ + private void written() { + this.strand.execute(() -> { + if (this.closed) return; + long generation = ++this.generation; + if (this.pending != null) this.pending.cancel(false); + this.pending = Workers.later(this.settleMillis, this.strand, () -> read(0, generation)); + }); + } + + private void read(int attempt, long generation) { + if (this.closed || generation != this.generation) return; + try { + readHere(); + } catch (IOException | RuntimeException unreadable) { + if (attempt < RETRY_MILLIS.size()) { + this.pending = Workers.later(RETRY_MILLIS.get(attempt), this.strand, () -> read(attempt + 1, generation)); + } + } + } + + /** Reads the file, publishes what it holds and tells the followers where that changed. On the strand only. */ + private T readHere() throws IOException { + T now; + try { + now = this.reader.read(this.file); + } catch (IOException | RuntimeException failure) { + this.unreadable = true; + throw failure instanceof IOException unreadable ? unreadable : new IOException(failure); + } + Read before = this.last; + boolean tell = this.unreadable || before != null && !Objects.equals(before.value(), now); + this.last = new Read<>(now); + this.unreadable = false; + if (tell && !this.closed) this.changed.fire(); + return now; + } +} diff --git a/companion/src/main/java/com/github/minecraft_ta/totalDebugCompanion/util/FileWatch.java b/companion/src/main/java/com/github/minecraft_ta/totalDebugCompanion/util/FileWatch.java new file mode 100644 index 000000000..0278ca0a6 --- /dev/null +++ b/companion/src/main/java/com/github/minecraft_ta/totalDebugCompanion/util/FileWatch.java @@ -0,0 +1,180 @@ +package com.github.minecraft_ta.totalDebugCompanion.util; + +import java.io.IOException; +import java.nio.file.ClosedWatchServiceException; +import java.nio.file.FileSystems; +import java.nio.file.Files; +import java.nio.file.Path; +import java.nio.file.StandardWatchEventKinds; +import java.nio.file.WatchEvent; +import java.nio.file.WatchKey; +import java.nio.file.WatchService; +import java.util.ArrayList; +import java.util.HashMap; +import java.util.List; +import java.util.Map; +import java.util.Objects; +import java.util.concurrent.ScheduledFuture; +import java.util.function.Predicate; + +/** + * The one watcher of the folders Companion follows (docs/SYSTEMS.md, section 2). A folder that does not exist yet, as a + * game's before it first ran, is watched through its nearest existing ancestor and watched itself once it appears; one + * removed while watched is watched that way again. Where a folder cannot be watched at all, it is tried again after 1 + * and 5 seconds and every 30 seconds after, and its follower is told each time, so it reads what it follows then. + * Followers are told on the watcher's thread and hand the work on at once. + */ +public final class FileWatch { + private static final List RETRY_MILLIS = List.of(1_000L, 5_000L, 30_000L); + private static final System.Logger LOGGER = System.getLogger(FileWatch.class.getName()); + private static final FileWatch SHARED = new FileWatch(); + + /** A folder followed, with the names of its entries that matter and what to tell. */ + private static final class Watched { + final Path folder; + final Predicate names; + final Runnable changed; + /** The folder watched for it: its own, or its nearest existing ancestor; null while it cannot be watched. */ + Path at; + ScheduledFuture retry; + boolean closed; + + Watched(Path folder, Predicate names, Runnable changed) { + this.folder = folder; + this.names = names; + this.changed = changed; + } + } + + /** Null where the system has no file watching at all; then every folder is followed by the retries alone. */ + private final WatchService service; + private final Map keys = new HashMap<>(); + private final Map folders = new HashMap<>(); + private final List watched = new ArrayList<>(); + + private FileWatch() { + WatchService created = null; + try { + created = FileSystems.getDefault().newWatchService(); + } catch (IOException | RuntimeException unavailable) { + LOGGER.log(System.Logger.Level.WARNING, "Files are read every 30 seconds, not watched: " + unavailable.getMessage()); + } + this.service = created; + if (created != null) Thread.ofPlatform().daemon().name("Companion file watch").start(this::run); + } + + /** The application's watcher. */ + public static FileWatch shared() { + return SHARED; + } + + /** + * Tells {@code changed} after an entry of {@code folder} whose name {@code names} accepts was created, changed or + * removed, and whenever events may have been lost; returns what stops it. + */ + public Runnable watch(Path folder, Predicate names, Runnable changed) { + Watched followed = new Watched(folder.toAbsolutePath().normalize(), Objects.requireNonNull(names, "names"), + Objects.requireNonNull(changed, "changed")); + synchronized (this) { + this.watched.add(followed); + place(followed, 0); + } + return () -> { + synchronized (this) { + followed.closed = true; + this.watched.remove(followed); + if (followed.retry != null) followed.retry.cancel(false); + release(followed.at); + } + }; + } + + /** Watches {@code followed}'s folder or its nearest existing ancestor, or tries again later. Under the lock. */ + private void place(Watched followed, int attempt) { + Path previous = followed.at; + Path at = followed.folder; + while (at != null && !Files.isDirectory(at)) at = at.getParent(); + followed.at = at != null && register(at) ? at : null; + if (previous != null && !previous.equals(followed.at)) release(previous); + if (followed.at == null) retryLater(followed, attempt); + } + + private boolean register(Path folder) { + if (this.service == null) return false; + if (this.keys.containsKey(folder)) return true; + try { + WatchKey key = folder.register(this.service, StandardWatchEventKinds.ENTRY_CREATE, + StandardWatchEventKinds.ENTRY_MODIFY, StandardWatchEventKinds.ENTRY_DELETE); + this.keys.put(folder, key); + this.folders.put(key, folder); + return true; + } catch (IOException | RuntimeException unwatchable) { + return false; + } + } + + /** Stops watching {@code folder} once nothing followed is watched there. Under the lock. */ + private void release(Path folder) { + if (folder == null) return; + for (Watched followed : this.watched) { + if (folder.equals(followed.at)) return; + } + WatchKey key = this.keys.remove(folder); + if (key == null) return; + this.folders.remove(key); + key.cancel(); + } + + /** Tries to watch {@code followed} again after a while, and tells it then, so it reads what it follows. Under the lock. */ + private void retryLater(Watched followed, int attempt) { + long delay = RETRY_MILLIS.get(Math.min(attempt, RETRY_MILLIS.size() - 1)); + followed.retry = Workers.later(delay, Workers.files(), () -> { + synchronized (this) { + if (followed.closed) return; + place(followed, attempt + 1); + } + followed.changed.run(); + }); + } + + private void run() { + try { + while (true) { + WatchKey key = this.service.take(); + List names = new ArrayList<>(); + boolean lost = false; + for (WatchEvent event : key.pollEvents()) { + if (event.kind() == StandardWatchEventKinds.OVERFLOW) lost = true; + else if (event.context() instanceof Path name) names.add(name); + } + List tell = new ArrayList<>(); + synchronized (this) { + Path folder = this.folders.get(key); + boolean gone = !key.reset(); + if (folder != null) { + if (gone) { + this.keys.remove(folder); + this.folders.remove(key); + } + for (Watched followed : List.copyOf(this.watched)) { + if (!folder.equals(followed.at)) continue; + if (gone || !folder.equals(followed.folder)) { + // Removed, or an ancestor whose entry toward the folder may have appeared: placed again. + Path toward = gone ? null : followed.folder.getName(folder.getNameCount()); + if (gone || lost || names.contains(toward)) { + place(followed, 0); + tell.add(followed.changed); + } + } else if (lost || names.stream().anyMatch(followed.names)) { + tell.add(followed.changed); + } + } + } + } + tell.forEach(Runnable::run); + } + } catch (InterruptedException | ClosedWatchServiceException stopped) { + // The application ends. + } + } +} diff --git a/companion/src/main/java/com/github/minecraft_ta/totalDebugCompanion/util/Strand.java b/companion/src/main/java/com/github/minecraft_ta/totalDebugCompanion/util/Strand.java new file mode 100644 index 000000000..6b7deac04 --- /dev/null +++ b/companion/src/main/java/com/github/minecraft_ta/totalDebugCompanion/util/Strand.java @@ -0,0 +1,52 @@ +package com.github.minecraft_ta.totalDebugCompanion.util; + +import java.util.ArrayDeque; +import java.util.Objects; +import java.util.Queue; +import java.util.concurrent.Executor; + +/** + * Runs tasks one at a time and in the order they came, on shared threads: an owner's one serialized path + * (docs/SYSTEMS.md, section 1). Two tasks of a strand never run at once, so what they change needs no lock among them. + */ +public final class Strand implements Executor { + private static final System.Logger LOGGER = System.getLogger(Strand.class.getName()); + + private final Executor threads; + private final Queue tasks = new ArrayDeque<>(); + private boolean running; + + Strand(Executor threads) { + this.threads = Objects.requireNonNull(threads, "threads"); + } + + @Override + public void execute(Runnable task) { + Objects.requireNonNull(task, "task"); + synchronized (this) { + this.tasks.add(task); + if (this.running) return; + this.running = true; + } + this.threads.execute(this::drain); + } + + private void drain() { + while (true) { + Runnable task; + synchronized (this) { + task = this.tasks.poll(); + if (task == null) { + this.running = false; + return; + } + } + try { + task.run(); + } catch (RuntimeException failure) { + // One task's failure does not stop the ones after it. + LOGGER.log(System.Logger.Level.WARNING, "A task failed on its strand", failure); + } + } + } +} diff --git a/companion/src/main/java/com/github/minecraft_ta/totalDebugCompanion/util/Workers.java b/companion/src/main/java/com/github/minecraft_ta/totalDebugCompanion/util/Workers.java new file mode 100644 index 000000000..cb9149650 --- /dev/null +++ b/companion/src/main/java/com/github/minecraft_ta/totalDebugCompanion/util/Workers.java @@ -0,0 +1,43 @@ +package com.github.minecraft_ta.totalDebugCompanion.util; + +import java.util.concurrent.Executor; +import java.util.concurrent.ExecutorService; +import java.util.concurrent.Executors; +import java.util.concurrent.ScheduledExecutorService; +import java.util.concurrent.ScheduledFuture; +import java.util.concurrent.TimeUnit; +import java.util.concurrent.atomic.AtomicInteger; + +/** + * The threads Companion's owners and pages share (docs/SYSTEMS.md, section 4): a bounded pool for file work, and one + * timer that only hands work on. An owner's serial path is a {@link Strand} over the file work. + */ +public final class Workers { + private static final AtomicInteger FILE_THREADS = new AtomicInteger(); + private static final ExecutorService FILES = Executors.newFixedThreadPool(4, task -> Thread.ofPlatform() + .daemon() + .name("Companion files " + FILE_THREADS.incrementAndGet()) + .unstarted(task)); + private static final ScheduledExecutorService TIMER = Executors.newSingleThreadScheduledExecutor(task -> Thread.ofPlatform() + .daemon() + .name("Companion timer") + .unstarted(task)); + + private Workers() { + } + + /** Where files, zips and the like are read: platform threads, since a virtual one is pinned while it reads a zip. */ + public static Executor files() { + return FILES; + } + + /** A new serial path over the file work, for an owner whose state changes in one order. */ + public static Strand strand() { + return new Strand(FILES); + } + + /** Hands {@code task} to {@code target} after {@code millis}; the timer itself runs nothing else. */ + public static ScheduledFuture later(long millis, Executor target, Runnable task) { + return TIMER.schedule(() -> target.execute(task), millis, TimeUnit.MILLISECONDS); + } +} diff --git a/companion/src/test/java/com/github/minecraft_ta/totalDebugCompanion/SystemsRulesTest.java b/companion/src/test/java/com/github/minecraft_ta/totalDebugCompanion/SystemsRulesTest.java index ffd93e5e5..ad39eced8 100644 --- a/companion/src/test/java/com/github/minecraft_ta/totalDebugCompanion/SystemsRulesTest.java +++ b/companion/src/test/java/com/github/minecraft_ta/totalDebugCompanion/SystemsRulesTest.java @@ -88,9 +88,10 @@ private record Allowed(int times, String why) { Map.entry("ui/components/catalog/TextureThumbnails.java", new Allowed(2, "moves onto Workers in PR 3")), Map.entry("ui/components/catalog/ModLogoIcons.java", new Allowed(2, "moves onto Workers in PR 3")), Map.entry("ui/components/editors/ResourceViewPanel.java", new Allowed(2, "moves onto Workers in PR 3")), - Map.entry("catalog/KeyAssignments.java", new Allowed(3, "becomes a file reading in PR 4")), Map.entry("pack/ExternalEdits.java", new Allowed(3, "becomes an adoption in PR 5")), - Map.entry("util/FileUtils.java", new Allowed(1, "its watcher becomes FileWatch in PR 4"))); + Map.entry("util/FileUtils.java", new Allowed(1, "its watcher becomes a FileWatch follower in PR 5")), + Map.entry("util/Workers.java", new Allowed(4, "the file work and the timer every owner shares")), + Map.entry("util/FileWatch.java", new Allowed(1, "the one watcher of the folders Companion follows"))); // All move onto Workers' file work in PR 3. private static final Map SHARED_POOL = Map.ofEntries( @@ -141,8 +142,8 @@ private record Allowed(int times, String why) { Map.entry("pack/ExternalEdits.java", new Allowed(1, "becomes an adoption in PR 5"))); private static final Map WATCHERS = Map.ofEntries( - Map.entry("util/FileUtils.java", new Allowed(1, "becomes FileWatch in PR 4")), - Map.entry("catalog/KeyAssignments.java", new Allowed(1, "becomes a file reading in PR 4")), + Map.entry("util/FileWatch.java", new Allowed(1, "the one watcher of the folders Companion follows")), + Map.entry("util/FileUtils.java", new Allowed(1, "becomes a FileWatch follower in PR 5")), Map.entry("pack/ExternalEdits.java", new Allowed(1, "becomes an adoption in PR 5"))); @BeforeAll diff --git a/companion/src/test/java/com/github/minecraft_ta/totalDebugCompanion/catalog/KeyAssignmentsTest.java b/companion/src/test/java/com/github/minecraft_ta/totalDebugCompanion/catalog/KeyAssignmentsTest.java index 135a013b2..32b1c4d4c 100644 --- a/companion/src/test/java/com/github/minecraft_ta/totalDebugCompanion/catalog/KeyAssignmentsTest.java +++ b/companion/src/test/java/com/github/minecraft_ta/totalDebugCompanion/catalog/KeyAssignmentsTest.java @@ -52,9 +52,9 @@ void aGameFolderThatAppearsLaterIsWatchedOnceItDoes() throws Exception { // The game runs for the first time: its folder and options.txt appear. Files.createDirectories(options.getParent()); Files.writeString(options, "key_key.jump:key.keyboard.g\n"); - assertEquals(Map.of("key.jump", KeyBindings.Assignment.decode("key.keyboard.g")), assignments.assignments(), - "until the folder is watched, the keys are read whenever asked"); await(told::get, 1); + assertEquals(Map.of("key.jump", KeyBindings.Assignment.decode("key.keyboard.g")), assignments.assignments(), + "once the folder is there, it is followed and its keys are told"); replace(options, "key_key.jump:key.keyboard.h\n"); await(told::get, 2); diff --git a/companion/src/test/java/com/github/minecraft_ta/totalDebugCompanion/util/FileReadingTest.java b/companion/src/test/java/com/github/minecraft_ta/totalDebugCompanion/util/FileReadingTest.java new file mode 100644 index 000000000..93e5234c2 --- /dev/null +++ b/companion/src/test/java/com/github/minecraft_ta/totalDebugCompanion/util/FileReadingTest.java @@ -0,0 +1,170 @@ +package com.github.minecraft_ta.totalDebugCompanion.util; + +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.io.TempDir; + +import java.io.IOException; +import java.nio.charset.StandardCharsets; +import java.nio.file.Files; +import java.nio.file.Path; +import java.time.Duration; +import java.util.concurrent.TimeUnit; +import java.util.concurrent.atomic.AtomicInteger; +import java.util.function.BooleanSupplier; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertTrue; + +/** A file others write, read by its one owner on one path (docs/SYSTEMS.md, sections 1 and 2). */ +class FileReadingTest { + private static final Duration SETTLE = Duration.ofMillis(200); + + @TempDir Path directory; + + @Test + void aWriteInPartsIsReadOnceItSettledAndAnEqualOneTellsNothing() throws Exception { + Path file = write(this.directory.resolve("options.txt"), "a"); + AtomicInteger told = new AtomicInteger(); + try (FileReading reading = new FileReading<>(file, FileReadingTest::text, SETTLE)) { + reading.changed().subscribe(told::incrementAndGet); + assertEquals("a", reading.value()); + + write(file, "b"); + write(file, "bc"); + write(file, "bcd"); + await(() -> told.get() == 1); + Thread.sleep(400); + assertEquals(1, told.get(), "a write in parts is read once, when it settled"); + assertEquals("bcd", reading.value()); + + write(file, "bcd"); + Thread.sleep(600); + assertEquals(1, told.get(), "a write that leaves the value as it was tells nobody"); + } + } + + @Test + void aValueAskedBeforeTheOwnersFirstReadIsWhatTheNextChangeIsToldAgainst() throws Exception { + Path file = write(this.directory.resolve("options.txt"), "a"); + AtomicInteger told = new AtomicInteger(); + try (FileReading reading = new FileReading<>(file, FileReadingTest::text, SETTLE)) { + reading.changed().subscribe(told::incrementAndGet); + // A page opened at once asks, and the game saves right after. + assertEquals("a", reading.value()); + write(file, "b"); + await(() -> told.get() == 1); + assertEquals("b", reading.value()); + } + } + + @Test + void aFolderThatAppearsLaterIsFollowedOnceItDoes() throws Exception { + Path file = this.directory.resolve("game/sub/options.txt"); + AtomicInteger told = new AtomicInteger(); + try (FileReading reading = new FileReading<>(file, FileReadingTest::text, SETTLE)) { + reading.changed().subscribe(told::incrementAndGet); + assertEquals("", reading.value(), "a file that is not there yet holds nothing"); + + // The game runs for the first time. + write(file, "a"); + await(() -> told.get() == 1); + write(file, "b"); + await(() -> told.get() == 2); + assertEquals("b", reading.value()); + } + } + + @Test + void aFailedReadIsTriedAgainAndToldWhenItSucceeds() throws Exception { + Path file = write(this.directory.resolve("options.txt"), "a"); + AtomicInteger failures = new AtomicInteger(0); + AtomicInteger told = new AtomicInteger(); + try (FileReading reading = new FileReading<>(file, path -> { + if (failures.getAndDecrement() > 0) throw new IOException("written in parts"); + return text(path); + }, SETTLE)) { + reading.changed().subscribe(told::incrementAndGet); + assertEquals("a", reading.value()); + + failures.set(1); + write(file, "b"); + await(() -> told.get() == 1); + assertEquals("b", reading.value(), "the read tried again a second later finds the file whole"); + } + } + + @Test + void readsNeverRunAtOnce() throws Exception { + Path file = write(this.directory.resolve("options.txt"), "a"); + AtomicInteger running = new AtomicInteger(); + AtomicInteger overlapping = new AtomicInteger(); + AtomicInteger reads = new AtomicInteger(); + try (FileReading reading = new FileReading<>(file, path -> { + if (running.incrementAndGet() > 1) overlapping.incrementAndGet(); + try { + Thread.sleep(20); + } catch (InterruptedException interrupted) { + Thread.currentThread().interrupt(); + } + reads.incrementAndGet(); + running.decrementAndGet(); + return text(path); + }, SETTLE)) { + reading.value(); + for (int ask = 0; ask < 20; ask++) { + reading.readNow(); + write(file, "v" + ask); + } + await(() -> reads.get() >= 20); + Thread.sleep(500); + assertEquals(0, overlapping.get(), "the owner reads on one path, whoever asks"); + } + } + + @Test + void aReadingMovedToAnotherFileReadsItAndTells() throws Exception { + Path first = write(this.directory.resolve("first/level.dat"), "first"); + Path second = write(this.directory.resolve("second/level.dat"), "second"); + AtomicInteger told = new AtomicInteger(); + try (FileReading reading = new FileReading<>(first, FileReadingTest::text, SETTLE)) { + reading.changed().subscribe(told::incrementAndGet); + assertEquals("first", reading.value()); + + // The game opened another world. + reading.moveTo(second); + await(() -> told.get() == 1); + assertEquals("second", reading.value()); + write(first, "first again"); + Thread.sleep(600); + assertEquals(1, told.get(), "the file followed before tells nothing any more"); + } + } + + @Test + void aClosedReadingTellsNothing() throws Exception { + Path file = write(this.directory.resolve("options.txt"), "a"); + AtomicInteger told = new AtomicInteger(); + FileReading reading = new FileReading<>(file, FileReadingTest::text, SETTLE); + reading.changed().subscribe(told::incrementAndGet); + assertEquals("a", reading.value()); + reading.close(); + write(file, "b"); + Thread.sleep(600); + assertEquals(0, told.get()); + } + + private static String text(Path file) throws IOException { + return Files.isRegularFile(file) ? Files.readString(file, StandardCharsets.UTF_8) : ""; + } + + private static Path write(Path file, String text) throws IOException { + Files.createDirectories(file.getParent()); + return Files.writeString(file, text, StandardCharsets.UTF_8); + } + + private static void await(BooleanSupplier condition) throws InterruptedException { + long deadline = System.nanoTime() + TimeUnit.SECONDS.toNanos(10); + while (!condition.getAsBoolean() && System.nanoTime() < deadline) Thread.sleep(10); + assertTrue(condition.getAsBoolean(), "timed out"); + } +} diff --git a/docs/SYSTEMS.md b/docs/SYSTEMS.md index 208fa1605..4feeb7d19 100644 --- a/docs/SYSTEMS.md +++ b/docs/SYSTEMS.md @@ -35,7 +35,11 @@ page action ─► change pipeline ─► project's write queue ─► owner ─ ## 1. Owners and signals -Every piece of state Companion shows has one owner: a service of the project scope or of the application. The owner holds the current value and compares every new one with it. It fires a `Signal` only when the value differs, so everything downstream can trust that a signal means a change. The comparison is the owner's, of its domain values (parsed assignments, the enabled packs, the catalog), which have a meaningful equality; nothing compares what a page shows, such as images or editor models. +Every piece of state Companion shows has one owner: a service of the project scope or of the application. + +> Each state owner updates its state through one serialized path and publishes immutable snapshots. Background reads return results to that path; they never modify owner state directly. Consumers use published snapshots. External changes are rechecked before writes. + +The serialized path is a `Strand`: the owner's tasks run one at a time and in order, on the shared threads of `Workers`, not on a thread of its own. It rules out races within Companion, such as a page's read and the owner's own read disagreeing about which one the next change is told against. It does not stop the game or an editor from changing a file meanwhile: a published snapshot is consistent but can become outdated, so conflict checks at the write, incomplete files and superseded results are still handled, once, in the owner. The owner holds the current value and compares every new one with it. It fires a `Signal` only when the value differs, so everything downstream can trust that a signal means a change. The comparison is the owner's, of its domain values (parsed assignments, the enabled packs, the catalog), which have a meaningful equality; nothing compares what a page shows, such as images or editor models. ```java public final class Signal { @@ -56,7 +60,7 @@ public final class Signal { - **Connection-bound work compares connections.** `GameLocation`'s connection value includes the connection's number and the game's process. A request waiting for an answer keeps the number of the connection it was sent on, and is failed when the connection signal shows another. This replaces reacting to `DISCONNECTED` as an event, which a quick reconnect can hide. - **The current project is state too.** The application owns it and signals a switch. UI that outlives a project (the tab strip, the Project tree, the status bar, search) follows "the current project's catalog" through its loader, which subscribes again on a switch. This deletes the `project.get() != scope` and `currentScope() == scope` checks. -**Newest wins, where it is the same read.** A page loader runs one read at a time: a request during a read makes one more read after it, and the older result is dropped. When file readings (section 2) need the same, it moves into a class of its own, `LatestRead`, that both use, which also removes `KeyAssignments.generation`. It does not replace an owner's protection between requests of different kinds, where a later request must win over an earlier one that finishes last: `ItemIconService` adopting an archive the game announced over restoring the newest from disk, `PackCatalogService` taking a prepared catalog over a restore. Those counters stay in their owners; a signal counts changes, a counter of requests tells which answer is the newest, and they are different jobs. +**Newest wins, where it is the same read.** A page loader runs one read at a time: a request during a read makes one more read after it, and the older result is dropped. A file reading needs no such bookkeeping: its reads run on its strand, one after the other, and a read waiting for a write to settle gives way to a later write. It does not replace an owner's protection between requests of different kinds, where a later request must win over an earlier one that finishes last: `ItemIconService` adopting an archive the game announced over restoring the newest from disk, `PackCatalogService` taking a prepared catalog over a restore. Those counters stay in their owners; a signal counts changes, a counter of requests tells which answer is the newest, and they are different jobs. Owners: @@ -81,20 +85,25 @@ Owners: Files Companion shows that others write are followed by an owner through a `FileReading`: a watch, a reader, the last value, a comparison and a signal. It is what `KeyAssignments` is today, made once for all. ```java -FileReading assignments = files.reading( - options, // the file or folder, resolved to one path form - Assignments::read, // reads the whole file; runs on file work - Duration.ofMillis(300)); // settle: read once writes stopped for this long +FileReading> reading = new FileReading<>( + options, // the file, which may not exist yet + KeyBindings::readOptions, // reads the whole file into a domain value; a missing file is a value too + Duration.ofMillis(300)); // settle: read once writes stopped for this long +reading.changed(); // fires only when a read found another value +reading.value(); // the published snapshot; read now, on the strand, where none was read yet +reading.readNow(); // after Companion's own write +reading.moveTo(otherFile); // a folder that moves, as the current world ``` -- **One `FileWatch` for the application** does the watching. A registration lives only while its reading has a follower. -- **A folder that does not exist yet** is watched through its nearest existing ancestor, and moves down when it appears: a new world's datapacks, `crash-reports`, `config`. -- **One path form.** Every registration resolves to the real path of its nearest existing ancestor plus the rest, so a linked folder is not watched twice or missed. -- **Settle and maximum wait.** A reading waits until writes stopped for its settle time, but at most a maximum wait, so a file written without pause still reads. -- **A read that fails is retried** after 1, 5 and 30 seconds, as a file written in place can be read half written, and a watch that fails or overflows reads everything it follows once. -- **Folders that move are the owner's.** `CurrentWorld` follows `playing` and moves its reading to the world the game plays, or with the game closed, the last played. -- **Windows renames.** A watched folder and its ancestors cannot be renamed while watched. `FileWatch` pauses around Companion's own renames; registrations sit on the smallest folders that answer the question, and readings of a world are dropped when the game leaves it. Whether the game deleting or renaming a world while it is followed works is tested on Windows in the step that adds world readings. -- **A reading's value is a domain value with a meaningful equality**, such as parsed assignments or a list of names; the reader supplies it, and a reading whose value cannot be compared fires on every change. +- **Every read and every change of the value runs on the reading's strand**, whoever asks: the watch, a retry, Companion's own write, a page. So a page that asks before the first read gets the value later changes are told against. +- **One `FileWatch` for the application** does the watching, with one thread. A registration lives until its reading closes. +- **A folder that does not exist yet** is watched through its nearest existing ancestor, and watched itself once it appears, as a game's folder before it first ran; one removed while watched is watched that way again. +- **Where a folder cannot be watched at all**, as without file watching or on some network drives, it is tried again after 1 and 5 seconds and every 30 seconds after, and the reading reads each time. +- **Events are filtered by name**, so `latest.log` growing beside `options.txt` does not read `options.txt`. +- **A read that fails is retried** after 1, 5 and 30 seconds, as a file written in place can be read half written; the next read that succeeds is told, since a page may show the failure. Overflowing events read once. +- **Folders that move are the owner's.** The current world follows `playing` and moves its reading to the world the game plays, or with the game closed, the last played. +- **Windows renames.** A watched folder and its ancestors cannot be renamed while watched. Registrations sit on the smallest folders that answer the question, and readings of a world move when the game leaves it. `FileUtils`' pause around Companion's own renames joins `FileWatch` when its watcher moves there. +- **A reading's value is a domain value with a meaningful equality**, such as parsed assignments or a list of names; the reader supplies it. - **What each reading holds decides what fires.** The logs' reading holds the names of the logs and crash reports, not their sizes, so `latest.log` growing does not fire; the Logs page reads the log a row shows when the row is shown. A configuration folder's reading ignores Companion's own `*.totaldebug-original` files. - **Readings replace every "read whenever shown":** `options.txt`, configuration files, logs and crash reports, the current world's `level.dat` and icon, the `saves` and `resourcepacks` folders. - **Caches are allowed where keyed by what they cache**, a file's size and time or its hash, as `TextureThumbnails`, `CatalogIcons` and the parsed logs are. A cache is never the truth another part reads. @@ -186,18 +195,17 @@ The test lists each exception with its reason. Adding one is a decision recorded ## Order of work -Six PRs on 1.21.1, each reviewed until clean. A shared mechanism comes with its first users; the PRs that carry risk (1, 2 and 4) are kept small, the ones that repeat a proven pattern (3, 5 and 6) move many users at once. A moved feature keeps no part of its old way. Until the last user of an old way has moved, the old way stays for those not moved yet, as `PageLoader`'s modes do; the PR that moves the last user deletes it. Decided on 2026-09-30 over a finer split of about 22 PRs, whose extra review rounds bought no safety for the mechanical moves. +PRs on 1.21.1, stacked, each reviewed until clean. A shared mechanism comes with its first users; the PRs that carry risk are kept small, the ones that repeat a proven pattern move many users at once. A moved feature keeps no part of its old way. Until the last user of an old way has moved, the old way stays for those not moved yet, as `PageLoader`'s modes do; the PR that moves the last user deletes it. Decided on 2026-09-30 over a finer split of about 22 PRs, whose extra review rounds bought no safety for the mechanical moves. The files came before the remaining owners the same day: they carry most of the races the reviews found, and they are `Workers`' first users. | PR | Content | Deletes | |---|---|---| -| 1 | The slice: `Signal` and the new `PageLoader` (`page`, `follows`, `hold`), with `Tables.keepingSelection`; its first users the Key bindings page, which only shows, and the resource editor, which edits, with the owners they follow on signals (catalog, key assignments, change record, packs, resource edits), key assignments read again after Companion's own write; the architecture test with every exception of today listed | the two pages' own subscriptions, constructor reads, navigation refreshes and selection keeping, `KeyBindingControl.addAssignmentListener`, `PackResourceEditor`'s read and write counters and follow flags | -| 2 | Game messages registered by their owners on the connection, in Companion and the mod | `CompanionSession.Listener`'s message methods, the relay and scope checks in `CompanionApplication`, the `RELAY_FAILED` switch, the mod's setters and repeated guards | -| 3 | The remaining owners on signals; the current project as state; connection numbers for waiting requests; the pipeline owning the write queue; the remaining executors onto `Workers` | the listener lists, `ConfigChanges`' executor, the scope checks, the UI classes' executors | -| 4 | `Workers`' timers, `FileWatch` and `FileReading`, with `KeyAssignments` and `CurrentWorld` (the World page and the tree) as first users | the watchers and schedulers of `FileUtils` and `KeyAssignments`, `WorldReadings` | -| 5 | The remaining readings (`GameLogs`, the configuration folders, `saves` and `resourcepacks`) with their pages; `ExternalEdits` as an adoption | `ExternalEdits`' watcher and scheduler, the `readsWhenShown` mode | +| 1 (#112) | The slice: `Signal` and the new `PageLoader` (`page`, `follows`, `hold`), with `Tables.keepingSelection`; its first users the Key bindings page, which only shows, and the resource editor, which edits, with the owners they follow on signals (catalog, key assignments, change record, packs, resource edits), key assignments read again after Companion's own write; the architecture test with every exception of today listed | the two pages' own subscriptions, constructor reads, navigation refreshes and selection keeping, `KeyBindingControl.addAssignmentListener`, `PackResourceEditor`'s read and write counters and follow flags | +| 2 (#113) | Game messages registered by their owners on the connection, in Companion and the mod | `CompanionSession.Listener`'s methods for the project's messages, their relay in `CompanionApplication`, the mod's setters and repeated guards | +| 3 | `Workers` and `Strand`, `FileWatch` and `FileReading`, with `KeyAssignments` as first user | `KeyAssignments`' watcher, scheduler and counters | +| 4 | The current world as a `FileReading` that follows `playing`, with the World page and the tree | `WorldReadings`, the World page's read whenever shown | +| 5 | The remaining readings (`GameLogs`, the configuration folders, `saves` and `resourcepacks`) with their pages; `FileUtils`' watcher onto `FileWatch`; `ExternalEdits` as an adoption | the watchers and schedulers of `FileUtils` and `ExternalEdits`, the `readsWhenShown` mode | | 6 | All remaining pages, the Project tree and the tab strip; no file checks on the Swing thread | `ShownUpdates`, the old modes, the pages' own subscriptions, the reads in constructors, the navigation refreshes, the `CompanionUi` relays, `ChangeRecord.observed` from page reads | - -If PR 1 shows a flaw, the design is revised here before anything else moves. PR 3 needs `Workers` before PR 4; it brings the file work and serial workers, PR 4 the timers. +| 7 | The remaining owners on signals; the current project as state; connection numbers for waiting requests; the pipeline owning the write queue; the remaining executors onto `Workers` | the listener lists, `ConfigChanges`' executor, the scope checks, the UI classes' executors | After the messages, A4 continues with categories registering their pages and Modpack rows. Splitting `CompanionApplication` (the game connection and the MCP server into their own classes) and the mod's `CompanionAppClient` (launching Companion) is easier then and is decided at that point. @@ -205,12 +213,12 @@ UI work that follows no system, such as every page's loading, empty and failed s ## Tests -- `Signal`: listeners in order, removal of one subscription. `LatestRead`, with PR 4: a request during a read, several during one, a failure. +- `Signal`: listeners in order, removal of one subscription. `FileReading`: a write in parts read once, a value asked before the first read, a folder that appears later, a failed read retried, reads never at once, a reading moved to another file, a closed reading. - `FileReading` and `FileWatch`, through a clock the test advances instead of real waits: settle and maximum wait, a folder that appears later, a linked folder, overflow, a failed read retried, a pause for a rename, a reading whose value did not change. - `PageLoader`: hidden and shown, a signal during a read, several while hidden, a change that does not concern the page, a held page, a read under way when the page holds, a part of a page. The old modes' tests go with the modes. - Messages: two handlers in order, a handler of a detached scope not called, an answer to another connection not taken, a request without a handler refused, an unauthenticated message refused in the mod. - The architecture test: no listener list, thread, executor, watcher or one-argument async call outside the owners and exceptions it lists. -- **Each moved page is tested along the user's path**, not only through its parts: the page built as the application builds it, opened through `NavigationService` as a click opens it, then shown. The test counts the reads: one when it opens; none when it is shown again with nothing changed; one after a signal while it was hidden; none when navigating to it while it is showing, apart from the selection asked for. `LatestRead` passing its own tests does not show that opening a page reads once. +- **Each moved page is tested along the user's path**, not only through its parts: the page built as the application builds it, opened through `NavigationService` as a click opens it, then shown. The test counts the reads: one when it opens; none when it is shown again with nothing changed; one after a signal while it was hidden; none when navigating to it while it is showing, apart from the selection asked for. The loader passing its own tests does not show that opening a page reads once. - Each migration PR keeps the tests of what it moves. ## Not in this From b49d46b45c8bc77c65d1825cd3be08c2298a65b0 Mon Sep 17 00:00:00 2001 From: Pelotrio <45769595+Pelotrio@users.noreply.github.com> Date: Wed, 30 Sep 2026 14:28:58 +0200 Subject: [PATCH 2/4] FileWatch looks for the folder beneath again once it watches one A folder made between looking for the nearest existing one and watching it, as a game makes its folders at once on its first start, was never seen, and the watch stayed a level too high. Once a folder is watched, the one beneath is looked for again, down to the folder followed; ancestors passed on the way are let go. Co-Authored-By: Claude Opus 5.5 (1M context) --- .../totalDebugCompanion/util/FileWatch.java | 22 ++++++++++++++----- .../util/FileReadingTest.java | 17 ++++++++++++++ 2 files changed, 34 insertions(+), 5 deletions(-) diff --git a/companion/src/main/java/com/github/minecraft_ta/totalDebugCompanion/util/FileWatch.java b/companion/src/main/java/com/github/minecraft_ta/totalDebugCompanion/util/FileWatch.java index 0278ca0a6..7f3fef485 100644 --- a/companion/src/main/java/com/github/minecraft_ta/totalDebugCompanion/util/FileWatch.java +++ b/companion/src/main/java/com/github/minecraft_ta/totalDebugCompanion/util/FileWatch.java @@ -92,11 +92,23 @@ public Runnable watch(Path folder, Predicate names, Runnable changed) { /** Watches {@code followed}'s folder or its nearest existing ancestor, or tries again later. Under the lock. */ private void place(Watched followed, int attempt) { Path previous = followed.at; - Path at = followed.folder; - while (at != null && !Files.isDirectory(at)) at = at.getParent(); - followed.at = at != null && register(at) ? at : null; - if (previous != null && !previous.equals(followed.at)) release(previous); - if (followed.at == null) retryLater(followed, attempt); + Path at; + do { + at = followed.folder; + while (at != null && !Files.isDirectory(at)) at = at.getParent(); + if (at == null || !register(at)) { + at = null; + break; + } + // A folder created between looking and watching, as a game makes its folders at once, was not seen: once + // watched, the folder beneath is looked for again. + } while (!at.equals(followed.folder) && Files.isDirectory(followed.folder.getRoot().resolve( + followed.folder.subpath(0, at.getNameCount() + 1)))); + followed.at = at; + if (previous != null && !previous.equals(at)) release(previous); + // Ancestors registered on the way down that no longer hold anything are let go. + for (Path passed = at == null ? null : at.getParent(); passed != null; passed = passed.getParent()) release(passed); + if (at == null) retryLater(followed, attempt); } private boolean register(Path folder) { diff --git a/companion/src/test/java/com/github/minecraft_ta/totalDebugCompanion/util/FileReadingTest.java b/companion/src/test/java/com/github/minecraft_ta/totalDebugCompanion/util/FileReadingTest.java index 93e5234c2..bdf3d8731 100644 --- a/companion/src/test/java/com/github/minecraft_ta/totalDebugCompanion/util/FileReadingTest.java +++ b/companion/src/test/java/com/github/minecraft_ta/totalDebugCompanion/util/FileReadingTest.java @@ -74,6 +74,23 @@ void aFolderThatAppearsLaterIsFollowedOnceItDoes() throws Exception { } } + @Test + void foldersMadeAtOnceAreFollowedDownToTheFile() throws Exception { + for (int round = 0; round < 5; round++) { + Path file = this.directory.resolve("round" + round + "/game/a/b/options.txt"); + AtomicInteger told = new AtomicInteger(); + try (FileReading reading = new FileReading<>(file, FileReadingTest::text, SETTLE)) { + reading.changed().subscribe(told::incrementAndGet); + assertEquals("", reading.value()); + // As a game makes its folders in one go on its first start, while the watch moves down to them. + write(file, "a"); + await(() -> told.get() == 1); + write(file, "b"); + await(() -> told.get() == 2); + } + } + } + @Test void aFailedReadIsTriedAgainAndToldWhenItSucceeds() throws Exception { Path file = write(this.directory.resolve("options.txt"), "a"); From cc5e2c24da8d56e094e08134c0fa31e2b38ee480 Mon Sep 17 00:00:00 2001 From: Pelotrio <45769595+Pelotrio@users.noreply.github.com> Date: Wed, 30 Sep 2026 14:37:41 +0200 Subject: [PATCH 3/4] FileReading: a read overtaken or closed publishes nothing; FileWatch shares a watch by folder From the review of the slice: - A write seen while a read runs is counted at once, on the watcher's thread, and that read publishes nothing, so a file read half written is never shown; the read after the write settled is. - close() stops publication at once, so a read under way then tells nothing. - A folder reached through a link shares its watch with every other path to it, and each follower is matched by its watch, so one closing does not stop another. Co-Authored-By: Claude Opus 5.5 (1M context) --- .../totalDebugCompanion/util/FileReading.java | 39 +++--- .../totalDebugCompanion/util/FileWatch.java | 87 +++++++------- .../util/FileReadingTest.java | 112 +++++++++++++++++- 3 files changed, 181 insertions(+), 57 deletions(-) diff --git a/companion/src/main/java/com/github/minecraft_ta/totalDebugCompanion/util/FileReading.java b/companion/src/main/java/com/github/minecraft_ta/totalDebugCompanion/util/FileReading.java index e72b644ba..96b275511 100644 --- a/companion/src/main/java/com/github/minecraft_ta/totalDebugCompanion/util/FileReading.java +++ b/companion/src/main/java/com/github/minecraft_ta/totalDebugCompanion/util/FileReading.java @@ -9,6 +9,7 @@ import java.util.concurrent.CompletableFuture; import java.util.concurrent.ExecutionException; import java.util.concurrent.ScheduledFuture; +import java.util.concurrent.atomic.AtomicLong; /** * The owner of a value read from a file others write, such as the keys {@code options.txt} assigns (docs/SYSTEMS.md, @@ -42,10 +43,14 @@ private record Read(T value) { private Path file; private Runnable unwatch = () -> { }; private ScheduledFuture pending; - /** Counts the reads asked for; one waiting for a write to settle gives way to a later one. */ - private long generation; + /** + * Counts the reads asked for, counted where they are asked, also on the watcher's thread: a read waiting for a write + * to settle, or one under way when another write is seen, gives way to the later one and publishes nothing. + */ + private final AtomicLong generation = new AtomicLong(); private boolean unreadable; - private boolean closed; + /** Set as soon as the reading closes, so a read under way then publishes nothing. */ + private volatile boolean closed; /** Reads {@code file} with {@code reader}, and again after each write that stopped for {@code settle}. */ public FileReading(Path file, Reader reader, Duration settle) { @@ -71,7 +76,7 @@ public T value() throws IOException { this.strand.execute(() -> { try { Read now = this.last; - read.complete(now != null ? now.value() : readHere()); + read.complete(now != null ? now.value() : readHere(0)); } catch (IOException | RuntimeException failure) { read.completeExceptionally(failure); } @@ -89,7 +94,8 @@ public T value() throws IOException { /** Reads the file now rather than once a write settled, as after Companion wrote it. */ public void readNow() { - this.strand.execute(() -> read(0, ++this.generation)); + long generation = this.generation.incrementAndGet(); + this.strand.execute(() -> read(0, generation)); } /** Follows {@code file} from now on, as the current world after the game opened another; its value is read now. */ @@ -98,11 +104,11 @@ public void moveTo(Path file) { this.strand.execute(() -> follow(followed)); } - /** Stops following the file; a read under way is not told. */ + /** Stops following the file; a read under way publishes nothing. */ @Override public void close() { + this.closed = true; this.strand.execute(() -> { - this.closed = true; this.unwatch.run(); if (this.pending != null) this.pending.cancel(false); }); @@ -114,23 +120,24 @@ private void follow(Path followed) { this.file = followed; Path name = followed.getFileName(); this.unwatch = FileWatch.shared().watch(followed.getParent(), name::equals, this::written); - read(0, ++this.generation); + read(0, this.generation.incrementAndGet()); } /** A write of the file was seen: it is read once no other came for the settle time. Any thread. */ private void written() { + // Counted here, so a read under way gives way at once. + long generation = this.generation.incrementAndGet(); this.strand.execute(() -> { if (this.closed) return; - long generation = ++this.generation; if (this.pending != null) this.pending.cancel(false); this.pending = Workers.later(this.settleMillis, this.strand, () -> read(0, generation)); }); } private void read(int attempt, long generation) { - if (this.closed || generation != this.generation) return; + if (this.closed || generation != this.generation.get()) return; try { - readHere(); + readHere(generation); } catch (IOException | RuntimeException unreadable) { if (attempt < RETRY_MILLIS.size()) { this.pending = Workers.later(RETRY_MILLIS.get(attempt), this.strand, () -> read(attempt + 1, generation)); @@ -138,8 +145,11 @@ private void read(int attempt, long generation) { } } - /** Reads the file, publishes what it holds and tells the followers where that changed. On the strand only. */ - private T readHere() throws IOException { + /** + * Reads the file, publishes what it holds and tells the followers where that changed. A read for {@code generation}, + * other than 0 for one a page waits for, publishes nothing where another write was seen meanwhile. On the strand only. + */ + private T readHere(long generation) throws IOException { T now; try { now = this.reader.read(this.file); @@ -147,11 +157,12 @@ private T readHere() throws IOException { this.unreadable = true; throw failure instanceof IOException unreadable ? unreadable : new IOException(failure); } + if (generation != 0 && generation != this.generation.get() || this.closed) return now; Read before = this.last; boolean tell = this.unreadable || before != null && !Objects.equals(before.value(), now); this.last = new Read<>(now); this.unreadable = false; - if (tell && !this.closed) this.changed.fire(); + if (tell) this.changed.fire(); return now; } } diff --git a/companion/src/main/java/com/github/minecraft_ta/totalDebugCompanion/util/FileWatch.java b/companion/src/main/java/com/github/minecraft_ta/totalDebugCompanion/util/FileWatch.java index 7f3fef485..1246c13c5 100644 --- a/companion/src/main/java/com/github/minecraft_ta/totalDebugCompanion/util/FileWatch.java +++ b/companion/src/main/java/com/github/minecraft_ta/totalDebugCompanion/util/FileWatch.java @@ -36,6 +36,8 @@ private static final class Watched { final Runnable changed; /** The folder watched for it: its own, or its nearest existing ancestor; null while it cannot be watched. */ Path at; + /** The watch of {@link #at}, which a folder reached through another path, as a link, shares. */ + WatchKey key; ScheduledFuture retry; boolean closed; @@ -48,8 +50,8 @@ private static final class Watched { /** Null where the system has no file watching at all; then every folder is followed by the retries alone. */ private final WatchService service; + /** The watches by the real path of their folder, so one reached through a link is watched once. */ private final Map keys = new HashMap<>(); - private final Map folders = new HashMap<>(); private final List watched = new ArrayList<>(); private FileWatch() { @@ -84,56 +86,62 @@ public Runnable watch(Path folder, Predicate names, Runnable changed) { followed.closed = true; this.watched.remove(followed); if (followed.retry != null) followed.retry.cancel(false); - release(followed.at); + release(followed.key); } }; } /** Watches {@code followed}'s folder or its nearest existing ancestor, or tries again later. Under the lock. */ private void place(Watched followed, int attempt) { - Path previous = followed.at; + WatchKey previous = followed.key; + List passed = new ArrayList<>(); Path at; - do { + WatchKey key; + while (true) { at = followed.folder; while (at != null && !Files.isDirectory(at)) at = at.getParent(); - if (at == null || !register(at)) { + key = at == null ? null : register(at); + if (key == null) { at = null; break; } // A folder created between looking and watching, as a game makes its folders at once, was not seen: once // watched, the folder beneath is looked for again. - } while (!at.equals(followed.folder) && Files.isDirectory(followed.folder.getRoot().resolve( - followed.folder.subpath(0, at.getNameCount() + 1)))); + if (at.equals(followed.folder) || !Files.isDirectory(followed.folder.getRoot().resolve( + followed.folder.subpath(0, at.getNameCount() + 1)))) break; + passed.add(key); + } followed.at = at; - if (previous != null && !previous.equals(at)) release(previous); - // Ancestors registered on the way down that no longer hold anything are let go. - for (Path passed = at == null ? null : at.getParent(); passed != null; passed = passed.getParent()) release(passed); - if (at == null) retryLater(followed, attempt); + followed.key = key; + if (previous != null && previous != key) release(previous); + // Ancestors watched on the way down that nothing else follows are let go. + passed.forEach(this::release); + if (key == null) retryLater(followed, attempt); } - private boolean register(Path folder) { - if (this.service == null) return false; - if (this.keys.containsKey(folder)) return true; + /** The watch of {@code folder}, shared with every path that leads to it; null where it cannot be watched. */ + private WatchKey register(Path folder) { + if (this.service == null) return null; try { - WatchKey key = folder.register(this.service, StandardWatchEventKinds.ENTRY_CREATE, + Path real = folder.toRealPath(); + WatchKey key = this.keys.get(real); + if (key != null && key.isValid()) return key; + key = real.register(this.service, StandardWatchEventKinds.ENTRY_CREATE, StandardWatchEventKinds.ENTRY_MODIFY, StandardWatchEventKinds.ENTRY_DELETE); - this.keys.put(folder, key); - this.folders.put(key, folder); - return true; + this.keys.put(real, key); + return key; } catch (IOException | RuntimeException unwatchable) { - return false; + return null; } } - /** Stops watching {@code folder} once nothing followed is watched there. Under the lock. */ - private void release(Path folder) { - if (folder == null) return; + /** Stops the watch {@code key} once nothing followed uses it. Under the lock. */ + private void release(WatchKey key) { + if (key == null) return; for (Watched followed : this.watched) { - if (folder.equals(followed.at)) return; + if (followed.key == key) return; } - WatchKey key = this.keys.remove(folder); - if (key == null) return; - this.folders.remove(key); + this.keys.values().remove(key); key.cancel(); } @@ -161,25 +169,20 @@ private void run() { } List tell = new ArrayList<>(); synchronized (this) { - Path folder = this.folders.get(key); boolean gone = !key.reset(); - if (folder != null) { - if (gone) { - this.keys.remove(folder); - this.folders.remove(key); - } - for (Watched followed : List.copyOf(this.watched)) { - if (!folder.equals(followed.at)) continue; - if (gone || !folder.equals(followed.folder)) { - // Removed, or an ancestor whose entry toward the folder may have appeared: placed again. - Path toward = gone ? null : followed.folder.getName(folder.getNameCount()); - if (gone || lost || names.contains(toward)) { - place(followed, 0); - tell.add(followed.changed); - } - } else if (lost || names.stream().anyMatch(followed.names)) { + if (gone) this.keys.values().remove(key); + for (Watched followed : List.copyOf(this.watched)) { + if (followed.key != key) continue; + if (gone || !followed.at.equals(followed.folder)) { + // Removed, or an ancestor whose entry toward the folder may have appeared: placed again. + Path toward = gone ? null : followed.folder.getName(followed.at.getNameCount()); + if (gone || lost || names.contains(toward)) { + if (gone) followed.key = null; + place(followed, 0); tell.add(followed.changed); } + } else if (lost || names.stream().anyMatch(followed.names)) { + tell.add(followed.changed); } } } diff --git a/companion/src/test/java/com/github/minecraft_ta/totalDebugCompanion/util/FileReadingTest.java b/companion/src/test/java/com/github/minecraft_ta/totalDebugCompanion/util/FileReadingTest.java index bdf3d8731..890674e41 100644 --- a/companion/src/test/java/com/github/minecraft_ta/totalDebugCompanion/util/FileReadingTest.java +++ b/companion/src/test/java/com/github/minecraft_ta/totalDebugCompanion/util/FileReadingTest.java @@ -3,6 +3,12 @@ import org.junit.jupiter.api.Test; import org.junit.jupiter.api.io.TempDir; +import java.util.concurrent.atomic.AtomicReference; +import java.util.concurrent.atomic.AtomicBoolean; +import java.util.concurrent.CountDownLatch; +import java.util.concurrent.CopyOnWriteArrayList; +import java.util.List; +import java.io.UncheckedIOException; import java.io.IOException; import java.nio.charset.StandardCharsets; import java.nio.file.Files; @@ -132,7 +138,8 @@ void readsNeverRunAtOnce() throws Exception { reading.readNow(); write(file, "v" + ask); } - await(() -> reads.get() >= 20); + // Reads a later request overtook are left out; the last write is read in the end. + await(() -> "v19".equals(value(reading))); Thread.sleep(500); assertEquals(0, overlapping.get(), "the owner reads on one path, whoever asks"); } @@ -170,6 +177,109 @@ void aClosedReadingTellsNothing() throws Exception { assertEquals(0, told.get()); } + @Test + void aReadOvertakenByAnotherWritePublishesNothing() throws Exception { + Path file = write(this.directory.resolve("options.txt"), "a"); + AtomicReference gate = new AtomicReference<>(); + CountDownLatch entered = new CountDownLatch(1); + List told = new CopyOnWriteArrayList<>(); + try (FileReading reading = new FileReading<>(file, path -> { + String read = text(path); + CountDownLatch waiting = gate.get(); + if (waiting != null) { + entered.countDown(); + try { + waiting.await(5, TimeUnit.SECONDS); + } catch (InterruptedException interrupted) { + Thread.currentThread().interrupt(); + } + } + return read; + }, SETTLE)) { + assertEquals("a", reading.value()); + reading.changed().subscribe(() -> told.add(value(reading))); + + // The game writes in parts: the first part is read, and the next part comes while that read runs. + CountDownLatch release = new CountDownLatch(1); + gate.set(release); + write(file, "partial"); + assertTrue(entered.await(5, TimeUnit.SECONDS)); + gate.set(null); + write(file, "partial and whole"); + Thread.sleep(100); + release.countDown(); + await(() -> !told.isEmpty()); + Thread.sleep(500); + assertEquals(List.of("partial and whole"), told, "the read the next write overtook is not told"); + } + } + + @Test + void aReadingClosedWhileItReadsTellsNothing() throws Exception { + Path file = write(this.directory.resolve("options.txt"), "a"); + CountDownLatch entered = new CountDownLatch(1); + CountDownLatch release = new CountDownLatch(1); + AtomicBoolean waitNext = new AtomicBoolean(); + AtomicInteger told = new AtomicInteger(); + FileReading reading = new FileReading<>(file, path -> { + String read = text(path); + if (waitNext.getAndSet(false)) { + entered.countDown(); + try { + release.await(5, TimeUnit.SECONDS); + } catch (InterruptedException interrupted) { + Thread.currentThread().interrupt(); + } + } + return read; + }, SETTLE); + assertEquals("a", reading.value()); + reading.changed().subscribe(told::incrementAndGet); + waitNext.set(true); + write(file, "b"); + assertTrue(entered.await(5, TimeUnit.SECONDS)); + // The project closes while its reading reads. + reading.close(); + release.countDown(); + Thread.sleep(500); + assertEquals(0, told.get(), "a reading closed while it read tells nothing"); + } + + @Test + void aFolderReachedThroughALinkIsFollowedByEveryReadingOfIt() throws Exception { + Path folder = Files.createDirectories(this.directory.resolve("game")); + Path link; + try { + link = Files.createSymbolicLink(this.directory.resolve("linked"), folder); + } catch (UnsupportedOperationException | IOException | SecurityException notAllowed) { + // Windows without the privilege to create links: nothing to check here. + return; + } + Path file = write(folder.resolve("options.txt"), "a"); + AtomicInteger direct = new AtomicInteger(); + AtomicInteger linked = new AtomicInteger(); + try (FileReading first = new FileReading<>(file, FileReadingTest::text, SETTLE); + FileReading second = new FileReading<>(link.resolve("options.txt"), FileReadingTest::text, SETTLE)) { + first.changed().subscribe(direct::incrementAndGet); + second.changed().subscribe(linked::incrementAndGet); + first.value(); + second.value(); + write(file, "b"); + await(() -> direct.get() == 1 && linked.get() == 1); + second.close(); + write(file, "c"); + await(() -> direct.get() == 2); + } + } + + private static String value(FileReading reading) { + try { + return reading.value(); + } catch (IOException unreadable) { + throw new UncheckedIOException(unreadable); + } + } + private static String text(Path file) throws IOException { return Files.isRegularFile(file) ? Files.readString(file, StandardCharsets.UTF_8) : ""; } From 301fe46b4f208ff4a253efacd530e7cc6db70aad Mon Sep 17 00:00:00 2001 From: Pelotrio <45769595+Pelotrio@users.noreply.github.com> Date: Wed, 30 Sep 2026 14:58:08 +0200 Subject: [PATCH 4/4] Owners' strands run apart from file work; a failing follower or an overtaken read changes nothing From the reviews of the slice: - Owners' strands run on threads of their own, so a page's read on the file work that waits for an owner's first value never holds the thread that value needs. - A follower whose callback fails is logged, and the watch goes on for every other. - A read another write overtook marks nothing unreadable, so the next read with the same value tells nobody. - moveTo() makes a read of the file before give way at once, as readNow(), a write and close() do. Co-Authored-By: Claude Opus 5.5 (1M context) --- .../totalDebugCompanion/util/FileReading.java | 14 ++- .../totalDebugCompanion/util/FileWatch.java | 9 +- .../totalDebugCompanion/util/Workers.java | 18 ++- .../totalDebugCompanion/SystemsRulesTest.java | 2 +- .../util/FileReadingTest.java | 109 ++++++++++++++++++ 5 files changed, 141 insertions(+), 11 deletions(-) diff --git a/companion/src/main/java/com/github/minecraft_ta/totalDebugCompanion/util/FileReading.java b/companion/src/main/java/com/github/minecraft_ta/totalDebugCompanion/util/FileReading.java index 96b275511..80415bd04 100644 --- a/companion/src/main/java/com/github/minecraft_ta/totalDebugCompanion/util/FileReading.java +++ b/companion/src/main/java/com/github/minecraft_ta/totalDebugCompanion/util/FileReading.java @@ -57,7 +57,8 @@ public FileReading(Path file, Reader reader, Duration settle) { this.reader = Objects.requireNonNull(reader, "reader"); this.settleMillis = settle.toMillis(); Path followed = Objects.requireNonNull(file, "file").toAbsolutePath().normalize(); - this.strand.execute(() -> follow(followed)); + long generation = this.generation.incrementAndGet(); + this.strand.execute(() -> follow(followed, generation)); } /** Fires on this reading's strand after a read found another value than the one before. */ @@ -101,7 +102,9 @@ public void readNow() { /** Follows {@code file} from now on, as the current world after the game opened another; its value is read now. */ public void moveTo(Path file) { Path followed = Objects.requireNonNull(file, "file").toAbsolutePath().normalize(); - this.strand.execute(() -> follow(followed)); + // A read of the file before gives way at once, so nothing it read is published after the move was asked for. + long generation = this.generation.incrementAndGet(); + this.strand.execute(() -> follow(followed, generation)); } /** Stops following the file; a read under way publishes nothing. */ @@ -114,13 +117,13 @@ public void close() { }); } - private void follow(Path followed) { + private void follow(Path followed, long generation) { if (this.closed) return; this.unwatch.run(); this.file = followed; Path name = followed.getFileName(); this.unwatch = FileWatch.shared().watch(followed.getParent(), name::equals, this::written); - read(0, this.generation.incrementAndGet()); + read(0, generation); } /** A write of the file was seen: it is read once no other came for the settle time. Any thread. */ @@ -154,7 +157,8 @@ private T readHere(long generation) throws IOException { try { now = this.reader.read(this.file); } catch (IOException | RuntimeException failure) { - this.unreadable = true; + // A read another write overtook fails for nothing that is current: the next read is not told for it. + if (generation == 0 || generation == this.generation.get()) this.unreadable = true; throw failure instanceof IOException unreadable ? unreadable : new IOException(failure); } if (generation != 0 && generation != this.generation.get() || this.closed) return now; diff --git a/companion/src/main/java/com/github/minecraft_ta/totalDebugCompanion/util/FileWatch.java b/companion/src/main/java/com/github/minecraft_ta/totalDebugCompanion/util/FileWatch.java index 1246c13c5..c4a32f097 100644 --- a/companion/src/main/java/com/github/minecraft_ta/totalDebugCompanion/util/FileWatch.java +++ b/companion/src/main/java/com/github/minecraft_ta/totalDebugCompanion/util/FileWatch.java @@ -186,7 +186,14 @@ private void run() { } } } - tell.forEach(Runnable::run); + for (Runnable changed : tell) { + try { + changed.run(); + } catch (RuntimeException failure) { + // One follower's failure does not stop the watch of every other. + LOGGER.log(System.Logger.Level.WARNING, "A follower of a watched folder failed", failure); + } + } } } catch (InterruptedException | ClosedWatchServiceException stopped) { // The application ends. diff --git a/companion/src/main/java/com/github/minecraft_ta/totalDebugCompanion/util/Workers.java b/companion/src/main/java/com/github/minecraft_ta/totalDebugCompanion/util/Workers.java index cb9149650..3d2994a95 100644 --- a/companion/src/main/java/com/github/minecraft_ta/totalDebugCompanion/util/Workers.java +++ b/companion/src/main/java/com/github/minecraft_ta/totalDebugCompanion/util/Workers.java @@ -9,15 +9,25 @@ import java.util.concurrent.atomic.AtomicInteger; /** - * The threads Companion's owners and pages share (docs/SYSTEMS.md, section 4): a bounded pool for file work, and one - * timer that only hands work on. An owner's serial path is a {@link Strand} over the file work. + * The threads Companion's owners and pages share (docs/SYSTEMS.md, section 4): a bounded pool for file work, one for + * owners' strands, and one timer that only hands work on. An owner's serial path is a {@link Strand} over the owners' + * pool, where no task waits for another owner's. */ public final class Workers { private static final AtomicInteger FILE_THREADS = new AtomicInteger(); + private static final AtomicInteger OWNER_THREADS = new AtomicInteger(); private static final ExecutorService FILES = Executors.newFixedThreadPool(4, task -> Thread.ofPlatform() .daemon() .name("Companion files " + FILE_THREADS.incrementAndGet()) .unstarted(task)); + /** + * The threads owners' strands run on, apart from the file work: a page's read that waits for an owner's first value + * never takes the thread that value needs. + */ + private static final ExecutorService OWNERS = Executors.newFixedThreadPool(4, task -> Thread.ofPlatform() + .daemon() + .name("Companion owners " + OWNER_THREADS.incrementAndGet()) + .unstarted(task)); private static final ScheduledExecutorService TIMER = Executors.newSingleThreadScheduledExecutor(task -> Thread.ofPlatform() .daemon() .name("Companion timer") @@ -31,9 +41,9 @@ public static Executor files() { return FILES; } - /** A new serial path over the file work, for an owner whose state changes in one order. */ + /** A new serial path, for an owner whose state changes in one order. */ public static Strand strand() { - return new Strand(FILES); + return new Strand(OWNERS); } /** Hands {@code task} to {@code target} after {@code millis}; the timer itself runs nothing else. */ diff --git a/companion/src/test/java/com/github/minecraft_ta/totalDebugCompanion/SystemsRulesTest.java b/companion/src/test/java/com/github/minecraft_ta/totalDebugCompanion/SystemsRulesTest.java index ad39eced8..290bd5f8c 100644 --- a/companion/src/test/java/com/github/minecraft_ta/totalDebugCompanion/SystemsRulesTest.java +++ b/companion/src/test/java/com/github/minecraft_ta/totalDebugCompanion/SystemsRulesTest.java @@ -90,7 +90,7 @@ private record Allowed(int times, String why) { Map.entry("ui/components/editors/ResourceViewPanel.java", new Allowed(2, "moves onto Workers in PR 3")), Map.entry("pack/ExternalEdits.java", new Allowed(3, "becomes an adoption in PR 5")), Map.entry("util/FileUtils.java", new Allowed(1, "its watcher becomes a FileWatch follower in PR 5")), - Map.entry("util/Workers.java", new Allowed(4, "the file work and the timer every owner shares")), + Map.entry("util/Workers.java", new Allowed(6, "the file work, the owners' strands and the timer everything shares")), Map.entry("util/FileWatch.java", new Allowed(1, "the one watcher of the folders Companion follows"))); // All move onto Workers' file work in PR 3. diff --git a/companion/src/test/java/com/github/minecraft_ta/totalDebugCompanion/util/FileReadingTest.java b/companion/src/test/java/com/github/minecraft_ta/totalDebugCompanion/util/FileReadingTest.java index 890674e41..227933e23 100644 --- a/companion/src/test/java/com/github/minecraft_ta/totalDebugCompanion/util/FileReadingTest.java +++ b/companion/src/test/java/com/github/minecraft_ta/totalDebugCompanion/util/FileReadingTest.java @@ -3,6 +3,8 @@ import org.junit.jupiter.api.Test; import org.junit.jupiter.api.io.TempDir; +import java.util.concurrent.CompletableFuture; +import java.util.ArrayList; import java.util.concurrent.atomic.AtomicReference; import java.util.concurrent.atomic.AtomicBoolean; import java.util.concurrent.CountDownLatch; @@ -272,6 +274,113 @@ void aFolderReachedThroughALinkIsFollowedByEveryReadingOfIt() throws Exception { } } + @Test + void readingsAskedForTheirFirstValueFromEveryFileWorkerAtOnceAllRead() throws Exception { + List> values = new ArrayList<>(); + List> readings = new ArrayList<>(); + try { + for (int each = 0; each < 8; each++) { + Path file = write(this.directory.resolve("file" + each + ".txt"), "v" + each); + FileReading reading = new FileReading<>(file, FileReadingTest::text, SETTLE); + readings.add(reading); + // As pages whose reads run on the file work each ask for a value not read yet. + values.add(CompletableFuture.supplyAsync(() -> value(reading), Workers.files())); + } + for (int each = 0; each < 8; each++) assertEquals("v" + each, values.get(each).get(10, TimeUnit.SECONDS)); + } finally { + readings.forEach(FileReading::close); + } + } + + @Test + void aFollowerThatFailsDoesNotStopTheWatchOfOthers() throws Exception { + Path folder = Files.createDirectories(this.directory.resolve("game")); + AtomicInteger told = new AtomicInteger(); + Runnable failing = FileWatch.shared().watch(folder, name -> true, () -> { + throw new IllegalStateException("a follower's bug"); + }); + Runnable counting = FileWatch.shared().watch(folder, name -> true, told::incrementAndGet); + try { + write(folder.resolve("first.txt"), "a"); + await(() -> told.get() >= 1); + int before = told.get(); + write(folder.resolve("second.txt"), "b"); + await(() -> told.get() > before); + } finally { + failing.run(); + counting.run(); + } + } + + @Test + void aFailedReadAnotherWriteOvertookDoesNotMakeTheNextOneTell() throws Exception { + Path file = write(this.directory.resolve("options.txt"), "a"); + AtomicReference gate = new AtomicReference<>(); + CountDownLatch entered = new CountDownLatch(1); + AtomicInteger told = new AtomicInteger(); + try (FileReading reading = new FileReading<>(file, path -> { + CountDownLatch waiting = gate.getAndSet(null); + if (waiting != null) { + entered.countDown(); + try { + waiting.await(5, TimeUnit.SECONDS); + } catch (InterruptedException interrupted) { + Thread.currentThread().interrupt(); + } + throw new IOException("read while it was written"); + } + return text(path); + }, SETTLE)) { + assertEquals("a", reading.value()); + reading.changed().subscribe(told::incrementAndGet); + CountDownLatch release = new CountDownLatch(1); + gate.set(release); + write(file, "a"); + assertTrue(entered.await(5, TimeUnit.SECONDS)); + // Written again, with the same text, while the read fails. + write(file, "a"); + Thread.sleep(100); + release.countDown(); + Thread.sleep(1_000); + assertEquals(0, told.get(), "the value is as it was, and the failure was of a read overtaken"); + } + } + + @Test + void aReadOfTheFileBeforeAMoveIsNotPublishedAfterIt() throws Exception { + Path first = write(this.directory.resolve("first/level.dat"), "first"); + Path second = write(this.directory.resolve("second/level.dat"), "second"); + AtomicReference gate = new AtomicReference<>(); + CountDownLatch entered = new CountDownLatch(1); + List told = new CopyOnWriteArrayList<>(); + try (FileReading reading = new FileReading<>(first, path -> { + String read = text(path); + CountDownLatch waiting = gate.getAndSet(null); + if (waiting != null) { + entered.countDown(); + try { + waiting.await(5, TimeUnit.SECONDS); + } catch (InterruptedException interrupted) { + Thread.currentThread().interrupt(); + } + } + return read; + }, SETTLE)) { + assertEquals("first", reading.value()); + reading.changed().subscribe(() -> told.add(value(reading))); + CountDownLatch release = new CountDownLatch(1); + gate.set(release); + write(first, "first saved"); + assertTrue(entered.await(5, TimeUnit.SECONDS)); + // The game opens the other world while the first is read. + reading.moveTo(second); + release.countDown(); + await(() -> !told.isEmpty()); + Thread.sleep(500); + assertEquals(List.of("second"), told, "what was read of the world before the move is not told"); + } + } + private static String value(FileReading reading) { try { return reading.value();