search

go-pg / pg

Golang ORM with focus on PostgreSQL features and performance
https://pg.uptrace.dev/
BSD 2-Clause "Simplified" License
5.65k stars 401 forks source link

nil-pointer dereference in getConn #1949

Open derfenix opened 2 years ago

derfenix commented 2 years ago

Have no idea how to handle or investigate this panic. 

runtime.errorString: runtime error: invalid memory address or nil pointer dereference
  File "/builds/tt/snami/snami/v2/interfaces/rest/middleware/sentry.go", line 158, in recoverWithSentry
  File "/builds/tt/snami/snami/.cache/go/pkg/mod/github.com/go-pg/pg/v10@v10.10.6/base.go", line 80, in (*baseDB).getConn
  File "/builds/tt/snami/snami/.cache/go/pkg/mod/github.com/go-pg/pg/v10@v10.10.6/base.go", line 136, in (*baseDB).withConn
  File "/builds/tt/snami/snami/.cache/go/pkg/mod/github.com/go-pg/pg/v10@v10.10.6/base.go", line 245, in (*baseDB).exec
  File "/builds/tt/snami/snami/.cache/go/pkg/mod/github.com/go-pg/pg/v10@v10.10.6/base.go", line 220, in (*baseDB).ExecContext
  File "/builds/tt/snami/snami/.cache/go/pkg/mod/github.com/go-pg/pg/v10@v10.10.6/base.go", line 119, in (*baseDB).initConn
  File "/builds/tt/snami/snami/.cache/go/pkg/mod/github.com/go-pg/pg/v10@v10.10.6/base.go", line 84, in (*baseDB).getConn
  File "/builds/tt/snami/snami/.cache/go/pkg/mod/github.com/go-pg/pg/v10@v10.10.6/base.go", line 136, in (*baseDB).withConn
  File "/builds/tt/snami/snami/.cache/go/pkg/mod/github.com/go-pg/pg/v10@v10.10.6/base.go", line 315, in (*baseDB).query
  File "/builds/tt/snami/snami/.cache/go/pkg/mod/github.com/go-pg/pg/v10@v10.10.6/base.go", line 290, in (*baseDB).QueryContext
derfenix commented 2 years ago

Seems like nil conection returned from the pool, but have no idea how it's possible

elliotcourant commented 2 years ago

It looks like there is a code path that might allow both the err and the connection to be nil.

https://github.com/go-pg/pg/blob/e8d0e95cbd9239859683c3b932c520616000edb1/internal/pool/pool.go#L172-L174

If there are more dial errors than the "pool size" then it will return the last dial error.

https://github.com/go-pg/pg/blob/e8d0e95cbd9239859683c3b932c520616000edb1/internal/pool/pool.go#L215-L220

But if the last dial error is nil for whatever reason, then this function could return nil, nil causing such a panic.

Looking at this, this seems to be the only way that you could get a nil connection without an error path. But I don't see an immediate way that that error could be nil under those circumstances.

derfenix commented 2 years ago

Found a conn.Execute in onConnect handler in legacy code. May be this thing can lead to the nil conn in case of full pool? Reproduced only on prod, so trying to reproduce it locally first with no luck yet.