Skip to content

Commit b7c87d8

Browse files
authored
fix(go): complete TCP client teardown when conn.Close fails (#3670)
Golang SDK TCP client returns the error directly when `c.conn.Close()` returns an error. This stops it from finishing the disconnect and shutdown process.
1 parent 02c5e4f commit b7c87d8

2 files changed

Lines changed: 74 additions & 18 deletions

File tree

foreign/go/client/tcp/tcp_core.go

Lines changed: 17 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -433,12 +433,20 @@ func (c *IggyTcpClient) sendLocked(wirePayload []byte) ([]byte, error) {
433433

434434
// invalidateConnLocked closes the connection and marks it as disconnected
435435
func (c *IggyTcpClient) invalidateConnLocked() {
436-
if c.conn != nil {
437-
_ = c.conn.Close()
438-
}
436+
_ = c.closeConnLocked()
439437
c.state = iggcon.StateDisconnected
440438
}
441439

440+
// closeConnLocked closes and drops the current connection.
441+
func (c *IggyTcpClient) closeConnLocked() error {
442+
if c.conn == nil {
443+
return nil
444+
}
445+
err := c.conn.Close()
446+
c.conn = nil
447+
return err
448+
}
449+
442450
func (c *IggyTcpClient) GetConnectionInfo() *iggcon.ConnectionInfo {
443451
c.mtx.Lock()
444452
defer c.mtx.Unlock()
@@ -610,22 +618,17 @@ func (c *IggyTcpClient) disconnect() error {
610618
c.mtx.Lock()
611619
defer c.mtx.Unlock()
612620

613-
if c.state == iggcon.StateDisconnected {
621+
if c.state == iggcon.StateDisconnected || c.state == iggcon.StateShutdown {
614622
return nil
615623
}
616624

617625
c.logger.Info("Iggy client is disconnecting from server...", slog.String("client_address", c.clientAddress))
618626
c.state = iggcon.StateDisconnected
619-
620-
if c.conn != nil {
621-
if err := c.conn.Close(); err != nil {
622-
return err
623-
}
624-
}
627+
err := c.closeConnLocked()
625628

626629
c.logger.Info("Iggy client has disconnected from server.", slog.String("client_address", c.clientAddress))
627630
// TODO event pushing logic
628-
return nil
631+
return err
629632
}
630633

631634
func (c *IggyTcpClient) shutdown() error {
@@ -638,16 +641,12 @@ func (c *IggyTcpClient) shutdown() error {
638641

639642
c.logger.Info("Shutting down the Iggy TCP client...", slog.String("client_address", c.clientAddress))
640643

641-
if c.conn != nil {
642-
if err := c.conn.Close(); err != nil {
643-
return err
644-
}
645-
}
646-
644+
err := c.closeConnLocked()
647645
c.state = iggcon.StateShutdown
646+
648647
c.logger.Info("Iggy TCP client has been shutdown.", slog.String("client_address", c.clientAddress))
649648
// TODO push shutdown event
650-
return nil
649+
return err
651650
}
652651

653652
func (c *IggyTcpClient) Close() error {

foreign/go/client/tcp/tcp_core_test.go

Lines changed: 57 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -272,3 +272,60 @@ func TestNewIggyTcpClient_StoresProvidedLogger(t *testing.T) {
272272
t.Errorf("expected logger output to contain 'source=tcp', got: %q", output)
273273
}
274274
}
275+
276+
var errCloseFailed = errors.New("close failed")
277+
278+
// failingCloseConn is a connection whose Close always fails, standing in for a
279+
// socket whose teardown reports an error the client cannot act on.
280+
type failingCloseConn struct {
281+
net.Conn
282+
closes int
283+
}
284+
285+
func (f *failingCloseConn) Close() error {
286+
f.closes++
287+
return errCloseFailed
288+
}
289+
290+
func TestShutdown_FailedCloseStillCompletesTeardown(t *testing.T) {
291+
c, _ := newTestClient(t)
292+
conn := &failingCloseConn{Conn: c.conn}
293+
c.conn = conn
294+
295+
if err := c.shutdown(); !errors.Is(err, errCloseFailed) {
296+
t.Fatalf("got %v, want %v", err, errCloseFailed)
297+
}
298+
if c.state != iggcon.StateShutdown {
299+
t.Errorf("expected state %v, got %v", iggcon.StateShutdown, c.state)
300+
}
301+
if c.conn != nil {
302+
t.Error("expected the closed connection to be dropped")
303+
}
304+
305+
if err := c.shutdown(); err != nil {
306+
t.Errorf("expected the second shutdown to be a no-op, got %v", err)
307+
}
308+
if conn.closes != 1 {
309+
t.Errorf("expected the connection to be closed once, got %d", conn.closes)
310+
}
311+
}
312+
313+
func TestDisconnect_ShutdownClientIsNotResurrected(t *testing.T) {
314+
c, _ := newTestClient(t)
315+
316+
if err := c.shutdown(); err != nil {
317+
t.Fatalf("unexpected shutdown error: %v", err)
318+
}
319+
if err := c.disconnect(); err != nil {
320+
t.Fatalf("unexpected disconnect error: %v", err)
321+
}
322+
323+
if c.state != iggcon.StateShutdown {
324+
t.Errorf("expected state to stay %v, got %v", iggcon.StateShutdown, c.state)
325+
}
326+
327+
_, err := c.sendWireAndFetchResponse(context.Background(), emptyWireReq)
328+
if !errors.Is(err, ierror.ErrClientShutdown) {
329+
t.Errorf("got %v, want %v", err, ierror.ErrClientShutdown)
330+
}
331+
}

0 commit comments

Comments
 (0)