From 0da80849d4ae205a47a8baa621eb8c2ee06b0d6b Mon Sep 17 00:00:00 2001 From: Marcus van Elst Date: Tue, 28 Apr 2026 15:28:18 +0200 Subject: [PATCH] Fix #52: Race Condition auf pendingMessages beim Test-Trigger pendingMessages.clear() wurde aus runTechnicalTestsAction() in den Coordinator verlagert (erste Anweisung in triggerTechnicalTests()). Damit liegen clear() und Worker-Start auf demselben Thread (FX), und das Race-Fenster zwischen clear() und den per Platform.runLater zurueckgefuehrten add()-Aufrufen entfaellt. Die fachliche Produktionssemantik (Replace beim Trigger) bleibt identisch. JavaDoc und Kommentare wurden auf Replace-Semantik korrigiert. Der Smoke-Test trigger_twice_accumulatesTestEntries wurde zu trigger_twice_replacesTestEntries umbenannt und prueft nun die Replace-Erwartung des isolierten Coordinators. Co-Authored-By: Claude Opus 4.7 (1M context) --- .../in/gui/GuiConfigurationEditorWorkspace.java | 3 ++- .../in/gui/GuiTechnicalTestCoordinator.java | 13 ++++++++++--- .../gui/GuiTechnicalTestCoordinatorSmokeTest.java | 15 ++++++++------- 3 files changed, 20 insertions(+), 11 deletions(-) diff --git a/pdf-umbenenner-adapter-in-gui/src/main/java/de/gecheckt/pdf/umbenenner/adapter/in/gui/GuiConfigurationEditorWorkspace.java b/pdf-umbenenner-adapter-in-gui/src/main/java/de/gecheckt/pdf/umbenenner/adapter/in/gui/GuiConfigurationEditorWorkspace.java index a7ee815..ef96b33 100644 --- a/pdf-umbenenner-adapter-in-gui/src/main/java/de/gecheckt/pdf/umbenenner/adapter/in/gui/GuiConfigurationEditorWorkspace.java +++ b/pdf-umbenenner-adapter-in-gui/src/main/java/de/gecheckt/pdf/umbenenner/adapter/in/gui/GuiConfigurationEditorWorkspace.java @@ -1925,7 +1925,8 @@ public final class GuiConfigurationEditorWorkspace { */ private void runTechnicalTestsAction() { LOG.info("Aktion Technische Tests ausführen gestartet."); - pendingMessages.clear(); + // Hinweis: pendingMessages.clear() erfolgt jetzt im Coordinator selbst, + // damit clear() und Worker-Start auf demselben Thread (FX) liegen. technicalTestsButton.setDisable(true); technicalTestCoordinator.triggerTechnicalTests(); } diff --git a/pdf-umbenenner-adapter-in-gui/src/main/java/de/gecheckt/pdf/umbenenner/adapter/in/gui/GuiTechnicalTestCoordinator.java b/pdf-umbenenner-adapter-in-gui/src/main/java/de/gecheckt/pdf/umbenenner/adapter/in/gui/GuiTechnicalTestCoordinator.java index 74d2427..15a9e99 100644 --- a/pdf-umbenenner-adapter-in-gui/src/main/java/de/gecheckt/pdf/umbenenner/adapter/in/gui/GuiTechnicalTestCoordinator.java +++ b/pdf-umbenenner-adapter-in-gui/src/main/java/de/gecheckt/pdf/umbenenner/adapter/in/gui/GuiTechnicalTestCoordinator.java @@ -113,6 +113,9 @@ public final class GuiTechnicalTestCoordinator { /** * Löst die asynchrone Ausführung des vollständigen technischen Gesamttests aus. *

+ * Vor dem Worker-Start wird die geteilte Nachrichtenliste auf dem FX-Thread geleert; + * jeder Aufruf ersetzt die zuvor angefügten Einträge (Replace-Semantik). + *

* Liest den aktuellen Editorzustand und den Konfigurationsdateipfad, baut einen * {@link TechnicalTestRequest} und startet den {@link TechnicalTestOrchestrator} auf * einem Hintergrund-Worker-Thread. Das Ergebnis wird via {@code resultDelivery} an den @@ -124,6 +127,11 @@ public final class GuiTechnicalTestCoordinator { * Muss auf dem JavaFX Application Thread aufgerufen werden. */ public void triggerTechnicalTests() { + // Bestehende Nachrichtenliste auf dem FX-Thread leeren, bevor der Worker-Thread + // startet. Dadurch laufen clear() und nachfolgende add()-Aufrufe (die per + // Platform.runLater wieder auf dem FX-Thread landen) auf demselben Thread und + // es entsteht kein Race-Fenster mit der UI. + pendingMessages.clear(); EditorValidationInput input = inputProvider.get(); String configFilePath = configFilePathProvider.get(); TechnicalTestRequest request = new TechnicalTestRequest(input, configFilePath); @@ -146,15 +154,14 @@ public final class GuiTechnicalTestCoordinator { * Wendet das Ergebnis des vollständigen Gesamttests auf die geteilte Nachrichtenliste an. *

* Fügt für jedes Checkpoint-Ergebnis einen neuen Eintrag zur geteilten Nachrichtenliste - * hinzu; vorhandene Einträge bleiben erhalten, sodass die Meldungen über mehrere - * Testläufe hinweg akkumulieren. Zusätzlich wird eine Zusammenfassung angehängt. + * hinzu. Die Liste wurde zuvor in {@link #triggerTechnicalTests()} geleert, sodass jeder + * Aufruf einen frischen Stand erzeugt. Zusätzlich wird eine Zusammenfassung angehängt. *

* Muss nur auf dem JavaFX Application Thread aufgerufen werden (via {@code resultDelivery}). * * @param report der vollständige Gesamttestbericht; darf nicht {@code null} sein */ private void applyResult(TechnicalTestReport report) { - // Akkumulieren: Vorherige Einträge anderer Läufe bleiben erhalten. long successCount = 0; long failureErrorCount = 0; diff --git a/pdf-umbenenner-adapter-in-gui/src/test/java/de/gecheckt/pdf/umbenenner/adapter/in/gui/GuiTechnicalTestCoordinatorSmokeTest.java b/pdf-umbenenner-adapter-in-gui/src/test/java/de/gecheckt/pdf/umbenenner/adapter/in/gui/GuiTechnicalTestCoordinatorSmokeTest.java index 05889fd..04355ff 100644 --- a/pdf-umbenenner-adapter-in-gui/src/test/java/de/gecheckt/pdf/umbenenner/adapter/in/gui/GuiTechnicalTestCoordinatorSmokeTest.java +++ b/pdf-umbenenner-adapter-in-gui/src/test/java/de/gecheckt/pdf/umbenenner/adapter/in/gui/GuiTechnicalTestCoordinatorSmokeTest.java @@ -162,18 +162,19 @@ class GuiTechnicalTestCoordinatorSmokeTest { } // ========================================================================= - // Scenario: accumulation semantics – second trigger appends fresh entries + // Scenario: replace semantics – second trigger replaces the previous batch // ========================================================================= /** - * Smoke test: triggering the coordinator twice accumulates both runs; the - * second trigger appends a fresh batch of SOURCE_TAG entries without - * removing the first batch. + * Smoke test: triggering the coordinator twice replaces the previous batch; + * the second trigger clears the shared message list before applying its own + * SOURCE_TAG entries, so the count after the second run equals the count + * after the first run. * * @throws Exception if the FX thread task fails or times out */ @Test - void trigger_twice_accumulatesTestEntries() throws Exception { + void trigger_twice_replacesTestEntries() throws Exception { runOnFx(() -> { List messages = new ArrayList<>(); GuiTechnicalTestCoordinator coordinator = buildSyncCoordinator(messages, report -> { }); @@ -190,8 +191,8 @@ class GuiTechnicalTestCoordinatorSmokeTest { && GuiTechnicalTestCoordinator.SOURCE_TAG.equals(m.source().get())) .count(); - assertEquals(countAfterFirst * 2, countAfterSecond, - "Second trigger must append a fresh batch, doubling the SOURCE_TAG entries"); + assertEquals(countAfterFirst, countAfterSecond, + "Second trigger must clear and replace the previous SOURCE_TAG batch"); }); }