Skip to content

Commit

Permalink
Merge pull request #1587 from ydb-platform/issue-1585
Browse files Browse the repository at this point in the history
fixed bug with multiple closing driver
  • Loading branch information
asmyasnikov authored Dec 13, 2024
2 parents 636ca2f + 4703bb7 commit f282329
Show file tree
Hide file tree
Showing 5 changed files with 33 additions and 0 deletions.
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
* Fixed panic on multiple closing driver

## v3.95.1
* Added alias from `ydb.WithFakeTx(ydb.ScriptingQueryMode)` to `ydb.WithFakeTx(ydb.QueryExecuteQueryMode)` for compatibility with legacy code

Expand Down
7 changes: 7 additions & 0 deletions driver.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import (
"fmt"
"os"
"sync"
"sync/atomic"

"google.golang.org/grpc"

Expand Down Expand Up @@ -103,6 +104,7 @@ type (
children map[uint64]*Driver
childrenMtx xsync.Mutex
onClose []func(c *Driver)
closed atomic.Bool

panicCallback func(e interface{})
}
Expand Down Expand Up @@ -149,6 +151,11 @@ func (d *Driver) Close(ctx context.Context) (finalErr error) {
defer func() {
onDone(finalErr)
}()

if !d.closed.CompareAndSwap(false, true) {
return nil
}

d.ctxCancel()

d.mtx.Lock()
Expand Down
4 changes: 4 additions & 0 deletions internal/query/client.go
Original file line number Diff line number Diff line change
Expand Up @@ -188,6 +188,10 @@ func (c *Client) ExecuteScript(
}

func (c *Client) Close(ctx context.Context) error {
if c == nil {
return xerrors.WithStackTrace(errNilClient)
}

close(c.done)

if err := c.pool.Close(ctx); err != nil {
Expand Down
1 change: 1 addition & 0 deletions internal/query/errors.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import (
)

var (
errNilClient = xerrors.Wrap(errors.New("table client is not initialized"))
ErrTransactionRollingBack = xerrors.Wrap(errors.New("ydb: the transaction is rolling back"))
errWrongNextResultSetIndex = errors.New("wrong result set index")
errWrongResultSetIndex = errors.New("critical violation of the logic - wrong result set index")
Expand Down
19 changes: 19 additions & 0 deletions tests/integration/driver_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -651,3 +651,22 @@ func TestClusterDiscoveryRetry(t *testing.T) {
}
require.Greater(t, counter, 1)
}

func TestMultipleClosingDriverIssue1585(t *testing.T) {
ctx, cancel := context.WithCancel(context.Background())
defer cancel()

db, err := ydb.Open(ctx, os.Getenv("YDB_CONNECTION_STRING"))
require.NoError(t, err)

require.NotPanics(t, func() {
err = db.Close(ctx)
require.NoError(t, err)

err = db.Close(ctx)
require.NoError(t, err)

err = db.Close(ctx)
require.NoError(t, err)
})
}

0 comments on commit f282329

Please sign in to comment.