From 85aeec85c0d909b03d91c83d2ceb005f75376e67 Mon Sep 17 00:00:00 2001 From: Mergen Imeev Date: Fri, 28 Mar 2025 14:29:10 +0300 Subject: [PATCH] api: fix panic in conn.NewWatcher() Before this patch, `conn.c` was not checked for `nil` before calling its method. This could cause a panic if the connection was lost or closed. Closes #438 --- CHANGELOG.md | 3 ++ connection.go | 2 +- tarantool_test.go | 70 +++++++++++++++++++++++++++++++++++++++++++++-- 3 files changed, 72 insertions(+), 3 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 2c3900ee..a71f999d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -14,6 +14,9 @@ Versioning](http://semver.org/spec/v2.0.0.html) except to the first release. ### Fixed +- Fixed panic when calling NewWatcher() during reconnection or after + connection is closed (#438). + ## [v2.3.2] - 2025-04-14 This release improves the logic of `Connect` and `pool.Connect` in case of a diff --git a/connection.go b/connection.go index 2b43f2ec..5f976fbf 100644 --- a/connection.go +++ b/connection.go @@ -1461,7 +1461,7 @@ func (conn *Connection) NewWatcher(key string, callback WatchCallback) (Watcher, // That's why we can't just check the Tarantool response for an unsupported // request error. if !isFeatureInSlice(iproto.IPROTO_FEATURE_WATCHERS, - conn.c.ProtocolInfo().Features) { + conn.serverProtocolInfo.Features) { err := fmt.Errorf("the feature %s must be supported by connection "+ "to create a watcher", iproto.IPROTO_FEATURE_WATCHERS) return nil, err diff --git a/tarantool_test.go b/tarantool_test.go index ded7ba45..b053c5ae 100644 --- a/tarantool_test.go +++ b/tarantool_test.go @@ -3550,6 +3550,72 @@ func TestConnection_NewWatcher(t *testing.T) { } } +func newWatcherReconnectionPrepareTestConnection(t *testing.T) (*Connection, context.CancelFunc) { + t.Helper() + + const server = "127.0.0.1:3015" + testDialer := dialer + testDialer.Address = server + + inst, err := test_helpers.StartTarantool(test_helpers.StartOpts{ + Dialer: testDialer, + InitScript: "config.lua", + Listen: server, + WaitStart: 100 * time.Millisecond, + ConnectRetry: 10, + RetryTimeout: 500 * time.Millisecond, + }) + t.Cleanup(func() { test_helpers.StopTarantoolWithCleanup(inst) }) + if err != nil { + t.Fatalf("Unable to start Tarantool: %s", err) + } + + ctx, cancel := test_helpers.GetConnectContext() + + reconnectOpts := opts + reconnectOpts.Reconnect = 100 * time.Millisecond + reconnectOpts.MaxReconnects = 0 + reconnectOpts.Notify = make(chan ConnEvent) + conn, err := Connect(ctx, testDialer, reconnectOpts) + if err != nil { + t.Fatalf("Connection was not established: %v", err) + } + + test_helpers.StopTarantool(inst) + + // Wait for reconnection process to be started. + for conn.ConnectedNow() { + time.Sleep(100 * time.Millisecond) + } + + return conn, cancel +} + +func TestNewWatcherDuringReconnect(t *testing.T) { + test_helpers.SkipIfWatchersUnsupported(t) + + conn, cancel := newWatcherReconnectionPrepareTestConnection(t) + defer conn.Close() + defer cancel() + + _, err := conn.NewWatcher("one", func(event WatchEvent) {}) + assert.NotNil(t, err) + assert.ErrorContains(t, err, "client connection is not ready") +} + +func TestNewWatcherAfterClose(t *testing.T) { + test_helpers.SkipIfWatchersUnsupported(t) + + conn, cancel := newWatcherReconnectionPrepareTestConnection(t) + defer cancel() + + _ = conn.Close() + + _, err := conn.NewWatcher("one", func(event WatchEvent) {}) + assert.NotNil(t, err) + assert.ErrorContains(t, err, "using closed connection") +} + func TestConnection_NewWatcher_noWatchersFeature(t *testing.T) { test_helpers.SkipIfWatchersSupported(t) @@ -4149,13 +4215,13 @@ func runTestMain(m *testing.M) int { startOpts.MemtxUseMvccEngine = !isStreamUnsupported inst, err := test_helpers.StartTarantool(startOpts) - defer test_helpers.StopTarantoolWithCleanup(inst) - if err != nil { log.Printf("Failed to prepare test tarantool: %s", err) return 1 } + defer test_helpers.StopTarantoolWithCleanup(inst) + return m.Run() }