ydb-go-sdk icon indicating copy to clipboard operation
ydb-go-sdk copied to clipboard

bug: panic on createSession when returned nil session

Open pelageech opened this issue 7 months ago • 2 comments

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:

pelageech avatar Jul 26 '24 07:07 pelageech