Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Stack overflows on upgrading from 5.6.0 to 5.7.1 #2141

Closed
ugexe opened this issue Oct 4, 2024 · 6 comments
Closed

Stack overflows on upgrading from 5.6.0 to 5.7.1 #2141

ugexe opened this issue Oct 4, 2024 · 6 comments
Labels

Comments

@ugexe
Copy link

ugexe commented Oct 4, 2024

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 #1331 (stackoverflow that requires pgx-google-uuid) and #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

  • Go: go version go1.23.1 linux/amd64
  • PostgreSQL: PostgreSQL 16.4 (Ubuntu 16.4-1.pgdg20.04+2) on x86_64-pc-linux-gnu, compiled by gcc (Ubuntu 9.4.0-1ubuntu1~20.04.2) 9.4.0, 64-bit
  • pgx: v5.7.1
@ugexe ugexe added the bug label Oct 4, 2024
@jackc
Copy link
Owner

jackc commented Oct 5, 2024

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

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
Copy link
Owner

jackc commented Oct 5, 2024

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

@jackc
Copy link
Owner

jackc commented Oct 5, 2024

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
Copy link
Owner

jackc commented Oct 5, 2024

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 123b59a.

This should resolve the issue.

@jackc jackc closed this as completed Oct 5, 2024
@zanmato
Copy link

zanmato commented Oct 22, 2024

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

@stefanfrings2
Copy link

stefanfrings2 commented Nov 27, 2024

I got the same issue with a "SELECT ... WHERE ref=$1" statement. In my case, the column ref is a varchar, while the argument for $1 is a UUID. I understand that the argument has wrong type, but that should not cause a stack overflow. Waiting for the next release. Thanks for fixing it.

Edit: We used pgx v5.5.4 before, which worked with the UUID. The SQL queries executed successfully.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

4 participants