Connection pool is not safe

[CodyDWJones]

Describe the bug The connection pool is only useful when writing concurrent code, but it doesn't actually stop multiple goroutines from accessing the same connection simultaneously.

To Reproduce The PR includes a test suite that reproduces the issue with the old Pool code, so long as it's edited to include the afterAcquire and beforeRelease test hooks.

Expected behavior

• Pool.Exec(), Pool.Ping(), Pool.Query(), and Pool.Server() are safe for concurrent use. Two calls can only use the same connection serially.
• When a query is submitted to the pool,
  • If there is an unused connection in the pool, it will be used to run the query immediately.
  • Otherwise, the pool will wait for a connection to become available and then run the query.
• A connection is only returned to the pool when it is safe to do so.
  • Cursors perform their work via the connection, which means if the pool returns a cursor, the connection cannot be returned to the pool until the cursor is closed.
• A mutex is not used in the hot path as per the expectation set in #488.

Screenshots n/a

System info n/a

Additional context Problems with the prior pool implementation:

• Pool.conn() uses the connections in round-robin order without checking if they are still in use. It appears queries and cursors can be multiplexed on one connection, so this would work fine in the single goroutine case. However, if multiple goroutines call the Pool there is no guarantee each Connection will only be used by one goroutine at a time.

• Similarly there is a race condition in the lines:

  pos := atomic.AddInt32(&p.pointer, 1)
  if pos == int32(len(p.conns)) {
  atomic.StoreInt32(&p.pointer, 0)
  }
  pos = pos % int32(len(p.conns))

Suppose pointer == 9 and len(conns) == 10. Three goroutines could run in this sequence:

GR 1: pointer=10                pointer=0                uses conns[0]
GR 2:              pointer=11                            uses conns[1]
GR 3:                                       pointer=1    uses conns[1]
      ---------time------->

Because there is no "in use" check, two of the goroutines end up using the same Connection.

[srh]

It sounds like a legitimate bug report. I don't have a timeline for a fix.

[CodyDWJones]

I have just submitted a proposed fix in #498.

[CodyDWJones]

Not an issue; the Connection and Cursor docs say they do not support concurrent calls, but those comments are very old and aren't actually true.