fix(database): 完善数据库与 Redis 资源生命周期#709
Conversation
确保数据源、连接、客户端和线程资源在启动失败、重连及并发关服场景下可靠释放,并补充相关回归测试。
FxRayHughes
left a comment
There was a problem hiding this comment.
Code Review:PR #709 fix(database): 完善数据库与 Redis 资源生命周期
改动范围很大(17 文件 / +1254 / -338),横跨四个独立模块。"资源所有权 + 幂等关闭"这个设计方向是对的,AsyncStartupCoordinator 和两个 Registry 的抽象也干净,"启动失败导致 future 永久 pending"与"订阅重连无界增长"这两处修复尤其有价值。
但 RedisDatabaseHandler 的配置读取修正会双向破坏现有用户的配置,两类用户升级后都会看到空数据。需要在合并前补充过渡措施。
说明:以下结论基于源码推导,行号与调用链已逐一核对;未实跑 gradle 测试。
🔴 严重问题(建议阻止合并)
1. RedisDatabaseHandler 的配置读取修正会双向破坏现有配置
文件: module/database/database-player-redis/src/main/kotlin/taboolib/expansion/RedisDatabaseHandler.kt
事实基础
ConfigSection.getBoolean(path) → Coerce.toBoolean(get(path)),而 Coerce.toBoolean(null) 明确返回 false(Coerce.java:211-214,注释原文 "false if the object is null")。读一个不存在的键结果是 false,不报错、不警告。
HostSQL(section) 构造函数(HostSQL.kt:32-38):
constructor(section: ConfigurationSection) : this(
section.getString("host", "localhost")!!,
section.getString("port", "3306")!!,
section.getString("user", "root")!!,
section.getString("password", "root")!!,
section.getString("database", "test")!!,
)全部是相对传入 section 的直读,取不到即落默认值。
旧代码的两层错误
table = conf.getConfigurationSection("Database")!!.getString("table", pluginId)!! // ← 从 Database 子节点读
database = if (conf.getBoolean("enable")) { // ← 从根节点读
buildPlayerDatabase(conf, table, flags, clearFlags, ssl) // ← 把根节点传进去同一段逻辑里,table 从 Database 子节点读,enable 却从根节点读——层级不一致,是笔误的痕迹。
按类文档声明的结构(enable 在 Database 之下),根节点没有 enable → 恒为 false → MySQL 分支不可达。
即使它可达(用户在根节点加了 enable: true),传入 HostSQL 的是根节点,而用户配置里是 Database.host → 取不到 → 全部落默认值,连到 localhost:3306/test、账号 root/root。
关键:修复是双向破坏的
文档只是注释,用户的实际配置可能已经适配了「真实行为」而非「文档结构」。于是分成两类人,两类都会断:
A 类:按文档写配置
Database:
enable: true
host: mysql.example.com| 行为 | 数据实际位置 | |
|---|---|---|
| 旧 | 根节点无 enable → false → SQLite |
data.db |
| 新 | Database.enable = true → 真的连 MySQL |
空表 |
用户以为一直在用 MySQL,升级后连上真 MySQL 却是空的 → 感知为数据全部丢失
B 类:曾经踩过坑,把配置改成了能工作的样子
enable: true
host: mysql.example.com
port: '3306'
Database:
table: mydata| 行为 | 数据实际位置 | |
|---|---|---|
| 旧 | 根节点有 enable/host → MySQL 正常工作 |
MySQL |
| 新 | Database.enable 不存在 → false → 掉回 SQLite |
空的 data.db |
原本工作正常的用户被改坏 → 同样感知为数据全部丢失
所以不存在"修好了就没事"的路径。两类用户升级后都会看到空数据,只是方向相反。
建议的处理方式
代码改动本身是对的,不该退回。需要补的是用户侧过渡措施,按优先级:
- 兼容期回退读取(最推荐)——
databaseConfig.getBoolean("enable")取不到键时,回退读根节点的enable并打印弃用警告。这样 B 类用户有一个版本的迁移窗口,而不是升级即断 - 启动检测与警告——读到
Database.enable: true且data.db存在且非空时,打印明确警告:数据源已切换、旧数据在哪、如何迁移 - 兼容性章节把 A/B 两类情形都写出来,而非仅"修正了配置读取"
- release note 提供迁移指引,包含
data.db→ MySQL 的导入方式
2. table 参数语义变化导致第三种数据位置改变
同一文件:
// 旧:无条件丢弃构造参数(注意 table 是 var,直接被覆盖)
table = conf.getConfigurationSection("Database")!!.getString("table", pluginId)!!
// 新:构造参数成为 fallback
table = databaseConfig.getString("table", table.ifEmpty { pluginId })!!差异只在"配置里没有 Database.table"时体现:
- 旧:表名 =
pluginId - 新:表名 = 构造参数传入的
table
若某插件写 RedisDatabaseHandler(conf, table = "user_data") 而配置未写 Database.table,表名会从 pluginId 变为 user_data——第三种数据位置变化。
改动方向是对的(旧代码让构造参数完全失效,显然非本意),但同样需要写入兼容性说明。
🟡 中等问题
3. Lettuce 的 pub/sub 连接从同步改为异步,破坏了隐式契约
文件: LettuceRedisClient.kt、LettuceClusterRedisClient.kt
// 旧:同步,start() 返回前必定已初始化
pubSubConnection = client.connectPubSub()
// 新:异步赋值
val pubSubReady = client.connectPubSubAsync(StringCodec.UTF8).thenAccept {
pubSubConnection = it
...
}.toCompletableFuture()而 pub/sub 方法直接裸用该 lateinit 字段,无任何初始化检查:
override fun <T> useSyncPubSub(block: ...): T? = block(pubSubConnection.sync()) // :208
override fun <T> useAsyncPubSub(block: ...): T? = block(pubSubConnection.async()) // :212coordinateStart 确实等 pubSubReady 完成后才 complete 返回的 future,所以正确 await 返回值的调用方是安全的。但旧代码的同步 connectPubSub() 意味着 fire-and-forget 调用 start() 后立刻用 pub/sub 是可行的;改动后这种写法会撞 UninitializedPropertyAccessException。
start() 返回 CompletableFuture<Void> 本就暗示要等待,但既有插件很可能没等。建议给 pub/sub 访问加初始化检查并抛出清晰错误,或在兼容性说明中明确"必须等待 start() 返回的 future"。
(startSync() 保持同步 connectPubSub(),不受影响。两个客户端类改动一致。)
4. Alkaid Redis 的连接仍在关闭不属于自己的 pool
文件: SingleRedisConnection.kt / SingleRedisConnector.kt
SingleRedisConnector.connection() 可被多次调用,每次都用同一个 pool 构造新的 SingleRedisConnection:
fun connection(): SingleRedisConnection {
return SingleRedisConnection(pool ?: error("connect first"), this)
}而 SingleRedisConnection.close() 会 pool.close()——销毁的是 connector 拥有的共享 pool。两个 connection 场景下关闭其一,另一个立即失效。
这是旧代码就有的问题(旧代码 pool.destroy() 语义相同),非本 PR 引入。但 PR 的核心命题正是"资源由谁创建、由谁关闭",database-player 那边用 ownsDataSource 把这件事做对了,Alkaid Redis 这边却仍是连接关闭连接器的 pool。建议同一标准处理,或至少注释说明"一个 connector 只应产生一个 connection"。
5. Database 的所有权传递用 ThreadLocal 侧信道,较为脆弱
文件: module/database/database-player/src/main/kotlin/taboolib/expansion/Database.kt
class Database(val type: Type, val dataSource: DataSource = createOwnedDataSource(type)) : AutoCloseable {
val ownsDataSource = takeOwnership(dataSource)
constructor(type: Type, dataSource: DataSource, ownsDataSource: Boolean)
: this(type, markOwnership(dataSource, ownsDataSource))用 ThreadLocal<IdentityHashMap<DataSource, Unit>> 在默认参数求值与属性初始化之间传递"这个 DataSource 是我建的"。
功能上可行(Kotlin 中默认参数先于属性初始化器求值,同线程内 put→remove 配对),但:
- 若
createOwnedDataSource之后、takeOwnership之前抛异常(窗口极小但存在),ThreadLocal 会残留一条 DataSource 强引用且永不清理 - 阅读成本高,后续维护者重构构造函数顺序时很容易无声破坏它
更直白的写法是私有主构造 + 两个工厂方法(Database.owning(type) / Database.borrowing(type, ds))。若因兼容性必须保留现有签名,建议至少加注释说明这个机制为何必须存在。
6. RedisDatabaseHandler 实现了 AutoCloseable 但无人调用
新增的 close() 写得很规范(幂等、异常聚合用 addSuppressed、按依赖顺序释放)。但框架内没有任何地方在 DISABLE 时调用它——不像 AlkaidRedis / LettuceRedis 有 @Awake(LifeCycle.DISABLE) 注册。
只提供了能力而未接线,插件作者需自行调用。建议在文档或注释中说明,或同样注册生命周期回调。
🔵 小建议
executor 关闭方式变了:ClusterRedisConnection / SingleRedisConnection 从 service.shutdown() + awaitTermination(30s) 改为 shutdownNow()。不再阻塞 DISABLE 30 秒是好事(旧写法在有阻塞 subscribe() 时那 30 秒纯属白等),但订阅线程现在是被立即中断的。行为变化,可在说明中提一句。
AsyncStartupCoordinator.complete 的 onSettled 在每个 stage 都触发,若已 stopped 则每次都调 closeResources()。靠 shutdownStarted 的 CAS 保证只执行一次,是对的,但建议加注释说明这是有意的重复调用。
🟢 确认正确的改动
| 改动 | 评价 |
|---|---|
start() 失败路径补 failStart |
核心修复:旧代码只在 thenAccept 里 complete,一旦前置步骤抛异常,返回的 future 永久 pending |
RedisConnectionRegistry / LettuceRedisResourceRegistry |
register 后二次检查 closed 并回收——正确处理了"注册与关闭并发"的窗口 |
| 迟到资源立即回收 | 各 thenAccept 内 if (stopped.get()) it.closeAsync(),覆盖"启动失败但连接稍后成功"的泄漏 |
AsyncStartupCoordinator |
AtomicInteger 计数 + AtomicBoolean 单次失败上报,多 stage 协调干净 |
closeResources 分层关闭 |
先连接/池(收集 future)→ allOf → client.shutdownAsync() → resources.shutdown(),顺序正确且不阻塞 |
RedisClient.create 失败时 newResources.shutdown() |
补上了旧代码漏掉的资源回收 |
| 订阅从 static 共享列表移到 per-connection | 旧代码所有连接的订阅混在一个 companion object 列表里,关闭时无法区分归属,且从不 clear() |
createPubSub 提到重试外层 |
旧代码每次重连都新建 JedisPubSub 并往 resources 追加一条 Closeable,反复重连会无界增长 |
订阅重连改用 reconnectService.schedule |
替代旧代码 exec(loop = true) 的无界递归(长时间断连会 StackOverflow) |
ClusterRedisConnector.build() 重复调用时先关旧 cluster |
修复重复 build 泄漏 |
SingleRedisConnector.connect() 重复调用时关旧 pool |
同上 |
Database 建表失败时 close() 自建数据源 |
避免构造失败留下悬空连接池 |
RedisDatabaseHandler.init 失败时回滚已建资源 |
构造函数异常安全 |
pool.destroy() → pool.close() |
语义等价(Jedis Pool.close() 内部调 destroy()),更符合 Closeable 约定 |
exec 中 check(!closed.get()) |
use-after-close 现在给出清晰错误而非 Jedis 内部异常 |
各处 @Volatile / @Synchronized |
修复 pool 字段跨线程可见性与 connect/close 竞态 |
总结
资源治理部分做得扎实,"启动失败导致 future 永久 pending"和"订阅重连无界增长"是真实且隐蔽的 bug,修得对。
问题 1 需要在合并前处理——不是代码错误,恰恰因为修对了才出问题。A 类(按文档配置)和 B 类(配置已适配旧行为)用户升级后都会看到空数据,方向相反但结果相同。建议至少加入兼容期回退读取 + 启动警告,给用户一个版本的迁移窗口。
问题 2 需写入兼容性说明;3、4 建议一并处理;5、6 是可选优化。
原有问题
数据库与 Redis 客户端对“资源由谁创建、由谁关闭”缺少一致约定,异步启动与关闭之间也存在竞态:
database-player无法区分外部传入的数据源和模块内部创建的数据源,可能误关共享数据源,也可能遗漏关闭自建数据源。典型触发场景与后果
本 PR 修改
database-player增加明确的数据源所有权和幂等关闭协议,只关闭模块自身拥有的资源。修改目的
建立可预测的资源所有权和生命周期边界,保证插件启停、重载及连接失败时不会误关共享资源,也不会泄漏连接、线程或订阅任务。
兼容性与行为变化
验证
./gradlew :module:database:database-player:test :module:database:database-alkaid-redis:test :module:database:database-player-redis:build :module:database:database-lettuce-redis:test --no-daemon --stacktracegit diff --checkRefs #703