jackc / pgx

PostgreSQL driver and toolkit for Go
MIT License
10.83k stars 845 forks source link

Stack overflows on upgrading from 5.6.0 to 5.7.1 #2141

Closed ugexe closed 1 month ago

ugexe commented 1 month ago

Describe the bug After upgrading from 5.6.0 to 5.7.1 the following program results in a stackoverflow. It seems somewhat related to https://github.com/jackc/pgx/issues/1331 (stackoverflow that requires pgx-google-uuid) and https://github.com/jackc/pgx/issues/2125 (custom type and jsonb, although they state that issue is in 5.6.0 whereas my issue did not occur in 5.6.0).

To Reproduce

package main

import (
    "context"
    "fmt"
    "log"
    "os"

    pgxUUID "github.com/vgarvardt/pgx-google-uuid/v5"

    "github.com/google/uuid"
    "github.com/jackc/pgx/v5"
    "github.com/jackc/pgx/v5/stdlib"
)

func main() {
    connConfig, err := pgx.ParseConfig(os.Getenv("DATABASE_URL"))
    if err != nil {
        log.Fatal(err)
    }

    db := stdlib.OpenDB(*connConfig, stdlib.OptionAfterConnect(
        func(ctx context.Context, conn *pgx.Conn) error {
            info := conn.TypeMap()
            pgxUUID.Register(info)
            return nil
        }),
    )
    defer db.Close()

    uuid := uuid.New()

    var exists bool
    err = db.QueryRow(
        `SELECT '{"id": "`+uuid.String()+`"}'::jsonb ->> 'id' = $1`,
        uuid,
    ).Scan(&exists)
    if err != nil {
        log.Fatal(err)
    }

    fmt.Println("Exists:", exists)
}

Expected behavior

Exists: true

Actual behavior

runtime: goroutine stack exceeds 1000000000-byte limit
runtime: sp=0xc020260320 stack=[0xc020260000, 0xc040260000]
fatal error: stack overflow

runtime stack:
runtime.throw({0x8ad878?, 0x7fc54a75ad70?})
    /usr/local/go/src/runtime/panic.go:1067 +0x48 fp=0x7fc54a75ad30 sp=0x7fc54a75ad00 pc=0x46c908
runtime.newstack()
    /usr/local/go/src/runtime/stack.go:1117 +0x5bd fp=0x7fc54a75ae70 sp=0x7fc54a75ad30 pc=0x45125d
runtime.morestack()
    /usr/local/go/src/runtime/asm_amd64.s:621 +0x7a fp=0x7fc54a75ae78 sp=0x7fc54a75ae70 pc=0x47255a

goroutine 1 gp=0xc0000061c0 m=3 mp=0xc000080e08 [running]:
runtime.newobject(0x838560?)
    /usr/local/go/src/runtime/malloc.go:1385 +0x35 fp=0xc020260330 sp=0xc020260328 pc=0x40f035
github.com/jackc/pgx/v5/pgtype.TryWrapBuiltinTypeEncodePlan({0x8065c0?, 0xc003e9b870})
    /home/ugexe/go/pkg/mod/github.com/jackc/pgx/v5@v5.7.1/pgtype/pgtype.go:1502 +0x473 fp=0xc0202605a0 sp=0xc020260330 pc=0x5c4ab3
github.com/jackc/pgx/v5/pgtype.(*Map).planEncode(0xc000155110, 0x19, 0x0, {0x8065c0, 0xc003e9b870})
    /home/ugexe/go/pkg/mod/github.com/jackc/pgx/v5@v5.7.1/pgtype/pgtype.go:1251 +0x189 fp=0xc020260608 sp=0xc0202605a0 pc=0x5c31c9
github.com/jackc/pgx/v5/pgtype.(*Map).PlanEncode(0xc000155110, 0x19, 0x0, {0x8065c0, 0xc003e9b870})
    /home/ugexe/go/pkg/mod/github.com/jackc/pgx/v5@v5.7.1/pgtype/pgtype.go:1213 +0x192 fp=0xc0202606b0 sp=0xc020260608 pc=0x5c2f32
github.com/jackc/pgx/v5/pgtype.(*Map).planEncode(0xc000155110, 0x19, 0x0, {0x827200, 0xc003e9b870})
    /home/ugexe/go/pkg/mod/github.com/jackc/pgx/v5@v5.7.1/pgtype/pgtype.go:1252 +0x1b1 fp=0xc020260718 sp=0xc0202606b0 pc=0x5c31f1
github.com/jackc/pgx/v5/pgtype.(*Map).PlanEncode(0xc000155110, 0x19, 0x0, {0x827200, 0xc003e9b870})
    /home/ugexe/go/pkg/mod/github.com/jackc/pgx/v5@v5.7.1/pgtype/pgtype.go:1213 +0x192 fp=0xc0202607c0 sp=0xc020260718 pc=0x5c2f32
github.com/jackc/pgx/v5/pgtype.(*Map).planEncode(0xc000155110, 0x19, 0x0, {0x8065c0, 0xc003e9b860})
    /home/ugexe/go/pkg/mod/github.com/jackc/pgx/v5@v5.7.1/pgtype/pgtype.go:1252 +0x1b1 fp=0xc020260828 sp=0xc0202607c0 pc=0x5c31f1
github.com/jackc/pgx/v5/pgtype.(*Map).PlanEncode(0xc000155110, 0x19, 0x0, {0x8065c0, 0xc003e9b860})
    /home/ugexe/go/pkg/mod/github.com/jackc/pgx/v5@v5.7.1/pgtype/pgtype.go:1213 +0x192 fp=0xc0202608d0 sp=0xc020260828 pc=0x5c2f32
github.com/jackc/pgx/v5/pgtype.(*Map).planEncode(0xc000155110, 0x19, 0x0, {0x827200, 0xc003e9b860})
    /home/ugexe/go/pkg/mod/github.com/jackc/pgx/v5@v5.7.1/pgtype/pgtype.go:1252 +0x1b1 fp=0xc020260938 sp=0xc0202608d0 pc=0x5c31f1
github.com/jackc/pgx/v5/pgtype.(*Map).PlanEncode(0xc000155110, 0x19, 0x0, {0x827200, 0xc003e9b860})
    /home/ugexe/go/pkg/mod/github.com/jackc/pgx/v5@v5.7.1/pgtype/pgtype.go:1213 +0x192 fp=0xc0202609e0 sp=0xc020260938 pc=0x5c2f32

...

Version

jackc commented 1 month ago

I used git bisect and found that the issue was introduced in 5747f37d9cd5ba79cfe6209f79b0c1b074f11fe6.

I've also found that json(b) has nothing to do with it. SELECT 'foobar' = $1 has the same issue. It is something to do with encoding into text.

Still investigating.

jackc commented 1 month ago

I'm also able to reproduce it with pgx directly without going through stdlib.

jackc commented 1 month ago

I've now been able to reproduce it without github.com/vgarvardt/pgx-google-uuid/v5. I think I'm close to understanding the issue.

New repro code:

package main

import (
    "context"
    "fmt"
    "log"
    "os"

    "github.com/jackc/pgx/v5"
)

func main() {
    conn, err := pgx.Connect(context.Background(), os.Getenv("DATABASE_URL"))
    defer conn.Close(context.Background())
    uuid := [16]byte{}

    var exists bool
    err = conn.QueryRow(
        context.Background(),
        `SELECT 'foobar' = $1`,
        uuid,
    ).Scan(&exists)
    if err != nil {
        log.Fatal(err)
    }

    fmt.Println("Exists:", exists)
}
jackc commented 1 month ago

Okay, I think I understand what is going on now.

pgx tries really hard to "just work" with whatever types you use. Part of how that works is when it doesn't know how to encode a particular Go type into a particular PostgreSQL type it tries various strategies to convert the Go value into a type that it does know how to handle. For example, it will dereference pointers and it convert values into their underlying types. For example, for type T int64 pgx doesn't know how to encode a T into PostgreSQL, but it figures out that it can be converted into an int64, and it does know how to encode that.

This process is attempted recursively. Here lies the problem. Your query is not actually attempting to send auuid to PostgreSQL. It is sending a text. pgx was getting caught on a path where it tried to wrap the uuid in something that maybe it could encode as text. But when it couldn't, it kept trying down that path, and unwrapped the value into a base Go type. And it continued recursively wrapping and unwrapping the value until it encountered a stack overflow.

I've implemented a depth check to the planning systems for encoding and scanning where they will give up when they reach a certain level. See 123b59a57e453d73779eeb5a74ec25195da7d34b.

This should resolve the issue.

zanmato commented 1 month ago

This also occurs when doing something like SELECT COALESCE($2, $1) where both $1 and $2 are actual uuids,

package main

import (
    "context"
    "fmt"
    "log"
    "os"

    "github.com/gofrs/uuid/v5"
    pgxuuid "github.com/jackc/pgx-gofrs-uuid"
    "github.com/jackc/pgx/v5"
)

func main() {
    ctx := context.Background()
    conn, err := pgx.Connect(ctx, os.Getenv("DATABASE_URL"))
    if err != nil {
        log.Fatal(err)
    }
    defer conn.Close(ctx)

    pgxuuid.Register(conn.TypeMap())

    uuid1 := uuid.Must(uuid.NewV4())
    var uuid2 *uuid.UUID

    var r *uuid.UUID
    if err := conn.QueryRow(
        ctx,
        `SELECT COALESCE($2, $1)`,
        uuid1,
        uuid2,
    ).Scan(&r); err != nil {
        log.Fatal(err)
    }

    fmt.Println("ret:", r)
}

Edit: my actual code did something like INSERT INTO table (id) VALUES(COALESCE($1, $2)) which led me here when upgrading from v4 to v5