fix(core): 修复异步任务与资源生命周期#704
Conversation
确保 Future 异常可观察,消除递归删除死锁并可靠关闭文件监听与加载资源。
FxRayHughes
left a comment
There was a problem hiding this comment.
Code Review:PR #704 fix(core): 修复异步任务与资源生命周期
感谢提交!整体方向正确,解决了真实问题,测试覆盖也有明显改善。但有两个严重问题需要在合并前修复,另有几处中等问题需要处理。
🔴 严重问题(阻止合并)
1. FileWatcher.addSimpleListener — WatchKey 共享导致监听器相互取消
文件: common-legacy-api/src/main/java/taboolib/common5/FileWatcher.java
JDK 规范(并经过实测验证):对同一目录重复调用 path.register(watchService, ...) 会返回同一个 WatchKey 实例。
PR 当前代码:
FileListener listener = new FileListener(canonicalFile, runnable, this);
FileListener previous = fileListenerMap.put(canonicalFile, listener);
if (previous != null) {
previous.cancel(); // ← 取消的是 previous.watchKey
}FileListener 构造函数中 watchKey = path.register(...) 拿到的是与 previous.watchKey 相同的对象。previous.cancel() 执行后,新监听器的 key 也随之失效,新监听器永远收不到事件。
同一问题还连带影响:
removeListener:取消的 key 是目录级共享的,会同时使同目录所有其他文件的监听器失效- 轮询循环中
!key.reset()的清理逻辑:key 失效时会把整个目录组的监听器一起移出 map
注意:原始代码直接覆盖 map entry 而不调用 cancel,反而是安全的。
建议修复: 在 cancel 前判断 previous.watchKey !== listener.watchKey(即不同目录/文件),或引入目录级引用计数(最后一个监听器离开该目录时才 cancel key)。
2. deepDeleteAsync — 遇到失败从静默跳过变为中止整棵树
文件: common-util/src/main/kotlin/taboolib/common/io/FileDeleteAsync.kt
SimpleFileVisitor 的默认 visitFileFailed 实现会重抛 IOException。新的 deleteTree 没有覆盖它,因此一旦遇到被锁住的文件(Windows 热重载场景极为常见),整棵目录树遍历立即中止,后续所有文件不再删除。
而 PR 的修复目标之一正是"Windows 热重载文件句柄泄漏",这个场景需要的是"已关闭的流 → 文件可删"的逻辑,而非"遇到锁定文件就崩"。
建议修复:
override fun visitFileFailed(file: Path, exc: IOException): FileVisitResult {
// 可选:记录日志
return FileVisitResult.CONTINUE // 跳过失败文件,继续遍历
}🟡 中等问题
3. deepDeleteAsync — executor 从非 daemon 线程变为 ForkJoinPool daemon 线程
旧实现的固定线程池使用非 daemon 线程,JVM 退出时会等待删除完成。CompletableFuture.runAsync 默认使用 ForkJoinPool.commonPool(),其线程是 daemon 线程,JVM 可能在删除完成前退出。插件卸载(服务器关闭期间触发深度删除)场景有数据丢失风险。
4. FileWatcherTest — macOS 上路径断言不稳定
文件: common-legacy-api/src/test/kotlin/taboolib/common5/FileWatcherTest.kt
在 macOS 上 File.createTempFile / @TempDir 创建的路径存在符号链接:
absolutePath=/var/folders/.../watched.txtcanonicalPath=/private/var/folders/.../watched.txt
两者 equals() 返回 false。测试断言 changed.absoluteFile == file.absoluteFile,而 callback 中收到的路径是经过 .toAbsolutePath().normalize() 的结果,在 macOS 上与原始 absoluteFile 字符串不同,导致 latch 永远不会 countDown,测试超时失败。
建议: 改用 Files.isSameFile(changed.toPath(), file.toPath()) 或统一先取 canonicalFile 再比较。
5. FileWatcherTest — 在 finally 中对全局单例调用 release()
FileWatcher.INSTANCE.release() // ← 全局单例永久失效INSTANCE 是 JVM 级静态对象,released 标志一旦置为 true 就不可逆,后续所有使用 FileWatcher.INSTANCE 的代码(同 JVM 内其他测试)全部失效。建议删除这一行,或在测试中只使用局部创建的 FileWatcher 实例。
🟢 确认正确的改动
| 改动 | 评价 |
|---|---|
SyncExecutor.completeWith |
正确修复,Future.join() 永久 pending 问题彻底解决 |
AetherResolver 失败后 remove id;add() 原子操作 |
修复了原有的 check-then-add 竞态,允许失败后重试 |
PrimitiveIO.downloadFile try-with-resources |
正确,流泄漏修复 |
PrimitiveLoader.shouldRelocate:!exists || length==0 |
语义比原来的 !exists && length==0(等价于 !exists)更准确 |
random(num1, num2) 改用 Long 避免 Int.MAX_VALUE + 1 溢出 |
真实 bug 修复 |
random(v: Double) 从 <= 改为 < |
正确,v=0.0 应永远返回 false |
Throttle.canExecute 改用 compute 原子操作 |
修复了 ConcurrentHashMap read-modify-write 竞态 |
Throttle.Singleton 改用 AtomicLong + CAS |
Long.MIN_VALUE 哨兵保证首次永远放行,设计合理 |
FileWatcher.release() 幂等(AtomicBoolean) |
正确 |
FileWatcher watchService 为 null 时 shutdownNow |
修复了线程泄漏 |
PrimitiveLoader temp 文件 finally 清理 |
正确 |
🔵 小建议
Throttle.Singleton.clearAll() / removeKey() 无法重置状态: Singleton 的实际状态存储在 lastExecuteTime(AtomicLong),但继承来的 clearAll() 和 removeKey() 操作的是父类 throttleMap(Singleton 从不写入它),调用这两个方法对 Singleton 毫无效果。建议 override 这两个方法或加注释说明。
原有问题
核心异步与资源工具中存在多条“任务已经失败,但调用方永远等不到终态”的路径:
SyncExecutor包装的平台任务一旦抛出异常,异常只会逃逸到调度器,返回的CompletableFuture不会成功也不会异常完成。Throttle的首次调用、零延迟参数以及完整Int随机区间存在边界计算错误。典型触发场景与后果
本 PR 修改
completeExceptionally收敛;成功、失败和取消都只能完成一次。Int随机区间计算。修改目的
确保基础工具在成功、异常、取消和关闭路径上都能得到确定终态,避免永久等待、线程泄漏、文件句柄泄漏和边界输入错误向上层模块扩散。
兼容性与行为变化
deepDeleteAsync的futures集合记录整棵树对应的根任务,不再暴露内部递归子任务。验证
./gradlew :common:build :common-env:build :common-util:build :common-platform-api:build :common-legacy-api:build --rerun-tasks --no-parallel./gradlew :common:test :common-env:build :common-util:test :common-platform-api:test :common-legacy-api:test --rerun-tasks --no-parallelgit diff --checkRefs #703