Skip to content
Open
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
29 changes: 0 additions & 29 deletions HMCL/src/main/java/org/jackhuang/hmcl/ui/FXUtils.java
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,6 @@
import javafx.util.Callback;
import javafx.util.Duration;
import javafx.util.StringConverter;
import org.glavo.url.WebURL;
import org.jackhuang.hmcl.setting.StyleSheets;
import org.jackhuang.hmcl.task.CacheFileTask;
import org.jackhuang.hmcl.task.Schedulers;
Expand Down Expand Up @@ -100,10 +99,8 @@
import java.lang.invoke.MethodHandles;
import java.lang.invoke.MethodType;
import java.lang.ref.WeakReference;
import java.net.HttpURLConnection;
import java.net.URI;
import java.net.URISyntaxException;
import java.net.URLConnection;
import java.nio.file.FileSystems;
import java.nio.file.Files;
import java.nio.file.Path;
Expand Down Expand Up @@ -1167,32 +1164,6 @@ public static Image loadImage(Path path,
}
}

public static Image loadImage(WebURL url) throws Exception {
URLConnection connection = NetworkUtils.createConnection(url);
if (connection instanceof HttpURLConnection httpConnection)
connection = NetworkUtils.resolveConnection(httpConnection);

try (BufferedInputStream input = new BufferedInputStream(connection.getInputStream())) {
String contentType = Objects.requireNonNull(connection.getContentType(), "");
Matcher matcher = ImageUtils.CONTENT_TYPE_PATTERN.matcher(contentType);
if (matcher.find())
contentType = matcher.group("type");

ImageLoader loader = ImageUtils.CONTENT_TYPE_TO_LOADER.get(contentType);
if (loader == null && !ImageUtils.DEFAULT_CONTENT_TYPES.contains(contentType)) {
input.mark(ImageUtils.HEADER_BUFFER_SIZE);
byte[] headerBuffer = input.readNBytes(ImageUtils.HEADER_BUFFER_SIZE);
input.reset();
loader = ImageUtils.guessLoader(headerBuffer);
}

if (loader == null)
loader = ImageUtils.DEFAULT;

return loader.load(input, 0, 0, false, false);
}
}

/**
* Suppress IllegalArgumentException since the url is supposed to be correct definitely.
*
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -34,10 +34,10 @@
import javafx.scene.paint.Paint;
import javafx.stage.Stage;
import javafx.util.Duration;
import org.glavo.url.WebURL;
import org.jackhuang.hmcl.Metadata;
import org.jackhuang.hmcl.auth.authlibinjector.AuthlibInjectorDnD;
import org.jackhuang.hmcl.setting.EnumBackgroundImage;
import org.jackhuang.hmcl.task.CacheFileTask;
import org.jackhuang.hmcl.task.Schedulers;
import org.jackhuang.hmcl.task.Task;
import org.jackhuang.hmcl.ui.Controllers;
Expand All @@ -52,7 +52,9 @@
import org.jackhuang.hmcl.ui.wizard.Refreshable;
import org.jackhuang.hmcl.ui.wizard.WizardProvider;
import org.jackhuang.hmcl.util.MathUtils;
import org.jackhuang.hmcl.util.io.FileUtils;
import org.jackhuang.hmcl.util.platform.OperatingSystem;
import org.jetbrains.annotations.NotNull;
import org.jetbrains.annotations.Nullable;

import java.io.IOException;
Expand All @@ -71,6 +73,8 @@
import static org.jackhuang.hmcl.util.logging.Logger.LOG;

public class DecoratorController {
private static final Path remoteBgCachePath = Metadata.HMCL_CURRENT_DIRECTORY.resolve("bg.png");
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

high

remoteBgCachePath 被硬编码为程序根目录下的 bg.png。这存在以下问题:

  1. 缓存逻辑错误:由于缓存文件名固定且未与 backgroundImageUrl 关联,当用户更改背景图 URL 时,getBackground (第 217 行) 会直接加载并显示旧 URL 的缓存图片,直到新图片下载完成。这会导致界面短暂显示错误的背景图。
  2. 文件规范:建议将缓存文件存放在专门的缓存目录(如 cache 子目录)中,并根据 URL 生成唯一的文件名(例如使用 URL 的哈希值)。
  3. 格式兼容性:硬编码 .png 后缀可能与实际下载的图片格式(如 JPG, WebP, SVG)不符。虽然 JavaFX 能够识别流内容,但错误的后缀可能在某些环境下引起问题。


private final Decorator decorator;
private final Navigator navigator;

Expand Down Expand Up @@ -169,13 +173,15 @@ public Decorator getDecorator() {
@SuppressWarnings("FieldCanBeLocal") // Strong reference
private final InvalidationListener changeBackgroundListener;

private volatile boolean remoteFetched = false;

private void updateBackground() {
final int currentCount = ++this.changeBackgroundCount;
Task.supplyAsync(Schedulers.io(), this::getBackground)
.setName("Update background")
.whenComplete(Schedulers.javafx(), (background, exception) -> {
if (exception == null) {
if (this.changeBackgroundCount == currentCount)
if (this.changeBackgroundCount == currentCount && !remoteFetched)
decorator.setContentBackground(background);
} else {
LOG.warning("Failed to update background", exception);
Expand All @@ -184,6 +190,7 @@ private void updateBackground() {
}

private Background getBackground() {
remoteFetched = false;
EnumBackgroundImage imageType = config().getBackgroundImageType();
if (imageType == null)
imageType = EnumBackgroundImage.DEFAULT;
Expand All @@ -206,7 +213,8 @@ private Background getBackground() {
String backgroundImageUrl = config().getBackgroundImageUrl();
if (backgroundImageUrl != null) {
try {
image = FXUtils.loadImage(WebURL.parseBrowserInput(backgroundImageUrl));
asyncFetchRemoteImage(backgroundImageUrl);
image = tryLoadImage(remoteBgCachePath);
} catch (Exception e) {
LOG.warning("Couldn't load background image", e);
}
Expand Down Expand Up @@ -338,6 +346,26 @@ private Image loadDefaultBackgroundImage() {
}
}

private void asyncFetchRemoteImage(@NotNull String backgroundImageUrl) {
final int currentCount = this.changeBackgroundCount;
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

high

此处存在多线程数据竞争(Data Race)。changeBackgroundCount 是一个普通的 int 字段,它在 JavaFX 线程中被 updateBackground 修改,但在此处(IO 线程)被读取。同样的问题也存在于第 354 行的异步回调中。

这可能导致 currentCount 获取到过期的值,从而在快速切换背景时导致逻辑错误(例如将旧请求的图片错误地应用或保存到缓存中)。

建议

  1. changeBackgroundCount 声明为 AtomicInteger
  2. 或者将 currentCount 作为参数从 updateBackground 传递给 getBackgroundasyncFetchRemoteImage,确保每个异步任务都持有其启动时的快照值。

new CacheFileTask(backgroundImageUrl)
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

请不要使用 CacheFileTaskCacheFileTask 会在设备上全局共享一些游戏相关的内容,而背景图更适合在局部进行缓存。把它缓存到全局文件夹的话,如果用户使用类似 Bing 壁纸这样的随机图片链接,那么可能会导致无用缓存越来越多。

.setExecutor(Schedulers.io())
.thenApplyAsync(Schedulers.io(), path -> {
if (this.changeBackgroundCount == currentCount) FileUtils.copyFile(path, remoteBgCachePath);
return FXUtils.loadImage(path);
})
.whenComplete(Schedulers.javafx(), ((image, exception) -> {
if (exception == null) {
if (this.changeBackgroundCount == currentCount) {
decorator.setContentBackground(createBackgroundWithOpacity(image, config().getBackgroundImageOpacity()));
remoteFetched = true;
}
} else {
LOG.warning("Failed to load network background image from " + backgroundImageUrl, exception);
}
})).start();
}

// ==== Navigation ====

public void navigate(Node node, AnimationProducer animationProducer, Duration duration, Interpolator interpolator) {
Expand Down