search

ydb-platform / ydb-go-sdk

Pure Go native and database/sql driver for YDB
https://ydb.tech
Apache License 2.0
136 stars 71 forks source link

bug: panic on `*Session.Begin` in `safe.ID()` #1419

Closed pelageech closed 1 month ago

pelageech commented 1 month ago

Bug Report

YDB GO SDK version: 3.76.5

Environment macOS Sonoma 14.2.1, amd64

Current behavior:

panic: runtime error: invalid memory address or nil pointer dereference
[signal SIGSEGV: segmentation violation code=0x1 addr=0x0 pc=0xb787997]

goroutine 1 [running]:
github.com/ydb-platform/ydb-go-sdk/v3/internal/query.(*Transaction).ID(0xc24a200?)
        <autogenerated>:1 +0x17
github.com/ydb-platform/ydb-go-sdk-otel/internal/safe.ID(...)
        /Users/artblaginin/GolandProjects/ydbPlayground/vendor/github.com/ydb-platform/ydb-go-sdk-otel/internal/safe/safe.go:63
github.com/ydb-platform/ydb-go-sdk-otel.query.func16.1({{0xbbf25e0?, 0xc000157780?}, {0xbbf2120?, 0x0?}})
        /Users/artblaginin/GolandProjects/ydbPlayground/vendor/github.com/ydb-platform/ydb-go-sdk-otel/query.go:266 +0x94
github.com/ydb-platform/ydb-go-sdk/v3/trace.(*Query).Compose.func19.2({{0xbbf25e0?, 0xc000157780?}, {0xbbf2120?, 0x0?}})
        /Users/artblaginin/GolandProjects/ydbPlayground/vendor/github.com/ydb-platform/ydb-go-sdk/v3/trace/query_gtrace.go:682 +0xcf
github.com/ydb-platform/ydb-go-sdk/v3/trace.QueryOnSessionBegin.func1({0xbbf25e0?, 0xc000157780?}, {0xbbf2120?, 0x0?})
        /Users/artblaginin/GolandProjects/ydbPlayground/vendor/github.com/ydb-platform/ydb-go-sdk/v3/trace/query_gtrace.go:1650 +0x2b
github.com/ydb-platform/ydb-go-sdk/v3/internal/query.(*Session).Begin.func1()
        /Users/artblaginin/GolandProjects/ydbPlayground/vendor/github.com/ydb-platform/ydb-go-sdk/v3/internal/query/session.go:267 +0x31
github.com/ydb-platform/ydb-go-sdk/v3/internal/query.(*Session).Begin(0xc00005b1a0, {0xbbfbb58, 0xc0000b9dd0}, {0xc00003f4a0, 0x1, 0x1})
        /Users/artblaginin/GolandProjects/ydbPlayground/vendor/github.com/ydb-platform/ydb-go-sdk/v3/internal/query/session.go:278 +0x3bd
github.com/ydb-platform/ydb-go-sdk/v3/internal/query.doTx.func1({0xbbfbb58, 0xc0000b9dd0}, {0xbc00c60?, 0xc00005b1a0?})
        /Users/artblaginin/GolandProjects/ydbPlayground/vendor/github.com/ydb-platform/ydb-go-sdk/v3/internal/query/client.go:168 +0x5e
github.com/ydb-platform/ydb-go-sdk/v3/internal/query.do.func1({0xbbfbb58?, 0xc0000b9dd0?}, 0xc00005b1a0)
        /Users/artblaginin/GolandProjects/ydbPlayground/vendor/github.com/ydb-platform/ydb-go-sdk/v3/internal/query/client.go:119 +0x39
github.com/ydb-platform/ydb-go-sdk/v3/internal/query.(*poolStub).With.func2({0xbbfbb58, 0xc0000b9dd0})
        /Users/artblaginin/GolandProjects/ydbPlayground/vendor/github.com/ydb-platform/ydb-go-sdk/v3/internal/query/client.go:80 +0xa6
github.com/ydb-platform/ydb-go-sdk/v3/retry.Retry.func1({0xbbfbb58?, 0xc0000b9dd0?})
        /Users/artblaginin/GolandProjects/ydbPlayground/vendor/github.com/ydb-platform/ydb-go-sdk/v3/retry/retry.go:262 +0x22
github.com/ydb-platform/ydb-go-sdk/v3/retry.opWithRecover[...]({0xbbfbb58?, 0xc0000b9dd0?}, 0xc00003f4d0?, 0x0?)
        /Users/artblaginin/GolandProjects/ydbPlayground/vendor/github.com/ydb-platform/ydb-go-sdk/v3/retry/retry.go:409 +0x91
github.com/ydb-platform/ydb-go-sdk/v3/retry.RetryWithResult[...]({0xbbfbb58, 0xc0000b9dd0}, 0xc0001b1c00, {0x0, 0x0, 0xc0001b1c30})
        /Users/artblaginin/GolandProjects/ydbPlayground/vendor/github.com/ydb-platform/ydb-go-sdk/v3/retry/retry.go:341 +0x4f7
github.com/ydb-platform/ydb-go-sdk/v3/retry.Retry({0xbbfbb58?, 0xc0000b9dd0?}, 0x10?, {0x0?, 0xc5bc108?, 0xc000082808?})
        /Users/artblaginin/GolandProjects/ydbPlayground/vendor/github.com/ydb-platform/ydb-go-sdk/v3/retry/retry.go:261 +0x4e
github.com/ydb-platform/ydb-go-sdk/v3/internal/query.(*poolStub).With(0x0?, {0xbbfbb58?, 0xc0000b9dd0?}, 0xb0d5525?, {0x0?, 0xbb03ac0?, 0xc00003f401?})
        /Users/artblaginin/GolandProjects/ydbPlayground/vendor/github.com/ydb-platform/ydb-go-sdk/v3/internal/query/client.go:71 +0x95
github.com/ydb-platform/ydb-go-sdk/v3/internal/query.do({0xbbfbb58, 0xc0000b9dd0}, {0xbbfaa30, 0xc00003f410}, 0xc000010978, {0x0, 0x0, 0x0})
        /Users/artblaginin/GolandProjects/ydbPlayground/vendor/github.com/ydb-platform/ydb-go-sdk/v3/internal/query/client.go:116 +0xa3
github.com/ydb-platform/ydb-go-sdk/v3/internal/query.doTx({0xbbfbb58, 0xc0000b9dd0}, {0xbbfaa30, 0xc00003f410}, 0xc000010960, 0xc0001b1e00?, {0x0?, 0xc5bc428?, 0x10?})
        /Users/artblaginin/GolandProjects/ydbPlayground/vendor/github.com/ydb-platform/ydb-go-sdk/v3/internal/query/client.go:167 +0xec
github.com/ydb-platform/ydb-go-sdk/v3/internal/query.(*Client).DoTx(0xc0000b9d70, {0xbbfbc00, 0xc000282770}, 0xc00003f420, {0x0, 0x0, 0x0})
        /Users/artblaginin/GolandProjects/ydbPlayground/vendor/github.com/ydb-platform/ydb-go-sdk/v3/internal/query/client.go:323 +0x278
main.main()
        /Users/artblaginin/GolandProjects/ydbPlayground/query_readrow/main.go:36 +0x266

Expected behavior: No panic.

Steps to reproduce: The following code:

func (s *Session) Begin(
    ctx context.Context,
    txSettings query.TransactionSettings,
) (
    _ query.Transaction, err error,
) {
    var tx *Transaction // 1. nil pointer

    onDone := trace.QueryOnSessionBegin(s.cfg.Trace(), &ctx,
        stack.FunctionID("github.com/ydb-platform/ydb-go-sdk/v3/internal/query.(*Session).Begin"), s)
    defer func() {
        onDone(err, tx) // 3. onDone takes an interface, we pass a nil pointer
    }()

    tx, err = begin(ctx, s.grpcClient, s, txSettings)
    if err != nil {
        if xerrors.IsOperationError(err, Ydb.StatusIds_BAD_SESSION) {
            s.setStatus(statusClosed)
        }

        return nil, xerrors.WithStackTrace(err) // 2. return, begin returns nil, err
    }
    tx.s = s

    return tx, nil
}

in ydb-go-sdk-otel:

OnSessionBegin: func(info trace.QuerySessionBeginStartInfo) func(info trace.QuerySessionBeginDoneInfo) {
    if cfg.detailer.Details()&trace.QuerySessionEvents == 0 {
        return nil
    }
    start := childSpanWithReplaceCtx(
        cfg.tracer,
        info.Context,
        info.Call.FunctionID(),
    )

    return func(info trace.QuerySessionBeginDoneInfo) {
        finish(
            start,
            info.Error,
            attribute.String("TransactionID", safe.ID(info.Tx)), // interface info.Tx is not nil, panic sigsegv
        )
    }
},

Related code:

insert short code snippets here

Other information: Reference https://github.com/ydb-platform/ydb-go-sdk/issues/1364

pelageech commented 1 month ago

Please, check all the places where you could pass a nil pointer as an interface. They can panic at any moment

asmyasnikov commented 1 month ago

This bug fixed in https://github.com/ydb-platform/ydb-go-sdk/commit/ded15e71be0661736ab31d664bd1dae70b7d11cf and wait next release