Do not use infinite slow write timers

[adriansmares]

Summary

References #1629

This PR removes the use of 'infinite' slow write timers (i.e. timers which expire at the end of time). Previously these slow write timers would be leaked and slowly buildup over time as connections are recycled.

Background

This 'bug' is a bit of a weird feature - the perpetual reference to the net.Conn ensures that the connection is not closed in case the library users don't correctly close the connection, and in turn this is visible when the connection limit is hit.

We discovered this while debugging why some of our tests were failing after #1629. The commit that triggered our issues was cd2986e, so we started to look into what could cause this issue in these changes.

The slowWriteTimer keeps a perpetual reference to the net.Conn (via the background reader). This in turn basically ensures that the net.Conn is never garbage collected, which in turn means that the finalizer is never run. The finalizer for a net.Conn under Unix closes the file descriptor.

What we were doing wrong in our code was that we did not close some rows returned by QueryContext, but we did stop referencing every related to the *sql.DB (after calling Close on it). However *sql.DB works with a reference counting mechanism, and because we did not close the sql.Rows, the underlying connection was never properly closed.

Our application code works correctly both with and without the fix proposed in this PR, but I would still suggest not using these timers, as they are inherently leaky.

Changes

• Instead of using timers that expire at the end of time, simply Stop the timers when they should not fire, and Reset otherwise.

Notes for reviewers

I use gofumpt which is a bit more exigent than gofmt and it picked up some whitespace changes. Let me know if I should remove these.

[drakkan]

Hello,

I'm investigating a similar issue. Unfortunately I cannot reproduce it myself, it was reported by a SFTPGo user. I'll share profiling results later.

Maybe is enough to add

pgConn.bgReader.Stop()
pgConn.slowWriteTimer.Stop()

before return pgConn.conn.Close() here.

The memory leak seems related to the update to pgx 5.4.x. I was using v5.3.2-0.20230520135323-70a200cff4d4 with no issues before.

In my case I always call rows.Close()

[ujihisa]

I also observed memory leak in our work app, and I did a quick git bisect against pgx and found that 26c79eb introduced the memory leak. Not sure if it's the same or not, but I share here just FYI.