Crash with non-set pipeline.conn inside sendBatchExtendedWithDescription() when doing SendBatch()
- Dominant language
- Go
- Stars
- 14.3k
- Forks
- 1.1k
- Avg merge
- 6d 9h
- Merged PRs (30d)
- 11
Description
**Describe the bug**
I have two parts of my codebase that crash when I use a transaction and call SendBatch(). The transaction comes from a `pgxpool.Pool`.
One is a query and another is a batch insert.
The basics are that I am doing a:
```go
batch := &&pgx.Batch{}
// Pretend I added queries to the Batch here.
if batch.Len() > 0 {
Tx.SendBatch(ctx, batch) // I know Tx is not nil and Tx.Conn() also returns non-nil
}
```
This causes a panic (in both versions 4 and 5):
runtime error: invalid memory address or nil pointer dereference.
github.com/jackc/pgx.(*Conn).sendBatchExtendedWithDescription.func1()
github.com/jackc/pgx/conn.go:1067 +0x1f
panic({0xa589c0, 0x10042e0})
/usr/local/go/src/runtime/panic.go:884 +0x213
github.com/jackc/pgx/pgconn.(*Pipeline).Sync(0xc0181a5900)
github.com/jackc/pgx/pgconn/pgconn.go:1973 +0x1c
github.com/jackc/pgx.(*Conn).sendBatchExtendedWithDescription(0xc0006d7200, {0xc05790?, 0xc00012c000}, 0xc0424e55d8, {0xc00d41e110, 0x1, 0x463685?}, {0xc07fb0, 0xc00b927bf0})
github.com/jackc/pgx/conn.go:1078 +0x6d5
github.com/jackc/pgx.(*Conn).sendBatchQueryExecModeCacheStatement(0xc0006d7200, {0xc05790?, 0xc00012c000}, 0xc0424e55d8)
github.com/jackc/pgx/conn.go:1008 +0x42c
github.com/jackc/pgx.(*Conn).SendBatch(0xc0006d7200, {0xc05790?, 0xc00012c000?}, 0xc0424e55d8)
github.com/jackc/pgx/conn.go:913 +0x5d9
github.com/jackc/pgx.(*dbTx).SendBatch(0xc000134050?, {0xc05790?, 0xc00012c000?}, 0xc0003eed20?)
github.com/jackc/pgx/tx.go:266 +0x76
github.com/jackc/pgx/pgxpool.(*Tx).SendBatch(0xa52c60?, {0xc05790?, 0xc00012c000?}, 0xb?)
github.com/jackc/pgx/pgxpool/tx.go:50 +0x2b
Looking into that code, it says it is in the defer statement. But that's not really the case, that is a second panic caused by the first panic that gets hidden by the defer.
```go
if len(distinctNewQueries) > 0 {
for _, sd := range distinctNewQueries {
pipeline.SendPrepare(sd.Name, sd.SQL, nil) // Real panic is here
}
```
In the code above, SendPrepare() panics, because pipeline.conn is nil. Because the panic still causes defers to happen, the defer goes off, but `pbr` was never set with a `return`, so `pbr.Err` causes another nil pointer dereference inside the defer statement and hides the first panic.
I'm thinking the defer might change to the following to prevent the "panic hiding" that I am seeing:
```go
defer func() {
if pbr != nil && pbr.err != nil {
pipeline.Close()
}
}()
```
With that said, I know that Tx is not nil, but somehow, somewhere the pipeline.conn doesn't get set. I also called Tx.Conn() right before I do the `.SendBatch()`, and it is not nil either.
I've run with `-race`, and nothing weird there.
I'm hoping maybe someone knows what bone headed thing I'm doing that would cause that.
**To Reproduce**
I don't have a reproduction, the code base this is failing in is too big to put here. In essence, the code above is what I'm doing, which means I know I'm doing something terribly wrong, but just failing to see it, because that is the simplest of use cases.
**Version**
- Go: go version go1.20 linux/amd64
- PostgreSQL: 14.7
- pgx: 5.4.2 , but also same problem with 4.x
**Additional context**
I want to take the opportunity to say thank you so much for this project. The level of work that has gone into this is quite amazing and I just want to say thank you.
Contributor guide
Research direction
Start by tracing SendBatch from pgxpool/tx.go through tx.go and conn.go, especially sendBatchExtendedWithDescription and the pgconn.Pipeline calls shown in the stack trace. Reproduce the nil pipeline.conn state with a focused transaction and batch test if possible, then verify that the original panic is preserved and the batch path handles the failure without hiding it.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- go, postgresql
- Domain
- databases
- Issue type
- Bug
- Difficulty
- 4/5
- Estimated time
- 3-5 days
- Activity status
- Stale
- Clarity
- Needs clarification
- Newbie friendliness
- 25/100