diff --git a/owlplug-client/src/main/java/com/owlplug/core/utils/Async.java b/owlplug-client/src/main/java/com/owlplug/core/utils/Async.java new file mode 100644 index 00000000..2fb92ea1 --- /dev/null +++ b/owlplug-client/src/main/java/com/owlplug/core/utils/Async.java @@ -0,0 +1,196 @@ +/* OwlPlug + * Copyright (C) 2021 Arthur + * + * This file is part of OwlPlug. + * + * OwlPlug is free software: you can redistribute it and/or modify + * it under the terms of the GNU General Public License version 3 + * as published by the Free Software Foundation. + * + * OwlPlug is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with OwlPlug. If not, see . + */ + +package com.owlplug.core.utils; + +import java.util.concurrent.CompletableFuture; +import java.util.concurrent.Executor; +import java.util.concurrent.Executors; +import java.util.concurrent.atomic.AtomicLong; +import java.util.function.Supplier; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +public final class Async { + + private static final Logger log = LoggerFactory.getLogger(Async.class); + + // One virtual thread per task — safe for blocking I/O and database calls. + private static final Executor VIRTUAL = Executors.newVirtualThreadPerTaskExecutor(); + + private Async() { + } + + /** + * Run a task on a virtual thread. Exceptions are logged as a side-effect; + * the returned future still completes exceptionally so callers can chain + * their own {@code .exceptionally()} if needed. + */ + public static CompletableFuture run(Runnable task) { + CompletableFuture cf = CompletableFuture.runAsync(task, VIRTUAL); + cf.whenComplete((result, ex) -> { + if (ex != null) { + log.error("Unhandled async exception", ex); + } + }); + return cf; + } + + /** + * Supply a value on a virtual thread. Exceptions are logged as a side-effect; + * the returned future still completes exceptionally so callers can chain + * their own {@code .exceptionally()} if needed. + */ + public static CompletableFuture supply(Supplier task) { + CompletableFuture cf = CompletableFuture.supplyAsync(task, VIRTUAL); + cf.whenComplete((result, ex) -> { + if (ex != null) { + log.error("Unhandled async exception", ex); + } + }); + return cf; + } + + /** + * Raw virtual-thread {@code runAsync} with no default handler — use when the + * caller owns exception handling entirely via {@code .exceptionally()}. + */ + public static CompletableFuture runAsync(Runnable task) { + return CompletableFuture.runAsync(task, VIRTUAL); + } + + /** + * Raw virtual-thread {@code supplyAsync} with no default handler — use when + * the caller owns exception handling entirely via {@code .exceptionally()}. + */ + public static CompletableFuture supplyAsync(Supplier task) { + return CompletableFuture.supplyAsync(task, VIRTUAL); + } + + /** + * Ensures that only the result of the latest async call is ever + * delivered to the caller, discarding results from superseded invocations. + * + *

Problem

+ * When the same async operation is triggered multiple times in quick + * succession (e.g. a UI selection change firing {@code refresh()}), results + * can arrive out of order: an older, slower query may resolve after a newer + * one, overwriting fresh data with stale data. + * + *

Solution

+ * Each call to {@link #supply} or {@link #run} atomically claims a new + * generation stamp. When the result arrives, it is only forwarded + * if the stamp still matches the current generation — i.e. no newer call has + * been made in the meantime. Stale futures are silently left incomplete, so + * any chained {@code .thenAccept()} simply never fires. No extra logic is + * required at the call site. + * + *

Usage

+ * Declare one {@code Sequence} field per independent refresh slot on the + * controller, then replace {@code Async.supply(...)} with + * {@code mySequence.supply(...)}: + * 
{@code
+   * private final Async.Sequence refreshSeq = new Async.Sequence();
+   *
+   * void refresh() {
+   *     refreshSeq.supply(() -> repository.findAll())
+   *               .thenAccept(data -> FX.run(() -> listView.setItems(data)));
+   * }
+   * }
+ * + *

Notes

+ *
    + *
  • Exceptions are always logged, even for stale results, because a DB + * error is worth knowing about regardless of whether it was superseded.
    • + *
  • Stale futures are never completed, so they carry no memory overhead + * beyond normal GC eligibility once the chain is unreachable.
    • + *
  • Use one {@code Sequence} per independent data slot. A controller with + * two unrelated async loads should use two separate instances.
    • + *
+ */ + public static final class Sequence { + + private final AtomicLong generation = new AtomicLong(); + + /** + * Submits {@code task} on a virtual thread and returns a guarded future. + * The future completes normally only if no newer call to this method has + * been made by the time the task finishes; otherwise it is left incomplete + * and downstream stages are never executed. + * + * @param task the blocking supplier to run off the FX thread + * @param the result type + * @return a future that delivers the result only when it is still current + */ + public CompletableFuture supply(Supplier task) { + // Claim this invocation's stamp before launching the task so that any + // call arriving concurrently gets a strictly higher generation number. + long stamp = generation.incrementAndGet(); + CompletableFuture inner = CompletableFuture.supplyAsync(task, VIRTUAL); + CompletableFuture guarded = new CompletableFuture<>(); + inner.whenComplete((result, ex) -> { + // Always log errors — a DB failure is worth knowing about even if a + // newer request has already superseded this one. + if (ex != null) { + log.error("Unhandled async exception", ex); + } + // Drop the result if a newer invocation has already claimed the slot. + // The guarded future is intentionally left incomplete; any chained + // .thenAccept() / .thenApply() will simply never fire. + if (generation.get() != stamp) { + return; + } + if (ex != null) { + guarded.completeExceptionally(ex); + } else { + guarded.complete(result); + } + }); + return guarded; + } + + /** + * Submits {@code task} on a virtual thread and returns a guarded future. + * Behaves identically to {@link #supply} but for fire-and-forget tasks + * that produce no value. + * + * @param task the blocking runnable to run off the FX thread + * @return a future that completes only when this invocation is still current + */ + public CompletableFuture run(Runnable task) { + long stamp = generation.incrementAndGet(); + CompletableFuture inner = CompletableFuture.runAsync(task, VIRTUAL); + CompletableFuture guarded = new CompletableFuture<>(); + inner.whenComplete((result, ex) -> { + if (ex != null) { + log.error("Unhandled async exception", ex); + } + if (generation.get() != stamp) { + return; + } + if (ex != null) { + guarded.completeExceptionally(ex); + } else { + guarded.complete(null); + } + }); + return guarded; + } + } + +} diff --git a/owlplug-client/src/main/java/com/owlplug/plugin/controllers/DirectoryInfoController.java b/owlplug-client/src/main/java/com/owlplug/plugin/controllers/DirectoryInfoController.java index a82bd366..bf211593 100644 --- a/owlplug-client/src/main/java/com/owlplug/plugin/controllers/DirectoryInfoController.java +++ b/owlplug-client/src/main/java/com/owlplug/plugin/controllers/DirectoryInfoController.java @@ -22,6 +22,8 @@ import com.owlplug.controls.DialogLayout; import com.owlplug.controls.DoughnutChart; import com.owlplug.core.controllers.BaseController; +import com.owlplug.core.utils.Async; +import com.owlplug.core.utils.FX; import com.owlplug.core.utils.FileUtils; import com.owlplug.core.utils.PlatformUtils; import com.owlplug.core.utils.StringUtils; @@ -64,6 +66,11 @@ public class DirectoryInfoController extends BaseController { @Autowired private FileStatRepository fileStatRepository; + // One sequence per refresh slot: guarantees that only the result of the + // latest refresh() call is applied to the UI, even if an older DB query + // resolves later. + private final Async.Sequence refreshSequence = new Async.Sequence(); + @FXML private Label directoryNameLabel; @FXML @@ -90,6 +97,7 @@ public class DirectoryInfoController extends BaseController { private TableColumn fileSizeColumn; private PieChart pieChart; + private final ObjectProperty pluginDirectoryProperty = new SimpleObjectProperty<>(); /** @@ -168,38 +176,38 @@ protected void layoutChartChildren(double top, double left, double contentWidth, /** * Refresh directory info. - * Most database accesses are performed in this method and expected to be run on - * UI thread to work around a bug with charts display with concurrent updates. */ public void refresh() { PluginDirectory pluginDirectory = pluginDirectoryProperty.get(); + directoryPathTextField.setText(pluginDirectory.getPath()); directoryNameLabel.setText(pluginDirectory.getName()); pluginDirectoryListView.getItems().setAll(pluginDirectory.getPluginList()); + directoryPluginsTab.setText("Plugins (" + pluginDirectory.getPluginList().size() + ")"); directoryMetricsTab.setText("0 KB"); - - File file = new File(pluginDirectory.getPath()); - deleteDirectoryButton.setDisable(!file.canWrite()); + deleteDirectoryButton.setDisable(!new File(pluginDirectory.getPath()).canWrite()); String path = pluginDirectory.getPath(); if (path.endsWith("/")) { path = path.substring(0, path.length() - 1); } - - directoryPluginsTab.setText("Plugins (" + pluginDirectory.getPluginList().size() + ")"); - - Optional directoryStat = fileStatRepository.findByPath(path); - directoryStat.ifPresent(fileStat -> directoryMetricsTab.setText( - FileUtils.humanReadableByteCount(fileStat.getLength(), true))); - - List fileStats = fileStatRepository.findByParentPathOrderByLengthDesc(path); - directoryFilesTab.setText("Files (" + fileStats.size() + ")"); - directoryFilesTableView.setItems(FXCollections.observableArrayList(fileStats)); - pieChart.setData(createStatChartBuckets(fileStats)); - pieChart.layout(); - + final String resolvedPath = path; + + refreshSequence.supply(() -> new FileStatResults( + fileStatRepository.findByPath(resolvedPath), + fileStatRepository.findByParentPathOrderByLengthDesc(resolvedPath) + )).thenAccept(results -> FX.run(() -> { + results.directoryStat().ifPresent(fileStat -> + directoryMetricsTab.setText(FileUtils.humanReadableByteCount(fileStat.getLength(), true))); + directoryFilesTab.setText("Files (" + results.fileStats().size() + ")"); + directoryFilesTableView.setItems(FXCollections.observableArrayList(results.fileStats())); + pieChart.setData(createStatChartBuckets(results.fileStats())); + + })); } + private record FileStatResults(Optional directoryStat, List fileStats) {} + private ObservableList createStatChartBuckets(List fileStats) { ObservableList chartData = FXCollections.observableArrayList(); int i = 0; diff --git a/owlplug-client/src/main/java/com/owlplug/plugin/controllers/PluginInfoController.java b/owlplug-client/src/main/java/com/owlplug/plugin/controllers/PluginInfoController.java index c809a095..72a2d7f5 100644 --- a/owlplug-client/src/main/java/com/owlplug/plugin/controllers/PluginInfoController.java +++ b/owlplug-client/src/main/java/com/owlplug/plugin/controllers/PluginInfoController.java @@ -23,6 +23,7 @@ import com.owlplug.core.components.ApplicationDefaults; import com.owlplug.core.components.ImageCache; import com.owlplug.core.controllers.BaseController; +import com.owlplug.core.utils.Async; import com.owlplug.core.utils.FX; import com.owlplug.core.utils.PlatformUtils; import com.owlplug.plugin.components.PluginTaskFactory; @@ -31,13 +32,14 @@ import com.owlplug.plugin.events.PluginUpdateEvent; import com.owlplug.plugin.model.Plugin; import com.owlplug.plugin.model.PluginComponent; +import com.owlplug.plugin.model.PluginState; import com.owlplug.plugin.services.PluginService; import com.owlplug.plugin.ui.PluginComponentCellFactory; import com.owlplug.plugin.ui.PluginStateView; import java.io.File; import java.util.ArrayList; +import java.util.List; import java.util.Optional; -import java.util.concurrent.CompletableFuture; import javafx.beans.property.ObjectProperty; import javafx.beans.property.SimpleObjectProperty; import javafx.collections.FXCollections; @@ -113,6 +115,10 @@ public class PluginInfoController extends BaseController { private final ObjectProperty pluginProperty = new SimpleObjectProperty(); private final ArrayList knownPluginImages = new ArrayList<>(); + // One sequence per refresh slot: guarantees only the latest selection's data + // reaches the UI, even if a previous DB query resolves out of order. + private final Async.Sequence refreshSequence = new Async.Sequence(); + /** * FXML initialize method. */ @@ -147,7 +153,7 @@ public void initialize() { enableButton.setOnAction(e -> { Plugin plugin = pluginProperty.get(); - CompletableFuture.runAsync(() -> pluginService.enablePlugin(plugin)); + Async.run(() -> pluginService.enablePlugin(plugin)); }); pluginComponentListView.setCellFactory(new PluginComponentCellFactory(this.getApplicationDefaults())); @@ -156,7 +162,7 @@ public void initialize() { Plugin plugin = pluginProperty.get(); if (plugin != null && plugin.getFootprint() != null) { plugin.getFootprint().setNativeDiscoveryEnabled(newValue); - CompletableFuture.runAsync(() -> pluginService.save(plugin.getFootprint())); + Async.run(() -> pluginService.save(plugin.getFootprint())); } }); @@ -168,6 +174,9 @@ public void refresh() { if (plugin == null) { return; } + + // All reads below come from already-loaded in-memory fields — safe to run + // on the FX thread without risk of blocking. pluginFormatIcon.setImage(this.getApplicationDefaults().getPluginFormatIcon(plugin.getFormat())); pluginFormatLabel.setText(plugin.getFormat().getText() + " Plugin"); pluginTitleLabel.setText(plugin.getName()); @@ -176,11 +185,8 @@ public void refresh() { pluginManufacturerLabel.setText(Optional.ofNullable(plugin.getManufacturerName()).orElse("Unknown")); pluginIdentifierLabel.setText(Optional.ofNullable(plugin.getUid()).orElse("Unknown")); pluginCategoryLabel.setText(Optional.ofNullable(plugin.getCategory()).orElse("Unknown")); - pluginStateView.setPluginState(pluginService.getPluginState(plugin)); pluginPathLabel.setText(plugin.getPath()); - - File file = new File(plugin.getPath()); - this.uninstallButton.setDisable(!file.canWrite()); + uninstallButton.setDisable(!new File(plugin.getPath()).canWrite()); if (plugin.isDisabled()) { enableButton.setManaged(true); @@ -198,12 +204,23 @@ public void refresh() { nativeDiscoveryToggleButton.setSelected(plugin.getFootprint().isNativeDiscoveryEnabled()); } - ObservableList components = FXCollections.observableList(new ArrayList(plugin.getComponents())); - pluginComponentListView.setItems(components); - setPluginImage(); + + // getPluginState may hit the database, and getComponents may trigger a + // lazy JPA load — offload both to a virtual thread. The sequence ensures + // that if the user selects another plugin before this resolves, the + // stale result is silently dropped. + refreshSequence.supply(() -> new PluginRefreshData( + pluginService.getPluginState(plugin), + new ArrayList<>(plugin.getComponents()) + )).thenAccept(data -> FX.run(() -> { + pluginStateView.setPluginState(data.state()); + pluginComponentListView.setItems(FXCollections.observableList(data.components())); + })); } + private record PluginRefreshData(PluginState state, List components) {} + private void setPluginImage() { Plugin plugin = pluginProperty.get(); String url = plugin.getScreenshotUrl(); diff --git a/owlplug-client/src/main/java/com/owlplug/plugin/controllers/PluginTableController.java b/owlplug-client/src/main/java/com/owlplug/plugin/controllers/PluginTableController.java index a659ae39..16172886 100644 --- a/owlplug-client/src/main/java/com/owlplug/plugin/controllers/PluginTableController.java +++ b/owlplug-client/src/main/java/com/owlplug/plugin/controllers/PluginTableController.java @@ -29,7 +29,7 @@ import com.owlplug.plugin.services.PluginService; import com.owlplug.plugin.ui.PluginStateView; import java.io.File; -import java.util.concurrent.CompletableFuture; +import com.owlplug.core.utils.Async; import javafx.beans.binding.Bindings; import javafx.beans.property.SimpleObjectProperty; import javafx.beans.property.SimpleStringProperty; @@ -231,7 +231,7 @@ private ContextMenu createPluginContextMenu(Plugin plugin) { if (plugin.isDisabled()) { MenuItem enableItem = new MenuItem("Enable plugin"); enableItem.setOnAction(e -> { - CompletableFuture.runAsync(() -> pluginService.enablePlugin(plugin)); + Async.run(() -> pluginService.enablePlugin(plugin)); }); menu.getItems().add(enableItem); } else { diff --git a/owlplug-client/src/main/java/com/owlplug/plugin/controllers/PluginsController.java b/owlplug-client/src/main/java/com/owlplug/plugin/controllers/PluginsController.java index 3c62c1a8..2a8ae9cf 100644 --- a/owlplug-client/src/main/java/com/owlplug/plugin/controllers/PluginsController.java +++ b/owlplug-client/src/main/java/com/owlplug/plugin/controllers/PluginsController.java @@ -30,7 +30,7 @@ import com.owlplug.plugin.model.Plugin; import com.owlplug.plugin.repositories.PluginRepository; import com.owlplug.plugin.services.PluginService; -import java.util.concurrent.CompletableFuture; +import com.owlplug.core.utils.Async; import javafx.fxml.FXML; import javafx.scene.control.Button; import javafx.scene.control.MenuItem; @@ -206,7 +206,7 @@ public void initialize() { } public void displayPlugins() { - CompletableFuture.supplyAsync(() -> pluginRepository.findAll()) + Async.supply(() -> pluginRepository.findAll()) .thenAccept(plugins -> FX.run(() -> { treeViewController.setPlugins(plugins); tableController.setPlugins(plugins); diff --git a/owlplug-client/src/main/java/com/owlplug/plugin/controllers/SymlinkInfoController.java b/owlplug-client/src/main/java/com/owlplug/plugin/controllers/SymlinkInfoController.java index 7635f40c..bafccd06 100644 --- a/owlplug-client/src/main/java/com/owlplug/plugin/controllers/SymlinkInfoController.java +++ b/owlplug-client/src/main/java/com/owlplug/plugin/controllers/SymlinkInfoController.java @@ -21,6 +21,8 @@ import com.owlplug.controls.Dialog; import com.owlplug.controls.DialogLayout; import com.owlplug.core.controllers.BaseController; +import com.owlplug.core.utils.Async; +import com.owlplug.core.utils.FX; import com.owlplug.core.utils.PlatformUtils; import com.owlplug.plugin.components.PluginTaskFactory; import com.owlplug.plugin.model.Plugin; @@ -59,6 +61,10 @@ public class SymlinkInfoController extends BaseController { private final ObjectProperty symlinkProperty = new SimpleObjectProperty<>(); + // One sequence per refresh slot: guarantees only the latest symlink's plugin + // list is applied to the UI, even if a previous lazy-load resolves out of order. + private final Async.Sequence refreshSequence = new Async.Sequence(); + /** * FXML Initialize. */ @@ -107,10 +113,15 @@ public void initialize() { public void refresh() { Symlink symlink = symlinkProperty.get(); + + // Simple field reads — in-memory, safe on the FX thread. directoryPathLabel.setText(symlink.getPath()); - pluginDirectoryListView.getItems().setAll(symlink.getPluginList()); targetPathLabel.setText(Optional.ofNullable(symlink.getTargetPath()).orElse("Unknown")); openTargetButton.setVisible(symlink.getTargetPath() != null); + + // getPluginList may trigger a lazy JPA load — offload to a virtual thread. + refreshSequence.supply(symlink::getPluginList) + .thenAccept(plugins -> FX.run(() -> pluginDirectoryListView.getItems().setAll(plugins))); } public ObjectProperty symlinkProperty() { diff --git a/owlplug-client/src/main/java/com/owlplug/plugin/ui/RecoveredPluginView.java b/owlplug-client/src/main/java/com/owlplug/plugin/ui/RecoveredPluginView.java index ac21b740..15ea87a3 100644 --- a/owlplug-client/src/main/java/com/owlplug/plugin/ui/RecoveredPluginView.java +++ b/owlplug-client/src/main/java/com/owlplug/plugin/ui/RecoveredPluginView.java @@ -21,7 +21,7 @@ import com.owlplug.core.components.ApplicationDefaults; import com.owlplug.plugin.model.Plugin; import com.owlplug.plugin.services.PluginService; -import java.util.concurrent.CompletableFuture; +import com.owlplug.core.utils.Async; import javafx.geometry.Pos; import javafx.scene.control.Label; import javafx.scene.image.ImageView; @@ -61,7 +61,7 @@ public RecoveredPluginView(Plugin plugin, PluginService pluginService, Applicati toggleSwitch.setSelected(plugin.getFootprint().isNativeDiscoveryEnabled()); toggleSwitch.selectedProperty().addListener((observable, oldValue, newValue) -> { plugin.getFootprint().setNativeDiscoveryEnabled(newValue); - CompletableFuture.runAsync(() -> pluginService.save(plugin.getFootprint())); + Async.run(() -> pluginService.save(plugin.getFootprint())); }); this.getChildren().add(toggleSwitch);