From 30199a867c64e96d81fb86aa19b8ad7395aa9a71 Mon Sep 17 00:00:00 2001 From: Stephen Cathcart Date: Thu, 22 Feb 2024 11:44:11 +0000 Subject: [PATCH 1/2] return error if operation is attempted on closed session --- neo4j/session_with_context.go | 21 ++++++++++++++++++++ neo4j/session_with_context_test.go | 32 ++++++++++++++++++++++++++++++ 2 files changed, 53 insertions(+) diff --git a/neo4j/session_with_context.go b/neo4j/session_with_context.go index 18b42b22..2b6336fb 100644 --- a/neo4j/session_with_context.go +++ b/neo4j/session_with_context.go @@ -206,6 +206,7 @@ type sessionWithContext struct { fetchSize int config SessionConfig auth *idb.ReAuthToken + closed bool } func newSessionWithContext( @@ -276,6 +277,13 @@ func (s *sessionWithContext) LastBookmarks() Bookmarks { } func (s *sessionWithContext) BeginTransaction(ctx context.Context, configurers ...func(*TransactionConfig)) (ExplicitTransaction, error) { + + if s.closed { + err := &UsageError{Message: "Operation attempted on a closed session"} + s.log.Error(log.Session, s.logId, err) + return nil, err + } + // Guard for more than one transaction per session if s.explicitTx != nil { err := &UsageError{Message: "Session already has a pending transaction"} @@ -388,6 +396,12 @@ func (s *sessionWithContext) runRetriable( api telemetry.API, configurers ...func(*TransactionConfig)) (any, error) { + if s.closed { + err := &UsageError{Message: "Operation attempted on a closed session"} + s.log.Error(log.Session, s.logId, err) + return nil, err + } + // Guard for more than one transaction per session if s.explicitTx != nil { err := &UsageError{Message: "Session already has a pending transaction"} @@ -580,6 +594,12 @@ func (s *sessionWithContext) retrieveSessionBookmarks(conn idb.Connection) { func (s *sessionWithContext) Run(ctx context.Context, cypher string, params map[string]any, configurers ...func(*TransactionConfig)) (ResultWithContext, error) { + if s.closed { + err := &UsageError{Message: "Operation attempted on a closed session"} + s.log.Error(log.Session, s.logId, err) + return nil, err + } + if s.explicitTx != nil { err := &UsageError{Message: "Trying to run auto-commit transaction while in explicit transaction"} s.log.Error(log.Session, s.logId, err) @@ -676,6 +696,7 @@ func (s *sessionWithContext) Close(ctx context.Context) error { }() <-poolCleanUpChan <-routerCleanUpChan + s.closed = true return txErr } diff --git a/neo4j/session_with_context_test.go b/neo4j/session_with_context_test.go index 108cd977..40cebc7b 100644 --- a/neo4j/session_with_context_test.go +++ b/neo4j/session_with_context_test.go @@ -736,6 +736,38 @@ func TestSession(outer *testing.T) { AssertErrorMessageContains(t, err, "cannot use this transaction") AssertIntEqual(t, poolReturnsCalls, 1) // pool.Return must not be called again }) + + ct.Run("Run returns error if session is closed", func(t *testing.T) { + _, _, sess := createSession() + sess.Close(context.Background()) + _, err := sess.Run(context.Background(), "cypher", nil) + AssertErrorMessageContains(t, err, "Operation attempted on a closed session") + }) + + ct.Run("BeginTransaction returns error if session is closed", func(t *testing.T) { + _, _, sess := createSession() + sess.Close(context.Background()) + _, err := sess.BeginTransaction(context.Background()) + AssertErrorMessageContains(t, err, "Operation attempted on a closed session") + }) + + ct.Run("ExecuteRead returns error if session is closed", func(t *testing.T) { + _, _, sess := createSession() + sess.Close(context.Background()) + _, err := sess.ExecuteRead(context.Background(), func(tx ManagedTransaction) (any, error) { + return nil, nil + }) + AssertErrorMessageContains(t, err, "Operation attempted on a closed session") + }) + + ct.Run("ExecuteWrite returns error if session is closed", func(t *testing.T) { + _, _, sess := createSession() + sess.Close(context.Background()) + _, err := sess.ExecuteWrite(context.Background(), func(tx ManagedTransaction) (any, error) { + return nil, nil + }) + AssertErrorMessageContains(t, err, "Operation attempted on a closed session") + }) }) } From d7096352aeafed613e9939532443e5f9cd3aba2a Mon Sep 17 00:00:00 2001 From: Stephen Cathcart Date: Fri, 23 Feb 2024 10:43:34 +0000 Subject: [PATCH 2/2] adding safeguard for multiple closes on session --- neo4j/session_with_context.go | 5 +++++ neo4j/session_with_context_test.go | 8 ++++++++ 2 files changed, 13 insertions(+) diff --git a/neo4j/session_with_context.go b/neo4j/session_with_context.go index 2b6336fb..aebc5e6d 100644 --- a/neo4j/session_with_context.go +++ b/neo4j/session_with_context.go @@ -674,6 +674,11 @@ func (s *sessionWithContext) Run(ctx context.Context, } func (s *sessionWithContext) Close(ctx context.Context) error { + if s.closed { + // Safeguard against closing more than once + return nil + } + var txErr error if s.explicitTx != nil { txErr = s.explicitTx.Close(ctx) diff --git a/neo4j/session_with_context_test.go b/neo4j/session_with_context_test.go index 40cebc7b..156f3de2 100644 --- a/neo4j/session_with_context_test.go +++ b/neo4j/session_with_context_test.go @@ -768,6 +768,14 @@ func TestSession(outer *testing.T) { }) AssertErrorMessageContains(t, err, "Operation attempted on a closed session") }) + + ct.Run("Session returns nil if closed multiple times", func(t *testing.T) { + _, _, sess := createSession() + err := sess.Close(context.Background()) + AssertNoError(t, err) + err = sess.Close(context.Background()) + AssertNoError(t, err) + }) }) }