Skip to content

Commit 9eecbcb

Browse files
committed
fix(client): rework client close
Replace nil conn with explicit bool and reduce possible NPE cases. Ref: #301
1 parent 14d3048 commit 9eecbcb

File tree

3 files changed

+15
-13
lines changed

3 files changed

+15
-13
lines changed

Diff for: client.go

+12-10
Original file line numberDiff line numberDiff line change
@@ -29,14 +29,16 @@ import (
2929
type Client struct {
3030
lg *zap.Logger
3131
conn net.Conn
32-
mux sync.Mutex
3332
buf *proto.Buffer
3433
reader *proto.Reader
3534
info proto.ClientHello
3635
server proto.ServerHello
3736
version clientVersion
3837
quotaKey string
3938

39+
mux sync.Mutex
40+
closed bool
41+
4042
// Single packet read timeout.
4143
readTimeout time.Duration
4244

@@ -80,14 +82,14 @@ var ErrClosed = errors.New("client is closed")
8082
// Close closes underlying connection and frees all resources,
8183
// rendering Client to unusable state.
8284
func (c *Client) Close() error {
83-
if c.conn == nil {
85+
c.mux.Lock()
86+
defer c.mux.Unlock()
87+
88+
if c.closed {
8489
return ErrClosed
8590
}
86-
defer func() {
87-
c.buf = nil
88-
c.reader = nil
89-
c.conn = nil
90-
}()
91+
92+
c.closed = true
9193
if err := c.conn.Close(); err != nil {
9294
return errors.Wrap(err, "conn")
9395
}
@@ -97,7 +99,9 @@ func (c *Client) Close() error {
9799

98100
// IsClosed indicates that connection is closed.
99101
func (c *Client) IsClosed() bool {
100-
return c.conn == nil
102+
c.mux.Lock()
103+
defer c.mux.Unlock()
104+
return c.closed
101105
}
102106

103107
// Exception is server-side error.
@@ -251,8 +255,6 @@ func (c *Client) packet(ctx context.Context) (proto.ServerCode, error) {
251255
}
252256

253257
func (c *Client) flushBuf(ctx context.Context, b *proto.Buffer) error {
254-
c.mux.Lock()
255-
defer c.mux.Unlock()
256258
if err := ctx.Err(); err != nil {
257259
return errors.Wrap(err, "context")
258260
}

Diff for: ping.go

+1-1
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@ import (
1515
//
1616
// Do not call concurrently with Do.
1717
func (c *Client) Ping(ctx context.Context) (err error) {
18-
if c.conn == nil {
18+
if c.IsClosed() {
1919
return ErrClosed
2020
}
2121
if c.otel {

Diff for: query.go

+2-2
Original file line numberDiff line numberDiff line change
@@ -75,7 +75,7 @@ func (c *Client) sendQuery(ctx context.Context, q Query) error {
7575
zap.String("query_id", q.QueryID),
7676
)
7777
}
78-
if c.conn == nil {
78+
if c.IsClosed() {
7979
return ErrClosed
8080
}
8181
c.encode(proto.Query{
@@ -537,7 +537,7 @@ func (c *Client) handlePacket(ctx context.Context, p proto.ServerCode, q Query)
537537

538538
// Do performs Query on ClickHouse server.
539539
func (c *Client) Do(ctx context.Context, q Query) (err error) {
540-
if c.conn == nil {
540+
if c.IsClosed() {
541541
return ErrClosed
542542
}
543543
if len(q.Parameters) > 0 && !proto.FeatureParameters.In(c.protocolVersion) {

0 commit comments

Comments
 (0)