Skip to content

fix(smtp): 防止 SQLite 锁冲突导致审计记录丢失 - #358

Merged
Jinnrry merged 1 commit into
Jinnrry:masterfrom
0clickjacking0:fix/sqlite-audit-reliability
Aug 20, 2026
Merged

Jinnrry merged 1 commit into
Jinnrry:masterfrom
0clickjacking0:fix/sqlite-audit-reliability

Conversation

@0clickjacking0

Copy link
Copy Markdown
Contributor

问题

PMail 使用 SQLite 时,外部读事务或并发写事务可能使 INSERT、UPDATE 或事务提交返回 SQLITE_BUSY/SQLITE_LOCKED。这是 SQLite 的正常锁语义,但原有 SMTP 持久化逻辑只记录数据库错误,随后仍继续执行下游投递并向提交方报告成功。

这会产生以下不一致:

  • 初始 email 写入失败,但 PMail 仍将邮件交给下游 SMTP,导致邮件已送达而审计行完全缺失。
  • email 已插入但 user_email 插入失败,留下部分记录。
  • 下游投递成功或失败后,email 与 user_email 的最终状态更新失败或只更新一张表。
  • 邮件实际已被目标 MX 接受,但数据库仍停留在默认状态,无法反映真实结果。
  • 数据库写入失败不会反馈给上游 SMTP 客户端。

问题的触发条件可以来自 SQLite 锁竞争,但静默吞掉持久化失败并继续报告成功是 PMail 的应用层 bug。

修改内容

1. 原子写入初始审计记录

  • 将 email 与对应的 user_email 初始写入放入同一个事务。
  • 任一步失败都会回滚整个事务,不再产生孤立或部分记录。
  • 初始持久化成功后才允许调用下游 SMTP。
  • 初始持久化失败时返回 451 4.3.0 Temporary local persistence failure,让提交方稍后重试。
  • 入站邮件共用的持久化路径也改为原子写入并传播数据库错误。

2. 原子写入最终投递结果

  • 将 email.status/error 与 user_email.status 的最终更新放入同一个事务。
  • 两张表必须一起提交或一起回滚。
  • 对更新行数进行校验,避免审计记录不存在时静默成功。

3. 正确处理 SQLite BUSY/LOCKED

  • 通过 *sqlite.Error.Code() 识别 SQLite 主错误码,不依赖错误字符串。
  • 对可安全重放的完整事务最多重试三次,退避时间为 50ms 和 100ms。
  • 为每个 SQLite 连接设置 busy_timeout=1000ms。
  • 拒绝畸形或重复的 busy_timeout pragma;过大的显式值会被限制到 1000ms 并记录警告。
  • 保留用户 DSN 中的其他参数,包括显式配置的 journal_mode(WAL)。
  • 将 modernc.org/sqlite 升级到 v1.46.1,确保 COMMIT 返回 BUSY 后连接可以安全复用并重试完整事务。

本 PR 不会自动切换 WAL。WAL 会影响备份、checkpoint、-wal/-shm 文件、网络文件系统兼容和磁盘监控,应作为独立的可选运维变更进行验证。

4. 区分投递前和投递后的持久化失败

出站审计初始状态改为:

status=6: delivery pending / outcome unknown

如果初始审计事务失败,邮件尚未交给目标 MX,此时返回 451 是安全的,不会造成重复投递。

如果目标 MX 已经接受 DATA,但最终状态事务仍然失败,则不能因本地数据库错误向上游返回 451,否则上游重试会产生重复邮件。此时:

  • 保留 status=6,表示最终投递结果尚未可靠写入。
  • 输出结构化高优先级日志:CRITICAL audit_event=delivery_result_persist_failed。
  • 日志包含 email_id、msg_id、目标状态以及下游是否已接受。
  • 向上游返回原始下游 SMTP 结果;已被目标 MX 接受的邮件仍返回成功。

status=6 会出现在 Web 发件列表中,但不会被 IMAP 错误地归入 INBOX 或 Sent。

5. 保持现有提交语义

SMTP 结果语义

初始审计提交失败
-> 不调用下游 SMTP
-> 返回 451

初始审计提交成功
-> 调用下游 SMTP
-> 原子写入最终结果

下游 DATA 已接受,但最终结果写入失败
-> 保留 status=6
-> 输出 CRITICAL 告警
-> 仍向上游返回成功,避免重复投递

下游投递失败
-> 尝试原子保存失败状态和错误原因
-> 按 PR #356 的规则向上游返回 451 或 550

已知限制

status=6 尚无跨重启自动恢复

当前实现没有 durable reconciler。以下崩溃窗口可能留下长期 status=6:

  • 初始审计提交后、调用下游 SMTP 前。
  • 目标 MX 返回 DATA 250 后、最终状态提交前。
  • 最终状态写入重试耗尽后进程退出。

数据库无法仅根据 stale pending 记录判断邮件究竟尚未发送、已经送达、明确失败,还是多域部分成功。因此不能按记录年龄自动改成 sent/fail,也不能自动重新投递。

后续应补充:

  • 进程存活期间,对已经持有明确下游结果的最终状态进行后台重试,但绝不再次投递邮件。
  • 启动和周期扫描 stale pending,持续产生告警并提供人工确认入口。
  • 如需跨重启恢复准确结果,引入独立持久的 delivery-attempt/result journal。

SMTP 无法天然提供端到端 exactly-once。即使增加 result journal,远端接受与本地结果持久化之间仍存在无法完全消除的未知窗口。

SendAfter hooks 的现有顺序

本 PR 保留了现有同步 SendAfter hook 顺序:hook 完成后才写入最终状态。这会扩大下游返回与最终持久化之间的窗口,建议在独立修改中评估将最终状态持久化提前到 hooks 之前。

兼容性

  • 不新增数据库表或数据库迁移。
  • SQLite 专用 BUSY/LOCKED 重试不会应用到 MySQL/PostgreSQL 的普通错误。
  • SQLite 连接池仍限制为一个连接。
  • 不自动修改数据库的 journal_mode。
  • 相同 Message-ID 的现有重复提交行为保持不变。

测试

新增测试覆盖:

  • 初始 email INSERT 永久 BUSY 时不调用下游,并向上游返回 451。
  • 初始短暂 BUSY 在锁释放后恢复,且只投递一次。
  • user_email INSERT 失败时整个初始事务回滚。
  • 最终状态短暂 BUSY 后重试成功。
  • 最终两张表任一步更新失败时事务整体回滚。
  • 下游已经接受但最终状态永久写入失败时保留 pending、输出 CRITICAL,且不返回 451。
  • 临时和永久下游失败的状态及 SMTP 返回码。
  • 相同 Message-ID 的多次提交保持独立。
  • pending 在 Web 发件列表可见,但不会进入 IMAP INBOX/Sent。
  • SQLite COMMIT BUSY 后连接仍可继续使用。
  • busy timeout 解析、连接级应用、错误码分类和完整事务重试。

已执行:

make build_fe
go test ./db -count=20
go test ./listen/smtp_server -count=10
go test -race ./db ./listen/smtp_server -count=1
go vet ./db ./listen/smtp_server
go mod verify
git diff --check

以上检查通过。另外,使用 go test -p 1 逐包运行除根 package 外的全部 Go package,结果均通过。

完整 make test 的根 package 测试仍受外部 DNS/SPF 时序影响:TestMaster/testUser2EmailList 使用固定 8 秒等待,本地一次 SPF/DNS 处理约 10 秒,邮件随后正常入库;另一次重试在外部 DNS/setup 阶段超时。未观察到与本修复相关的 BUSY、451 或审计写入失败。

完整 go vet ./... 仍会报告仓库既有的 unkeyed struct literal;修改包的 scoped vet 已通过。

未运行 MySQL 集成测试,因为本地没有可用的 MySQL 服务或容器。

@Jinnrry
Jinnrry merged commit a6049bf into Jinnrry:master Aug 20, 2026
1 check passed
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