ydb-platform / ydb-go-sdk

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

bug: panic on createSession when returned nil session #1364

Open pelageech opened 1 month ago

pelageech commented 1 month ago

Bug Report

YDB GO SDK version: 3.75.2

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=0x8 pc=0x99133c0]

goroutine 1 [running]:
github.com/ydb-platform/ydb-go-sdk/v3/internal/query.(*Session).ID(0xa33ddf0?)
        /Users/artblaginin/GolandProjects/ydbPlayground/vendor/github.com/ydb-platform/ydb-go-sdk/v3/internal/query/session.go:282
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.func12.1({{0x9d52540?, 0x0?}, {0x9d4a640?, 0xc0002e7770?}})
        /Users/artblaginin/GolandProjects/ydbPlayground/vendor/github.com/ydb-platform/ydb-go-sdk-otel/query.go:194 +0xab
github.com/ydb-platform/ydb-go-sdk/v3/trace.(*Query).Compose.func15.2({{0x9d52540?, 0x0?}, {0x9d4a640?, 0xc0002e7770?}})
        /Users/artblaginin/GolandProjects/ydbPlayground/vendor/github.com/ydb-platform/ydb-go-sdk/v3/trace/query_gtrace.go:542 +0xd3
github.com/ydb-platform/ydb-go-sdk/v3/trace.QueryOnSessionCreate.func1({0x9d52540?, 0x0?}, {0x9d4a640?, 0xc0002e7770?})
        /Users/artblaginin/GolandProjects/ydbPlayground/vendor/github.com/ydb-platform/ydb-go-sdk/v3/trace/query_gtrace.go:1596 +0x2b
github.com/ydb-platform/ydb-go-sdk/v3/internal/query.createSession.func3()
        /Users/artblaginin/GolandProjects/ydbPlayground/vendor/github.com/ydb-platform/ydb-go-sdk/v3/internal/query/session.go:102 +0x31
github.com/ydb-platform/ydb-go-sdk/v3/internal/query.createSession({0x9d53ab8, 0xc00015fc70}, {0x9d59800, 0xc0002b79a0}, 0xc000131c20)
        /Users/artblaginin/GolandProjects/ydbPlayground/vendor/github.com/ydb-platform/ydb-go-sdk/v3/internal/query/session.go:107 +0x496
github.com/ydb-platform/ydb-go-sdk/v3/internal/query.New.func1({0x9d53690?, 0xc00015fc20?})
        /Users/artblaginin/GolandProjects/ydbPlayground/vendor/github.com/ydb-platform/ydb-go-sdk/v3/internal/query/client.go:378 +0x14a
github.com/ydb-platform/ydb-go-sdk/v3/internal/query.(*poolStub).With.func2({0x9d53690, 0xc00015fc20})
        /Users/artblaginin/GolandProjects/ydbPlayground/vendor/github.com/ydb-platform/ydb-go-sdk/v3/internal/query/client.go:73 +0x49
github.com/ydb-platform/ydb-go-sdk/v3/retry.Retry.func1({0x9d53690?, 0xc00015fc20?})
        /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[...]({0x9d53690?, 0xc00015fc20?}, 0xc0002b7a50?, 0x0?)
        /Users/artblaginin/GolandProjects/ydbPlayground/vendor/github.com/ydb-platform/ydb-go-sdk/v3/retry/retry.go:409 +0x97
github.com/ydb-platform/ydb-go-sdk/v3/retry.RetryWithResult[...]({0x9d53690, 0xc00015fc20}, 0xc0001c3c30, {0x0, 0x0, 0x15bba0})
        /Users/artblaginin/GolandProjects/ydbPlayground/vendor/github.com/ydb-platform/ydb-go-sdk/v3/retry/retry.go:341 +0x4eb
github.com/ydb-platform/ydb-go-sdk/v3/retry.Retry({0x9d53690?, 0xc00015fc20?}, 0x10?, {0x0?, 0x10?, 0xc000368008?})
        /Users/artblaginin/GolandProjects/ydbPlayground/vendor/github.com/ydb-platform/ydb-go-sdk/v3/retry/retry.go:261 +0x54
github.com/ydb-platform/ydb-go-sdk/v3/internal/query.(*poolStub).With(0x0?, {0x9d53690?, 0xc00015fc20?}, 0x9300685?, {0x0?, 0x9ca25e0?, 0x1?})
        /Users/artblaginin/GolandProjects/ydbPlayground/vendor/github.com/ydb-platform/ydb-go-sdk/v3/internal/query/client.go:72 +0xaa
github.com/ydb-platform/ydb-go-sdk/v3/internal/query.do({0x9d53690, 0xc00015fc20}, {0x9d525a0, 0xc0002b79b0}, 0xc0002e06c0, {0x0, 0x0, 0x0})
        /Users/artblaginin/GolandProjects/ydbPlayground/vendor/github.com/ydb-platform/ydb-go-sdk/v3/internal/query/client.go:117 +0xa3
github.com/ydb-platform/ydb-go-sdk/v3/internal/query.(*Client).ReadRow(0xc0002e69f0, {0x9d53700, 0xc00028e700}, {0x9a074b2, 0x1a}, {0x0, 0x0, 0x0})
        /Users/artblaginin/GolandProjects/ydbPlayground/vendor/github.com/ydb-platform/ydb-go-sdk/v3/internal/query/client.go:214 +0x2f2
main.main()
        /Users/artblaginin/GolandProjects/ydbPlayground/query_readrow/main.go:30 +0x249

Expected behavior: No panic.

Steps to reproduce: In vendor/github.com/ydb-platform/ydb-go-sdk/v3/internal/query/session.go with func createSession when onDone is deferred, there is named returned variable passed to the function. If something goes wrong, the named variable is overwritten to nil and the tracer (ydb-go-sdk-otel in my case) calls *Session.ID() where *Session is already nil. In ydb-go-sdk-otel/internal/safe.ID(id id) func it's checked if the interface id is nil, but it isn't since the covered type is *Session == nil.

To reproduce the panic cancel context after deferred onDone before grpc call in createSession and run the code below.

Related code:

package main

import (
    "context"
    "fmt"
    "log"
    "time"

    ydbOtel "github.com/ydb-platform/ydb-go-sdk-otel"
    "github.com/ydb-platform/ydb-go-sdk/v3"
    "github.com/ydb-platform/ydb-go-sdk/v3/trace"
    "go.opentelemetry.io/otel/trace/noop"
)

func main() {
    ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second)
    defer cancel()

    driver, err := ydb.Open(ctx, "grpc://localhost:2136/local",
        ydbOtel.WithTraces(
            ydbOtel.WithDetails(trace.DetailsAll),
            ydbOtel.WithTracer(noop.NewTracerProvider().Tracer("")),
        ),
    )
    if err != nil {
        log.Fatal("open", err)
    }
    defer driver.Close(ctx)

    row, err := driver.Query().ReadRow(ctx, "SELECT CAST(42 AS Uint32);")
    if err != nil {
        log.Fatal("query", err)
    }

    var i uint32
    if err := row.Scan(&i); err != nil {
        log.Fatal("scan", err)
    }

    fmt.Println(i)
}

createSession snippet:

    ...
onDone := trace.QueryOnSessionCreate(s.cfg.Trace(), &ctx,
        stack.FunctionID("github.com/ydb-platform/ydb-go-sdk/3/internal/query.createSession"),
    )
    defer func() {
        onDone(s, finalErr)
    }()

        ctx, cancel := context.WithCancel(ctx)
        cancel()

    response, err := client.CreateSession(ctx, &Ydb_Query.CreateSessionRequest{})
    if err != nil {
        return nil, xerrors.WithStackTrace(err)
    }
    ...

Other information:

rekby commented 1 month ago

Thanks for the issue. It is bug into ydb-go-sdk-otel repository, bad implementation of safe func.

I saw that and will research it next week.

hurd54 commented 1 month ago

Thanks for the issue. It is bug into ydb-go-sdk-otel repository, bad implementation of safe func. I saw that and will research it next week.

Hi. Please raise the priority this is a blocker for us.