Skip to content

fix(platform): 修复生命周期终态与任务清理#715

Open
zhibeigg wants to merge 1 commit into
TabooLib:dev/6.3.0from
zhibeigg:fix/703-platform-lifecycle-cleanup
Open

fix(platform): 修复生命周期终态与任务清理#715
zhibeigg wants to merge 1 commit into
TabooLib:dev/6.3.0from
zhibeigg:fix/703-platform-lifecycle-cleanup

Conversation

@zhibeigg

@zhibeigg zhibeigg commented Jul 13, 2026

Copy link
Copy Markdown
Contributor

原有问题

平台生命周期和执行器缺少明确的终态,异步启动、关闭与任务句柄绑定之间可能发生倒退或遗漏清理:

  • Application 未完整触发 ACTIVE,关闭期间仍可能收到迟到的启用回调,生命周期从 DISABLE 倒退到 ENABLE/ACTIVE。
  • Application、Velocity、AfyBroker 执行器在停止后仍可能接收任务,延迟绑定的句柄和异常也可能丢失。
  • Velocity 异步事件/关闭流程没有统一的 Future 终态,监听器失败时调用方可能无法观察。
  • Bukkit 命令注销只删除部分映射,主名、别名、namespace 或内部绑定可能残留。
  • Application 命令递归注销、控制台关闭和资源流/URI 处理存在遗漏。

典型触发场景与后果

  • 插件快速启用后立即禁用,或启动 Future 在关闭后才完成:已进入 DISABLE 的插件再次触发 ENABLE/ACTIVE,清理后的资源被重新使用。
  • 插件禁用同时提交延迟/重复任务:任务在停止后继续执行,线程池无法关闭或访问已释放对象。
  • Velocity 事件监听器异步失败:事件 Future 永久等待,关闭流程被卡住且异常被吞掉。
  • reload 后以相同主名或 alias 重新注册命令:旧映射仍指向失效插件,出现命令冲突、重复执行或无法重注册。

本 PR 修改

  • 补齐 Application 的 ACTIVE 生命周期,并以单向终态状态机阻止关闭期间发生生命周期倒退;停止标记下仍允许执行 DISABLE 清理。
  • 为 Application、Velocity、AfyBroker 执行器增加停止终态、幂等取消、延迟句柄绑定、停止后拒绝、异常报告和线程池关闭。
  • 使用 Velocity EventTask 串联异步关闭,保留旧同步入口,并为自定义事件增加 callAsync() 以暴露完成和异常状态。
  • PluginCommand 身份清理 Bukkit 主名、别名、namespace 及内部绑定,同名重注册前移除旧映射。
  • 修复 Application 命令递归注销、控制台终态及资源流/URI 路径关闭。

修改目的

让所有平台生命周期只向前推进,并确保插件停止后不再产生新任务或残留注册;异步事件和关闭流程的异常必须能够被调用方观察。

兼容性与行为变化

  • 保留既有公开同步入口和 JVM 签名。
  • Application 会补发此前缺失的 ACTIVE 生命周期。
  • 插件停止后提交的新任务会被拒绝,而不是静默遗留。
  • 命令注销现在会完整清除该命令的全部映射。

验证

  • ./gradlew :common:test :platform:platform-application:test :platform:platform-bukkit-impl:test :platform:platform-velocity:test :platform:platform-velocity-impl:test :platform:platform-afybroker:test --rerun-tasks --no-parallel
  • ./gradlew :common:build :platform:platform-application:build :platform:platform-bukkit-impl:build :platform:platform-velocity:build :platform:platform-velocity-impl:build :platform:platform-afybroker:build --no-parallel
  • 关键公开 JVM 构造器、字段和方法签名检查
  • git diff --check upstream/dev/6.3.0...HEAD

Refs #703

@FxRayHughes

Copy link
Copy Markdown
Contributor

Code Review — #715 fix(platform): 修复生命周期终态与任务清理

这个 PR 挖出了几个存在很久、影响很实际的 bug,其中两个我实测复现了。质量上和 #713 一个水平。

最有价值的三处发现:

  • Application 平台从来没有触发过 ACTIVE,而 ClassVisitorSchedule 注册在 ACTIVE——意味着 Application 平台上 @Schedule 一直是失效的。
  • AppCommand.unregister(name: String)无条件无限递归,调用即栈溢出。
  • TabooLib.lifeCycleisStopped 时连 DISABLE 都不执行——启动失败后资源永远不清理。

有几处破坏性变更需要进兼容性说明,另有一处 Velocity 的 @Subscribe 迁移我认为需要确认。

审阅方式:读 patch + 对照源码逐条验证 + 实测(Velocity API 3.1.1 jar 反编译、reflex 双构造器行为、CME 复现)。未实跑 gradle 测试


🟢 先说三个值得单独点出的真 bug

a. AppCommand.unregister(String) 无限递归AppCommand.kt:35-38):

fun unregister(name: String) {
    commands.find { it.command.aliases.contains(name) } ?: return
    unregister(name)          // ← 调自己,不是 unregister(Command)
}

第一行的 find 结果被丢弃(只用于 ?: return 提前退出),第二行 unregister(name) 参数类型是 String,解析到的仍是自身而非 unregister(command: Command) 重载。只要命令存在就无限递归 → StackOverflowError

顺带一提,这个 bug 还掩盖了另一个问题:it.command.aliasesCommandStructure.aliases,不含主名;而 Command.aliases:72)才是 listOf(command.name, *aliases)。所以按主名注销根本匹配不到。unregisterCommand:88)同样用了 it.command.aliases.contains(command),有相同问题。新代码统一改用 matches(name)(基于 Command.aliases,含主名,且 ignoreCase),两个问题一起修掉了。

b. unregisterCommands() 抛 CMEAppCommand.kt:91-93):

override fun unregisterCommands() {
    commands.forEach { unregister(it) }   // unregister → commands.remove(it)
}

commandsmutableSetOf()LinkedHashSet,边遍历边删。我实测复现:

$ java CME
ConcurrentModificationException 复现成功
单元素: 无异常, left=[]

注意单元素时不抛(HashMap.forEachmodCount 检查在循环结束后才触发一次,单元素删完正好走完),所以这个 bug 在只注册了一条命令时不会暴露——很符合"长期存在但没人报"的特征。新代码改成 commands.clear() + CopyOnWriteArraySet,两层都修了。

c. isStopped 时 DISABLE 被跳过TabooLib.java:66-68):

public static void lifeCycle(LifeCycle lifeCycle) {
    if (isStopped) {
        return;                    // ← DISABLE 也被拦住
    }

isStopped 会在 Kotlin 环境检查失败时被置为 true(:72),插件也可以在 onEnable 里主动停止。这两种情况下所有 @Awake(LifeCycle.DISABLE) 清理逻辑都不会跑——数据库连接池、调度器线程池、文件监听器全部泄漏。6 个平台的 onDisable 都调 lifeCycle(DISABLE),全部受影响。

改成 if (isStopped && lifeCycle != LifeCycle.DISABLE) 是必要修复。Kotlin 环境缺失时额外加的 if (lifeCycle == DISABLE) return 也对——没有 Kotlin 环境时 DISABLE 任务本身也跑不了,抛异常只会掩盖真正的启动错误。


🟡 问题 1 — Velocity 的 @Subscribe 迁移让旧同步入口变成死代码

-    @Subscribe
+    /**
+     * 保留旧同步入口;该入口无法向调用方表达异步完成,只负责观察失败。
+     */
     public void e(ProxyShutdownEvent e) {
+        observeDisable(disableAfterActivation());
+    }
+
+    @Subscribe
+    public EventTask eAsync(ProxyShutdownEvent e) {
+        return EventTask.resumeWhenComplete(disableAfterActivation());
     }

@Subscribee 移到了 eAsynce 现在没有任何注解,Velocity 不会再调用它,注释说的"保留旧同步入口"实际上只是保留了方法签名——它变成了死代码(除非有人手动调用,但这是 Velocity 的事件回调,不会有外部调用方)。

EventTask.resumeWhenComplete 我确认在 3.1.1 存在(本机反编译 velocity-api-3.1.1.jar):

public static com.velocitypowered.api.event.EventTask resumeWhenComplete(java.util.concurrent.CompletableFuture<?>);

所以技术上可行。但有两点建议:

  1. 如果目的是"保留公开签名以防有人反射调用",建议在 KDoc 里写明它已不再被 Velocity 触发;否则直接删掉更清晰。目前的注释容易让人以为它还在工作。
  2. 方法名 eAsync 和原有 e 并存,而 disableFuture 是幂等的(CAS + 复用),所以即使两者都被触发也只会 disable 一次——这个设计是对的。

另外 disableAfterActivation() 里有一处小瑕疵:

CompletableFuture<Void> created = new CompletableFuture<>();
if (!disableFuture.compareAndSet(null, created)) {
    return disableFuture.get();
}

CAS 失败后 disableFuture.get() 理论上可能读到 null(如果另一线程刚 CAS 成功但…… 实际不会,因为 CAS 成功即写入非 null,且 AtomicReference 保证可见性)。这条没问题,只是读起来需要绕一下,写成 updateAndGetcomputeIfAbsent 语义会更直观。


🟡 问题 2 — VelocityProxyEvent.call() 的返回值语义从"结果"变成"快照"

 fun call(): Boolean {
-    VelocityPlugin.getInstance().server.eventManager.fire(this)
-    return !isCancelled
+    val future = fireEvent()
+    val snapshot = !isCancelled
+    future.whenComplete { _, throwable -> if (throwable != null) reportCallFailure(throwable) }
+    return if (future.isDone) !isCancelled else snapshot
 }

需要说明的是:旧代码同样不等待——eventManager.fire() 返回 CompletableFuture<E>(我确认了 3.1.1 的签名),旧代码直接丢弃返回值后读 isCancelled。所以"异步监听器的取消结果读不到"这个问题旧代码就有,本 PR 没有引入。

新代码的改进是:加了 @Volatile(旧代码读 isCancelled 存在可见性问题)、加了异常上报(旧代码异步监听器抛异常会被 future 静默吞掉)。这两点都是实质改善。

return if (future.isDone) !isCancelled else snapshot 这一句我建议再想想:

  • snapshot 是在 fireEvent() 返回之后读的,同步监听器场景下此时 future 已完成,snapshot 就已经是最终值,future.isDone 分支与 snapshot 等价
  • 异步场景下返回 snapshot,即"调用瞬间可见的取消状态",这个值对调用方基本没有意义——它既不是"未取消"也不是"最终结果"

也就是说这个三元表达式的两个分支在实践中要么等价、要么都不可靠。如果目的是让调用方明确知道"这个结果不可信",不如让 call() 在 future 未完成时明确记录一次 warning,或者干脆在 KDoc 里写明"仅在所有监听器均为同步时返回值可靠"。新加的 callAsync() 是正确的出路,建议在 call() 的 KDoc 里指引过去。


🟡 问题 3 — 三个平台的 submit 在停止后改为抛 RejectedExecutionException

AfyBroker、Velocity、Application 三个 executor 都加了停止后拒绝:

AfyBrokerTaskRegistration.REJECTED -> {
    task.cancel()
    throw RejectedExecutionException("AfyBrokerExecutor has been stopped")
}

PR 描述里写了"插件停止后提交的新任务会被拒绝,而不是静默遗留",方向我认同。但需要注意这会打到 DISABLE 阶段自己的清理代码

stop() 注册在 LifeCycle.DISABLE 优先级 2。如果某个 @Awake(LifeCycle.DISABLE) 的清理逻辑(优先级默认 0,但排序后可能在 2 之后)里调用了 submit,就会拿到 RejectedExecutionException。TabooLib 自身在 DISABLE 阶段有多处清理,插件侧更不可控。

建议确认一下优先级 2 是否足够靠后(registerLifeCycleTaskComparator.comparingInt(LifeCycleTask::priority) 升序,所以 2 会在 0 之后执行,看起来是对的),并在兼容性说明里明确"DISABLE 阶段之后 submit 会抛"。

另外 AfyBrokerExecutor 用的是 TabooLib.registerLifeCycleTask,而 VelocityExecutor / AppExecutor 用的是 taboolib.common.platform.function.registerLifeCycleTaskCommon.kt:26)。两者应该等价,但同一批改动里用两种写法,建议统一。


🟡 问题 4 — Application 补上 ACTIVE 会让此前从未执行的代码开始执行

这是本 PR 影响面最大的一处,但 PR 描述只写了一句"Application 会补发此前缺失的 ACTIVE 生命周期"。

App.java:49-52 确认只跑 CONST/INIT/LOAD/ENABLE,没有 ACTIVE。而 ClassVisitorSchedule.getLifeCycle() 返回 LifeCycle.ACTIVE——也就是说 Application 平台上 @Schedule 从来没有生效过

修复本身是对的(这是个真 bug),但后果需要说清楚:

  • 所有 @Schedule 会开始执行。如果有插件在 Application 平台上写了 @Schedule 但因为一直不生效而没发现里面的问题(比如访问了未初始化的资源),升级后会暴露
  • 所有 @Awake(LifeCycle.ACTIVE) 同理

建议在兼容性说明里点明"Application 平台上 @Schedule@Awake(ACTIVE) 此前不会执行,现在会开始执行",让用户知道该检查什么。


🔵 次要

a. AfyBrokerExecutorLifecycle.kt 的辅助类只服务单个平台。 新增的 AfyBrokerTaskRegistry / AfyBrokerTaskCancellation / runAfyBrokerDispatch / runAfyBrokerTask 共 173 行,而 VelocityExecutorAppExecutor 各自又实现了一套等价的状态机(State.NEW/RUNNING/STOPPED + pending/active 双集合 + 停止后拒绝)。三份实现逻辑高度相似但代码完全独立。考虑抽到 common-platform-api 供三个平台共用,能显著减少后续维护成本。不阻塞。

b. AfyBrokerExecutor 少了停止后拒绝的对称性。 AfyBrokerRunningTask.execute(async, delay, period)if (cancellation.isCancelled()) { onCompleted(); return },但 execute()(now 分支)没有这个检查。executeUserTask 内部有 cancellation.runIfActive,所以实际是安全的,只是两条路径的防护位置不一致。

c. AfyBrokerPlugin.disable()rethrow 会把异常抛给 AfyBroker 的 onDisable AfyBrokerPlugin.<RuntimeException>rethrow(failure) 用泛型擦除绕过 checked exception。这个技巧本身没问题,但抛给平台的 onDisable 后行为取决于 AfyBroker 如何处理——可能中断后续插件的卸载。而 reportDisableFailure 路径(异步分支)是只记录不抛。同一个方法的同步/异步两条路径异常处理不一致,建议统一为"记录但不抛"。

d. AppExecutor / VelocityExecutor 的双构造器与 reflex 实例化。 两者都改成了「私有/internal 主构造器 + 公开无参次构造器」。我实测验证了 reflex 1.2.4 的 newInstance() 在这两种形态下都能正确选中无参构造器:

reflex newInstance() -> Dual(a=null,b=0,c=true)      # public 3 参 + public 0 参
reflex -> Dual2(b=42)                                 # private 3 参 + public 0 参

所以 PlatformFactorycls.newInstance() 不会选错。这条列出来是因为它是个容易踩的坑,确认没问题。

e. Executors.newFixedThreadPool(16) → 自定义 ThreadFactory 三个 executor 都加了命名线程工厂和 shutdownNow()AppExecutorshutdownNow() 而非 shutdown() + awaitTermination,意味着正在执行的任务会被 interrupt。对 DISABLE 阶段这是合理选择(快速退出优先),但如果有清理任务正跑在这个池子里会被打断。提一下备查。

f. Bukkit 命令注销的真实缺陷确认。unregisterCommandBukkitCommand.kt:133)只有 knownCommands.remove(command),而注册时写入了四个 key:namespace:namename、以及每个 alias。所以旧代码注销后 namespace 形式和所有 alias 都残留,且 PluginCommand 自身没有 unregister(commandMap)。新代码 removeMappingsByIdentity + binding.command.unregister(commandMap) 是完整的。这条修得对。

g. registeredCommands 是 public 字段但现在受 commandLock 保护。 新代码在 synchronized(commandLock)add/removeAt,但 registeredCommands 本身是 public val ArrayList,外部读取时不持锁。当前仓库内没有外部使用(grep 确认只有 BukkitCommand 自己),但作为公开 API 存在并发读的可能。考虑改成 CopyOnWriteArrayList 或收窄可见性。


🟢 已核对无误

结论
EventTask.resumeWhenComplete 可用性 本机反编译 velocity-api-3.1.1.jar 确认存在该静态方法,签名 (CompletableFuture<?>) -> EventTask,与用法匹配
EventManager.fire 返回 future 确认签名 <E> CompletableFuture<E> fire(E),旧代码丢弃返回值属实,"call() 读不到异步结果"是既有问题而非本 PR 引入
reflex 双构造器选择 实测两种形态(public+public、private+public)下 newInstance() 均正确选中无参构造器,AppExecutor / VelocityExecutor 的构造器重构安全
unregisterCommands CME 实测 LinkedHashSet 边遍历边删确实抛 ConcurrentModificationException;单元素时不抛,解释了为何长期未被发现
Application 缺失 ACTIVE App.java:49-52 确认只有四个阶段;ClassVisitorSchedule.getLifeCycle() 返回 ACTIVE,所以 @Schedule 在该平台一直失效。修复属实
isStopped 拦住 DISABLE TabooLib.java:66-68 确认;6 个平台(bukkit/bungee/velocity/afybroker/application/hytale)的 onDisable 全部调 lifeCycle(DISABLE),全部受影响
setStopped 已存在 TabooLib.java:170,测试用它切换状态不需要新增 API
生命周期任务优先级顺序 registerLifeCycleTaskComparator.comparingInt(LifeCycleTask::priority) 升序排序,优先级 2 的 stop() 会在默认优先级 0 的清理任务之后执行,顺序正确
AppLifeCycle 状态机单向性 NEW → INITIALIZING → ACTIVE,shutdown 在 INITIALIZING 时设 STOP_REQUESTED,由 finishTransition 在当前阶段结束后转入 DISABLING。不存在从 DISABLING/DISABLED 回退到 ACTIVE 的路径
AppLifeCycletransitionRunning 语义 beginTransition 在状态非 INITIALIZING 时返回 false 中断循环;finishTransition 检测 STOP_REQUESTED 并转 DISABLING。关闭请求落在阶段执行中间时,会等当前阶段结束再 disable,不会打断半个阶段
AfyBrokerActiveGate / VelocityActivationGate OPEN → ACTIVATING → CLOSED 单向 CAS;close() 在 OPEN 时直接完成 future,在 ACTIVATING 时返回未完成的 future 让 disable 等待 ACTIVE 跑完。避免了 DISABLE 先于 ACTIVE 完成导致的倒退
activate 的 finally state.set(CLOSED) + activationClosed.complete(null) 在 finally 中,即使 action 抛异常也会释放等待方,不会让 disable 永久挂起
ACTIVE 内的 isStopped 二次检查 activate 的 lambda 开头 if (TabooLib.isStopped()) return,防止 gate 打开但插件已停止时仍触发 ACTIVE。与 gate 形成双重保护
DISABLE 幂等 AfyBroker 用 AtomicBoolean disabled CAS;Velocity 用 AtomicReference<CompletableFuture> disableFuture CAS;Application 用 AppLifeCycle 状态机。三者都保证 disable 只执行一次
用户回调异常不阻断 DISABLE 三个平台都是 try { pluginInstance.onDisable() } catch { failure = ex }继续执行 TabooLib.lifeCycle(DISABLE),再统一 rethrow。用户代码抛异常不会跳过框架清理。这个顺序是对的
commandLabelMatches 的 namespace 处理 : 时先校验前缀等于 plugin.name.lowercase(),不匹配直接 false;无 : 时按 label 比对主名与 alias,全部 ignoreCase。逻辑正确
removeMappingsByIdentity 用引用比较 filterValues { it === target },按 PluginCommand 实例身份删除,不会误删其他插件注册的同名命令。这是关键——按字符串删会伤到别人
同名重注册前清理 registerCommand 里先 filter { it.structure.name.equals(command.name, ignoreCase = true) }unregisterBinding,解决了 reload 后旧映射指向失效插件的问题
注册与身份记录的原子性 knownCommands 写入、pluginCommand.register(commandMap)registeredCommands.addregisteredCommandBindings.add 全部在同一个 synchronized(commandLock) 内,不会出现"命令已注册但没记录"的中间态
submit(now = true)#712 的兼容 registerCommand 用的是 submit(now = true),而 #712 新增的 Folia 检查是 !now && !async,now = true 不会命中。两个 PR 在这一点上不冲突
AppCommand.matches 修复了主名匹配 旧代码用 it.command.aliasesCommandStructure.aliases,不含主名),新代码用 Command.aliases(含主名)。顺带修了按主名注销匹配不到的问题
CopyOnWriteArraySet 替换 AppCommand.commands 改为 CoW,配合 removeIf / clear,消除了 CME 与并发读风险
suggeststartsWith(ignoreCase) 补齐了大小写不敏感,与 matches 保持一致
AfyBrokerTaskCancellation.bind 幂等 check(current == null || current === value) 允许重复 bind 同一实例(cancel() 里会再 bind 一次),不同实例才抛。设计正确
延迟绑定的取消 bind 时若已 cancelled,立即调 cancelDelegate(value)。解决了"任务还没拿到 ScheduledTask 句柄就被取消"导致的取消丢失
BrokerPlatformTask.cancel 幂等 新增 AtomicBoolean cancelled CAS,重复 cancel 只执行一次 close
测试覆盖 新增 6 个测试文件:DISABLE 放行、AfyBroker 执行器生命周期(249 行)、Velocity 激活门、Velocity 事件、Velocity 执行器、Bukkit 命令注册表、Application 平台。覆盖了状态机、幂等、延迟绑定、拒绝路径

总结

三个真 bug(Application 缺 ACTIVE 导致 @Schedule 一直失效、AppCommand.unregister 无限递归、isStopped 拦住 DISABLE 导致资源泄漏)都是长期存在且影响实际功能的,修得对。激活门 + DISABLE 幂等 + 用户回调异常不阻断框架清理这三层设计是处理生命周期终态的正确形态。Bukkit 命令按实例身份清理全部映射也修得完整。

建议处理:

  1. 问题 4(Application 补 ACTIVE)——影响面最大,兼容性说明里应明确"此前 @Schedule / @Awake(ACTIVE) 在该平台不执行,现在会执行"。
  2. 问题 1(Velocity @Subscribe 迁移)——e(ProxyShutdownEvent) 已不再被触发,注释说"保留旧同步入口"容易误解,建议说明或删除。
  3. 问题 3(停止后拒绝)——确认 DISABLE 阶段自身的清理代码不会撞上 RejectedExecutionException,并写进兼容性说明。
  4. 问题 2call() 快照语义)——建议在 KDoc 里写明可靠性边界并指引 callAsync()
  5. 🔵 a(三份等价状态机可抽公共)和 c(AfyBroker 同步/异步分支异常处理不一致)建议一并考虑。

说明:本次审阅未实跑 gradle 测试(含 PR 描述列出的两条命令)。以下为本机实测:velocity-api-3.1.1.jar 反编译确认 EventTask.resumeWhenCompleteEventManager.fire 签名;reflex 1.2.4 双构造器 newInstance() 选择行为;LinkedHashSet 边遍历边删的 CME 复现。其余结论基于 patch 与仓库源码推导,已逐条注明依据位置。

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants