jackc / jackc/pgx

Crash with non-set pipeline.conn inside sendBatchExtendedWithDescription() when doing SendBatch()

Open
#1,689 9 comments 0 reactions 0 assignees View on GitHub
bug
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

Open the contributing 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

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.