Ethan-Xingyue opened a new issue, #1155: URL: https://github.com/apache/incubator-seata-go/issues/1155
| Field | Value | | --- | --- | | Issue title | `[BUG] [TM] Nested Required transaction overwrites the outer Launcher role and skips the outer commit (regression of #848)` | | Labels (exist in repo) | `bug`, `module/tm` | | Suggested priority | P1 (missing commit = data consistency risk) | | Related | #864 (closed) "bug: In nested transactions of tcc, transactions cannot be committed or rolled back correctly"; fix PR #848 (commit `b89343b`, merged 2025-08-30); reintroduced by PR #1113 "Feat: support grpc" (commit `082999a`, 2026-07-04) | | Verification | Reproduced 2026-09-02 on master `3bf73586` with go1.24.3 darwin/arm64 | --- ## 🚀 Go Version go1.24.3 darwin/arm64 ## 📦 Seata-go Version master, commit 3bf73586af81db1bd428982c93d82000d80cb1c8 (fetched 2026-09-02) ## 💾 Operating System macOS ## 📝 Bug Description When a `Required` global transaction is started inside another `Required` transaction (`tm.WithGlobalTx` nested in `tm.WithGlobalTx`), the inner call rewrites the transaction role stored in the shared Seata context. After the inner call returns, the outer transaction is no longer recognised as `Launcher`, so its commit is skipped. The TC-side global transaction is never committed by the process that began it. This is a regression. #848 fixed exactly this by replacing `ClearTxConf(ctx)` with `ctx = transferTx(ctx)` in `WithGlobalTx`; #1113 (commit `082999a`) changed the line back to `ClearTxConf(ctx)`: - Current code: https://github.com/apache/incubator-seata-go/blob/3bf73586af81db1bd428982c93d82000d80cb1c8/pkg/tm/transaction_executor.go#L57-L59 - `transferTx` is still defined but no longer called: https://github.com/apache/incubator-seata-go/blob/3bf73586af81db1bd428982c93d82000d80cb1c8/pkg/tm/transaction_executor.go#L223-L225 - `git log -S'ClearTxConf(ctx)' -- pkg/tm/transaction_executor.go` shows `082999a Feat:support grpc (#1113)`; `git log -S'transferTx' -- pkg/tm/transaction_executor.go` shows `b89343b fix: The context issue of nested transactions (#848)`. **Root cause (from reading the code):** the Seata context stores a mutable `*ContextVariable`. The inner `Required` calls `ClearTxConf` on the same context, then `useExistGtx` sets the role to `Participant`. When the inner call exits the outer still sees `Participant` and skips the second phase that the Launcher is responsible for. `RequiresNew` / `NotSupported` also lack proper suspend/resume isolation for the same reason. Note: `WithGlobalTx` has few in-repo callers, but it is the public API used by applications, so in-repo call counts do not indicate low user impact. ## 🔄 Steps to Reproduce 1. Check out the commit and create a throwaway module that points at the local checkout: ```bash git clone https://github.com/apache/incubator-seata-go.git cd incubator-seata-go git checkout 3bf73586af81db1bd428982c93d82000d80cb1c8 repo_root="$(pwd)" repro_dir="$(mktemp -d)" cd "$repro_dir" go mod init seata-repro go mod edit -require=seata.apache.org/seata-go/[email protected] go mod edit -replace=seata.apache.org/seata-go/v2="$repo_root" ``` 2. Save the following as `repro_test.go` in `$repro_dir` (a recording `GlobalTransactionManager` replaces the TC round trips): ```go package repro import ( "context" "fmt" "testing" "time" "seata.apache.org/seata-go/v2/pkg/tm" ) // recordingManager replaces the real TC round trips and records every Commit. type recordingManager struct { next int committed []string } func (m *recordingManager) Begin(ctx context.Context, _ time.Duration) error { m.next++ tm.SetXID(ctx, fmt.Sprintf("xid-%d", m.next)) return nil } func (m *recordingManager) Commit(_ context.Context, tx *tm.GlobalTransaction) error { m.committed = append(m.committed, tx.Xid) return nil } func (*recordingManager) Rollback(context.Context, *tm.GlobalTransaction) error { return nil } func (*recordingManager) GlobalReport(context.Context, *tm.GlobalTransaction) (interface{}, error) { return nil, nil } func TestNestedRequiredMustCommitOuterTransaction(t *testing.T) { manager := &recordingManager{} tm.SetGlobalTransactionManager(manager) err := tm.WithGlobalTx(context.Background(), &tm.GtxConfig{ Name: "outer", Propagation: tm.Required, }, func(ctx context.Context) error { return tm.WithGlobalTx(ctx, &tm.GtxConfig{ Name: "inner", Propagation: tm.Required, }, func(context.Context) error { return nil }) }) if err != nil { t.Fatal(err) } if len(manager.committed) != 1 || manager.committed[0] != "xid-1" { t.Fatalf("outer transaction was not committed: commits=%v", manager.committed) } } ``` 3. Run: ```bash go mod tidy go test -run '^TestNestedRequiredMustCommitOuterTransaction$' -count=1 -v ``` ## ✅ Expected Behavior The inner `Required` joins the outer transaction. Only the outer Launcher commits `xid-1`, exactly once. Leaving the inner scope must not change the outer role or XID. ## ❌ Actual Behavior ```text === RUN TestNestedRequiredMustCommitOuterTransaction repro_test.go:47: outer transaction was not committed: commits=[] --- FAIL: TestNestedRequiredMustCommitOuterTransaction (0.00s) FAIL FAIL seata-audit-repros/tx002 0.578s FAIL ``` ## 💡 Possible Solution - Restore the "derive an isolated Seata context for the nested call" semantics from #848, but pin them with a table-driven propagation test so the next refactor cannot silently drop the line again. - Define and implement suspend/resume for `RequiresNew` / `NotSupported` instead of sharing a mutable role. Acceptance criteria: - [ ] The test above fails before the fix and passes after it. - [ ] Table covers `Required→Required`, `Required→RequiresNew`, `Required→NotSupported`, `Supports`, `Mandatory`, `Never`. - [ ] For every propagation, the inner business function returning success, returning an error, and panicking are covered; the outer XID and role are restored in each case. - [ ] `Required→Required` commits the outer transaction once; `RequiresNew` completes the second phase for inner and outer independently; `NotSupported` runs the inner without a transaction and the outer still commits. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
