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]

Reply via email to