diff --git a/HMCL/src/main/java/org/jackhuang/hmcl/ui/download/DownloadPage.java b/HMCL/src/main/java/org/jackhuang/hmcl/ui/download/DownloadPage.java index cf5c19a9a7..e96cc90cc2 100644 --- a/HMCL/src/main/java/org/jackhuang/hmcl/ui/download/DownloadPage.java +++ b/HMCL/src/main/java/org/jackhuang/hmcl/ui/download/DownloadPage.java @@ -307,7 +307,7 @@ public class DownloadPage extends DecoratorAnimatedPage implements DecoratorPage /// @param settings the installer selections and target instance id /// @return the builder task with its stage hints preserved as the outermost task wrapper private Task finishVersionDownloadingAsync(SettingsMap settings) { - GameInstanceID instanceId = settings.get(AbstractInstallersPage.INSTANCE_ID); + @Nullable GameInstanceID instanceId = settings.get(AbstractInstallersPage.INSTANCE_ID); if (instanceId == null) { throw new IllegalStateException("Instance ID is not set"); } @@ -329,6 +329,7 @@ public class DownloadPage extends DecoratorAnimatedPage implements DecoratorPage } Task buildTask = builder.buildAsync(); + settings.put(TaskCleanup.KEY, builder::abort); buildTask.onDone().register(event -> { if (!event.isFailed()) { runInFX(() -> repository.setSelectedInstance(repository.getInstance(instanceId))); diff --git a/HMCL/src/main/java/org/jackhuang/hmcl/ui/wizard/TaskExecutorDialogWizardDisplayer.java b/HMCL/src/main/java/org/jackhuang/hmcl/ui/wizard/TaskExecutorDialogWizardDisplayer.java index ccba6dd81f..5e2e6beb61 100644 --- a/HMCL/src/main/java/org/jackhuang/hmcl/ui/wizard/TaskExecutorDialogWizardDisplayer.java +++ b/HMCL/src/main/java/org/jackhuang/hmcl/ui/wizard/TaskExecutorDialogWizardDisplayer.java @@ -28,6 +28,7 @@ import org.jackhuang.hmcl.ui.construct.TaskExecutorDialogPane; import org.jackhuang.hmcl.util.SettingsMap; import org.jackhuang.hmcl.util.StringUtils; import org.jackhuang.hmcl.util.TaskCancellationAction; +import org.jetbrains.annotations.Nullable; import java.util.Queue; import java.util.concurrent.CancellationException; @@ -41,9 +42,16 @@ public abstract class TaskExecutorDialogWizardDisplayer extends AbstractWizardDi super(cancelQueue); } + /// Displays a task and releases its optional resources after execution stops. @Override public void handleTask(SettingsMap settings, Task task) { + @Nullable WizardProvider.TaskCleanup cleanup = settings.remove(WizardProvider.TaskCleanup.KEY); TaskExecutorDialogPane pane = new TaskExecutorDialogPane(new TaskCancellationAction(it -> { + if (cleanup != null) { + // Keep the installation modal until onStop has released its repository draft. + it.setCancel(null); + return; + } it.fireEvent(new DialogCloseEvent()); onEnd(); })); @@ -61,24 +69,37 @@ public abstract class TaskExecutorDialogWizardDisplayer extends AbstractWizardDi TaskExecutor executor = task.executor(new TaskListener() { @Override public void onStop(boolean success, TaskExecutor executor) { + @Nullable Exception failure = success ? null : executor.getException(); + if (cleanup != null) { + try { + cleanup.cleanup(); + } catch (Exception cleanupFailure) { + if (failure == null || failure instanceof CancellationException) { + failure = cleanupFailure; + } else if (failure != cleanupFailure) { + failure.addSuppressed(cleanupFailure); + } + } + } + @Nullable Exception completedFailure = failure; runInFX(() -> { - if (success) { + if (success && completedFailure == null) { if (settings.get("success_message") instanceof String successMessage) Controllers.dialog(successMessage, null, MessageType.SUCCESS, () -> onEnd()); else if (!settings.containsKey("forbid_success_message")) Controllers.dialog(i18n("message.success"), null, MessageType.SUCCESS, () -> onEnd()); } else { - if (executor.getException() == null) { + if (completedFailure == null) { onEnd(); return; } - if (executor.getException() instanceof CancellationException) { + if (completedFailure instanceof CancellationException) { onEnd(); return; } - if (executor.getException().getCause() instanceof OutOfMemoryError outOfMemoryError) { + if (completedFailure.getCause() instanceof OutOfMemoryError outOfMemoryError) { try { Controllers.dialog(StringUtils.getStackTrace(outOfMemoryError), null, MessageType.ERROR, () -> onEnd()); } catch (OutOfMemoryError ignored) { @@ -87,9 +108,9 @@ public abstract class TaskExecutorDialogWizardDisplayer extends AbstractWizardDi return; } - String appendix = StringUtils.getStackTrace(executor.getException()); + String appendix = StringUtils.getStackTrace(completedFailure); if (settings.get(WizardProvider.FailureCallback.KEY) != null) - settings.get(WizardProvider.FailureCallback.KEY).onFail(settings, executor.getException(), () -> onEnd()); + settings.get(WizardProvider.FailureCallback.KEY).onFail(settings, completedFailure, () -> onEnd()); else if (settings.get("failure_message") instanceof String failureMessage) Controllers.dialog(appendix, failureMessage, MessageType.ERROR, () -> onEnd()); else if (!settings.containsKey("forbid_failure_message")) diff --git a/HMCL/src/main/java/org/jackhuang/hmcl/ui/wizard/WizardProvider.java b/HMCL/src/main/java/org/jackhuang/hmcl/ui/wizard/WizardProvider.java index 1c077ca7e3..66dfa63815 100644 --- a/HMCL/src/main/java/org/jackhuang/hmcl/ui/wizard/WizardProvider.java +++ b/HMCL/src/main/java/org/jackhuang/hmcl/ui/wizard/WizardProvider.java @@ -19,6 +19,7 @@ package org.jackhuang.hmcl.ui.wizard; import javafx.scene.Node; import org.jackhuang.hmcl.util.SettingsMap; +import org.jetbrains.annotations.NotNullByDefault; public interface WizardProvider { @@ -39,4 +40,20 @@ public interface WizardProvider { void onFail(SettingsMap settings, Exception exception, Runnable next); } + + /// Releases resources owned by a task displayed by [TaskExecutorDialogWizardDisplayer]. + /// + /// Cleanup runs on the executor's completion thread after task execution stops, including + /// cancellation before the first task starts, and before the dialog reports its outcome. + @NotNullByDefault + @FunctionalInterface + interface TaskCleanup { + /// Optional cleanup consumed by the task dialog when it accepts the task. + SettingsMap.Key KEY = new SettingsMap.Key<>("task_cleanup"); + + /// Releases the completed task's resources. + /// + /// @throws Exception if cleanup fails; the task dialog reports the failure + void cleanup() throws Exception; + } } diff --git a/HMCLCore/src/main/java/org/jackhuang/hmcl/download/DefaultGameBuilder.java b/HMCLCore/src/main/java/org/jackhuang/hmcl/download/DefaultGameBuilder.java index 404a43a73a..517031de9e 100644 --- a/HMCLCore/src/main/java/org/jackhuang/hmcl/download/DefaultGameBuilder.java +++ b/HMCLCore/src/main/java/org/jackhuang/hmcl/download/DefaultGameBuilder.java @@ -207,6 +207,22 @@ public class DefaultGameBuilder extends GameBuilder { } } + /// Aborts this builder's open draft, including after ownership has passed to its build task. + /// + /// If [#buildAsync()] has returned a task, the caller must ensure its executor has stopped + /// before invoking this method. This operation does not cancel or wait for task execution. + /// It has no effect if the draft has already committed or aborted, and does not undo a + /// committed installation. Subsequent calls have no effect after cleanup has completed or + /// failed. Shared download caches are retained. + /// + /// @throws IOException if reserved instance files cannot be removed; draft exclusivity is + /// released even in this case + public void abort() throws IOException { + if (draft.isOpen()) { + draft.abort(); + } + } + /// Performs repository-specific initialization after an instance is committed. /// /// This method is invoked after the repository has published the committed snapshot and before diff --git a/HMCLCore/src/test/java/org/jackhuang/hmcl/download/DefaultGameBuilderCancellationTest.java b/HMCLCore/src/test/java/org/jackhuang/hmcl/download/DefaultGameBuilderCancellationTest.java new file mode 100644 index 0000000000..27c109ae8e --- /dev/null +++ b/HMCLCore/src/test/java/org/jackhuang/hmcl/download/DefaultGameBuilderCancellationTest.java @@ -0,0 +1,176 @@ +/* + * Hello Minecraft! Launcher + * Copyright (C) 2026 huangyuhui and contributors + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation, either version 3 of the License, or + * (at your option) any later version. + * + * This program 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 this program. If not, see . + */ +package org.jackhuang.hmcl.download; + +import org.jackhuang.hmcl.game.*; +import org.jackhuang.hmcl.task.Task; +import org.jackhuang.hmcl.task.TaskExecutor; +import org.jackhuang.hmcl.task.TaskListener; +import org.jetbrains.annotations.NotNullByDefault; +import org.jetbrains.annotations.Nullable; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.io.TempDir; + +import java.io.IOException; +import java.nio.file.Path; +import java.util.concurrent.CountDownLatch; +import java.util.concurrent.TimeUnit; +import java.util.concurrent.atomic.AtomicBoolean; +import java.util.concurrent.atomic.AtomicReference; + +import static org.junit.jupiter.api.Assertions.*; + +/// Verifies that cancelled builds retain exclusivity only until their cleanup completes. +@NotNullByDefault +public final class DefaultGameBuilderCancellationTest { + + /// Explicit cleanup after an early cancellation releases a draft skipped by the task chain. + @Test + public void testAbortAfterCancelBeforeBuildStarts(@TempDir Path directory) throws Exception { + verifyCancellation(directory, false); + } + + /// Cancellation waits for active component work before releasing the draft. + @Test + public void testAbortAfterCancelDuringComponentInstallation(@TempDir Path directory) throws Exception { + verifyCancellation(directory, true); + } + + /// Runs an installation with a gate at the chosen cancellation point. + private static void verifyCancellation(Path directory, boolean duringInstallation) throws Exception { + TestRepository repository = new TestRepository(directory); + CountDownLatch ready = new CountDownLatch(1); + CountDownLatch release = new CountDownLatch(1); + CountDownLatch stopped = new CountDownLatch(1); + AtomicBoolean componentStarted = new AtomicBoolean(); + TestDependencyManager manager = new TestDependencyManager( + repository, ready, release, componentStarted); + AtomicReference<@Nullable IOException> cleanupFailure = new AtomicReference<>(); + DefaultGameBuilder builder = manager.newGameBuilder(new GameInstanceID("first")); + Task task; + try (builder) { + builder.component(GameComponentType.GAME, "1.21.1"); + task = builder.buildAsync(); + } + TaskExecutor executor = task.executor(new TaskListener() { + /// Holds the outer stage wrapper before its cleanup task can be scheduled. + @Override + public void onReady(Task readyTask) { + if (!duringInstallation && readyTask == task) { + ready.countDown(); + await(release); + } + } + + /// Releases the draft only after the executor stops, as the installation dialog does. + @Override + public void onStop(boolean success, TaskExecutor completedExecutor) { + try { + builder.abort(); + builder.abort(); + } catch (IOException exception) { + cleanupFailure.set(exception); + } finally { + stopped.countDown(); + } + } + }); + executor.start(); + try { + await(ready); + executor.cancel(); + assertThrows(IllegalStateException.class, repository::openDraft); + assertEquals(1L, stopped.getCount()); + } finally { + release.countDown(); + } + await(stopped); + + assertNull(cleanupFailure.get()); + assertEquals(duringInstallation, componentStarted.get()); + assertFalse(repository.hasInstance(new GameInstanceID("first"))); + try (DefaultGameBuilder ignored = manager.newGameBuilder(new GameInstanceID("second"))) { + assertThrows(IllegalStateException.class, repository::openDraft); + } + } + + /// Waits for a deterministic test gate and fails instead of hanging indefinitely. + private static void await(CountDownLatch latch) { + try { + assertTrue(latch.await(10, TimeUnit.SECONDS), "Task lifecycle timed out"); + } catch (InterruptedException exception) { + Thread.currentThread().interrupt(); + throw new AssertionError(exception); + } + } + + /// Provides an empty repository for builds that never publish an instance. + @NotNullByDefault + private static final class TestRepository extends DefaultGameRepository { + /// Creates a repository rooted at the test directory. + private TestRepository(Path directory) { + super(directory); + } + + /// {@inheritDoc} + @Override + protected DefaultGameRepositoryLayout createLayout(Path directory) { + return new DefaultGameRepositoryLayout(directory); + } + + /// Rejects publication because every test cancels before an instance is installed. + @Override + protected DefaultGameInstance createInstance(DefaultGameRepositorySnapshot snapshot, + GameInstanceID id, GameInstanceManifest manifest, @Nullable Path manifestFile) { + throw new AssertionError("Cancelled installation must not publish an instance"); + } + } + + /// Replaces network installation with a controlled local task. + @NotNullByDefault + private static final class TestDependencyManager extends DefaultDependencyManager { + /// Signals when component installation begins. + private final CountDownLatch ready; + /// Allows component installation to return after cancellation. + private final CountDownLatch release; + /// Records whether component installation ran. + private final AtomicBoolean componentStarted; + + /// Creates a manager with an isolated cache and no remote requests. + private TestDependencyManager(TestRepository repository, CountDownLatch ready, + CountDownLatch release, AtomicBoolean componentStarted) { + super(repository, new MojangDownloadProvider(), + new DefaultCacheRepository(repository.getBaseDirectory().resolve("cache"))); + this.ready = ready; + this.release = release; + this.componentStarted = componentStarted; + } + + /// Returns a task that waits for cancellation before reporting a local installation failure. + @Override + Task installUnpublishedComponentAsync(GameInstanceManifest manifest, + Path modsDirectory, String gameVersion, GameComponentType componentType, String version) { + return Task.supplyAsync(() -> { + componentStarted.set(true); + ready.countDown(); + await(release); + throw new IOException("Simulated component installation failure"); + }); + } + } +}