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

nil-pointer dereference in getConn #1949

Open
derfenix opened this issue May 23, 2022 · 3 comments
Open

nil-pointer dereference in getConn #1949

derfenix opened this issue May 23, 2022 · 3 comments

Comments

@derfenix
Copy link

derfenix commented May 23, 2022

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/[email protected]/base.go", line 80, in (*baseDB).getConn
  File "/builds/tt/snami/snami/.cache/go/pkg/mod/github.com/go-pg/pg/[email protected]/base.go", line 136, in (*baseDB).withConn
  File "/builds/tt/snami/snami/.cache/go/pkg/mod/github.com/go-pg/pg/[email protected]/base.go", line 245, in (*baseDB).exec
  File "/builds/tt/snami/snami/.cache/go/pkg/mod/github.com/go-pg/pg/[email protected]/base.go", line 220, in (*baseDB).ExecContext
  File "/builds/tt/snami/snami/.cache/go/pkg/mod/github.com/go-pg/pg/[email protected]/base.go", line 119, in (*baseDB).initConn
  File "/builds/tt/snami/snami/.cache/go/pkg/mod/github.com/go-pg/pg/[email protected]/base.go", line 84, in (*baseDB).getConn
  File "/builds/tt/snami/snami/.cache/go/pkg/mod/github.com/go-pg/pg/[email protected]/base.go", line 136, in (*baseDB).withConn
  File "/builds/tt/snami/snami/.cache/go/pkg/mod/github.com/go-pg/pg/[email protected]/base.go", line 315, in (*baseDB).query
  File "/builds/tt/snami/snami/.cache/go/pkg/mod/github.com/go-pg/pg/[email protected]/base.go", line 290, in (*baseDB).QueryContext
@derfenix
Copy link
Author

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

@elliotcourant
Copy link
Collaborator

elliotcourant commented May 23, 2022

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

pg/internal/pool/pool.go

Lines 172 to 174 in e8d0e95

if atomic.LoadUint32(&p.dialErrorsNum) >= uint32(p.opt.PoolSize) {
return nil, p.getLastDialError()
}

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

pg/internal/pool/pool.go

Lines 215 to 220 in e8d0e95

func (p *ConnPool) getLastDialError() error {
p.lastDialErrorMu.RLock()
err := p.lastDialError
p.lastDialErrorMu.RUnlock()
return err
}

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

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.

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

No branches or pull requests

2 participants