matrixorigin / matrixorigin/matrixone

[Bug]: txnContext is published before initialization

Open
#28,759 0 comments 0 reactions 1 assignee Claimed by @XuPeng-SH View on GitHub
kind/bug needs-triage
Dominant language
Go
Stars
1.9k
Forks
311
Avg merge
1d 3h
Merged PRs (30d)
768

Description

## 问题概述

`pkg/txn/service/service.go` 中的 `maybeAddTxn` 会先将新建的 `txnContext` 放入 `transactions`,之后才初始化事务元数据和 notifier。

因此,并发的同一事务请求可能观察到一个尚未完成初始化的 context。

## 触发时序

两个请求同时首次处理同一个事务 ID:

1. 请求 A 和请求 B 都在初始 `transactions.Load` 中看到事务不存在;
2. A 执行 `LoadOrStore` 成功,将未初始化的 `txnContext` 发布到 map;
3. A 在执行 `txnCtx.init` 前被调度暂停;
4. B 执行 `LoadOrStore`,发现已有 context;
5. B 释放自己从 pool 获取但尚未初始化的 context;
6. `releaseTxnContext` 调用 `resetLocked`,而 `resetLocked` 无条件调用 nil notifier 的 `close`,导致 panic。

该路径只需要普通 goroutine 调度,不依赖 race detector 才能成立。

## 其他影响

在 A 尚未完成初始化时,其他消费者也可能读取 A 发布的半初始化 context:

- Write 可能看到空事务 ID 并返回 `TxnNotFound`;
- Commit 可能返回错误的 `TNShardNotFound`;
- Rollback 可能在清理半初始化 context 时触发 nil pointer panic;
- Read 的等待注册可能错误地认为事务不存在,导致等待语义失效。

## 根因

当前顺序为:

```text
acquireTxnContext
LoadOrStore
txnCtx.init
```

`sync.Map` 只保证 map 操作本身的并发安全,不会把后续的 `txnCtx.init` 与发布动作合并为原子操作。

## 建议修复

在发布前完成完整初始化:

```text
acquireTxnContext
txnCtx.init
LoadOrStore
```

如果 `LoadOrStore` 发现已有 context,只释放这个已经完整初始化但未发布成功的 context。仅在 `resetLocked` 中增加 nil 判断只能隐藏 panic,不能修复半初始化对象被消费者观察到的问题。

## 回归测试建议

增加确定性并发单元测试,使用 channel 屏障确保两个调用者都先通过初始 `Load`,验证:

- 重复创建不 panic;
- 恰好一个 context 被发布;
- 两个调用者得到同一个已初始化 context;
- 失败竞争者可以安全 reset 并返回 pool;
- 发布 context 的事务 ID、notifier 和创建时间均已初始化。

相关修复 PR:#28758

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.