Non-racing lock for driver pool (#552)

* Non-racing lock for driver pool

The mutex guarding the driver's pool attribute does not need to be held while
doing IO. Therefore, we can use a standard (non-cancelable) mutex to guard it.
This comes at the benefit of not needing a context for session creation. Session
creation doesn't perform any I/O, so it shouldn't take long anyway. There's
little to no gain in it accepting a context.

* TestKit backend: fix error swallowing introduced in refactor

---------

Co-authored-by: Stephen Cathcart <stephen.cathcart@neotechnology.com>

neo4j/driver_with_context.go

@@ -1,5 +1,3 @@
-// Package neo4j provides required functionality to connect and execute statements against a Neo4j Database.
-
 /*
  * Copyright (c) "Neo4j"
  * Neo4j Sweden AB [https://neo4j.com]
@@ -17,22 +15,23 @@
  * limitations under the License.
  */
 
+// Package neo4j provides required functionality to connect and execute statements against a Neo4j Database.
 package neo4j
 
 import (
 	"context"
 	"fmt"
+	"net/url"
+	"strings"
+	"sync"
+
 	"github.com/neo4j/neo4j-go-driver/v5/neo4j/auth"
 	"github.com/neo4j/neo4j-go-driver/v5/neo4j/internal/connector"
 	idb "github.com/neo4j/neo4j-go-driver/v5/neo4j/internal/db"
 	"github.com/neo4j/neo4j-go-driver/v5/neo4j/internal/errorutil"
 	"github.com/neo4j/neo4j-go-driver/v5/neo4j/internal/pool"
-	"github.com/neo4j/neo4j-go-driver/v5/neo4j/internal/racing"
 	"github.com/neo4j/neo4j-go-driver/v5/neo4j/internal/router"
 	"github.com/neo4j/neo4j-go-driver/v5/neo4j/log"
-	"net/url"
-	"strings"
-	"sync"
 )
 
 // AccessMode defines modes that routing driver decides to which cluster member
@@ -144,7 +143,7 @@ func NewDriverWithContext(target string, auth auth.TokenManager, configurers ...
 		return nil, err
 	}
 
-	d := driverWithContext{target: parsed, mut: racing.NewMutex(), auth: auth}
+	d := driverWithContext{target: parsed, mut: sync.Mutex{}, auth: auth}
 
 	routing := true
 	d.connector.Network = "tcp"
@@ -319,7 +318,7 @@ type driverWithContext struct {
 	target    *url.URL
 	config    *Config
 	pool      *pool.Pool
-	mut       racing.Mutex
+	mut       sync.Mutex
 	connector connector.Connector
 	router    sessionRouter
 	logId     string
@@ -336,7 +335,9 @@ func (d *driverWithContext) Target() url.URL {
 	return *d.target
 }
 
-func (d *driverWithContext) NewSession(ctx context.Context, config SessionConfig) SessionWithContext {
+// TODO 6.0: remove unused Context parameter
+
+func (d *driverWithContext) NewSession(_ context.Context, config SessionConfig) SessionWithContext {
 	if config.DatabaseName == "" {
 		config.DatabaseName = idb.DefaultDatabase
 	}
@@ -356,10 +357,7 @@ func (d *driverWithContext) NewSession(ctx context.Context, config SessionConfig
 		}
 	}
 
-	if !d.mut.TryLock(ctx) {
-		return &erroredSessionWithContext{
-			err: racing.LockTimeoutError("could not acquire lock in time when creating session")}
-	}
+	d.mut.Lock()
 	defer d.mut.Unlock()
 	if d.pool == nil {
 		return &erroredSessionWithContext{
@@ -386,16 +384,18 @@ func (d *driverWithContext) GetServerInfo(ctx context.Context) (_ ServerInfo, er
 }
 
 func (d *driverWithContext) Close(ctx context.Context) error {
-	if !d.mut.TryLock(ctx) {
-		return racing.LockTimeoutError("could not acquire lock in time when closing driver")
-	}
-	defer d.mut.Unlock()
-	// Safeguard against closing more than once
-	if d.pool != nil {
-		d.pool.Close(ctx)
-		d.pool = nil
-		d.log.Infof(log.Driver, d.logId, "Closed")
+	d.mut.Lock()
+	if d.pool == nil {
+		// Safeguard against closing more than once
+		return nil
 	}
+	pool := d.pool
+	d.pool = nil
+	d.mut.Unlock()
+
+	pool.Close(ctx)
+	pool = nil
+	d.log.Infof(log.Driver, d.logId, "Closed")
 	return nil
 }
 

neo4j/driver_with_context_test.go

@@ -21,7 +21,6 @@ import (
 	"context"
 	"errors"
 	"fmt"
-	"github.com/neo4j/neo4j-go-driver/v5/neo4j/internal/racing"
 	. "github.com/neo4j/neo4j-go-driver/v5/neo4j/internal/testutil"
 	"net/url"
 	"sync"
@@ -415,7 +414,7 @@ func TestDriverExecuteQuery(outer *testing.T) {
 				},
 				delegate: &driverWithContext{
 					executeQueryBookmarkManager: defaultBookmarkManager,
-					mut:                         racing.NewMutex(),
+					mut:                         sync.Mutex{},
 				},
 			}
 
@@ -435,7 +434,7 @@ func TestDriverExecuteQuery(outer *testing.T) {
 				}
 			},
 			delegate: &driverWithContext{
-				mut: racing.NewMutex(),
+				mut: sync.Mutex{},
 			},
 		}