From 57c1a207e9d6efe40baba5b098cc4007530ec10e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Frank=20Schr=C3=B6der?= Date: Sat, 25 Sep 2021 08:33:49 +0200 Subject: [PATCH 1/2] guard against double close This patch guards the Close() method of the secure channel to be called more than once. This seems like a workaround I would like to remove once we understand the root cause of the problem. Fixes #430 --- uasc/secure_channel.go | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/uasc/secure_channel.go b/uasc/secure_channel.go index ac4796bf..00395b62 100644 --- a/uasc/secure_channel.go +++ b/uasc/secure_channel.go @@ -115,6 +115,8 @@ type SecureChannel struct { // errorCh receive dispatcher errors errCh chan<- error + + closeOnce sync.Once } func NewSecureChannel(endpoint string, c *uacp.Conn, cfg *Config, errCh chan<- error) (*SecureChannel, error) { @@ -808,7 +810,13 @@ func (s *SecureChannel) nextRequestID() uint32 { } // Close closes an existing secure channel -func (s *SecureChannel) Close() error { +func (s *SecureChannel) Close() (err error) { + err = io.EOF + s.closeOnce.Do(func() { err = s.close() }) + return +} + +func (s *SecureChannel) close() error { debug.Printf("uasc Close()") defer func() { From 6689798a0e902ee6957f837f5cec62ab451a95ed Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Frank=20Schr=C3=B6der?= Date: Mon, 27 Sep 2021 14:33:28 +0200 Subject: [PATCH 2/2] guard uacp.Conn against double close --- client.go | 8 ++++++++ uacp/conn.go | 13 +++++++++++-- uasc/secure_channel.go | 2 ++ 3 files changed, 21 insertions(+), 2 deletions(-) diff --git a/client.go b/client.go index 77286b18..8712a770 100644 --- a/client.go +++ b/client.go @@ -299,6 +299,14 @@ func (c *Client) monitor(ctx context.Context) { // a reconnection to the server // close previous secure channel + // + // todo(fs): the two calls to Close() trigger a double-close on both the + // todo(fs): secure channel and the UACP connection. I have guarded for this + // todo(fs): with a sync.Once but that feels like a band-aid. We need to investigate + // todo(fs): why we are trying to create a new secure channel when we shut the client + // todo(fs): down. + // + // https://github.com/gopcua/opcua/pull/470 _ = c.conn.Close() c.sechan.Close() c.sechan = nil diff --git a/uacp/conn.go b/uacp/conn.go index e6a17fa7..379c1c26 100644 --- a/uacp/conn.go +++ b/uacp/conn.go @@ -9,6 +9,7 @@ import ( "fmt" "io" "net" + "sync" "sync/atomic" "github.com/gopcua/opcua/debug" @@ -144,7 +145,7 @@ func (l *Listener) Accept(ctx context.Context) (*Conn, error) { if err != nil { return nil, err } - conn := &Conn{c, nextid(), l.ack} + conn := &Conn{TCPConn: c, id: nextid(), ack: l.ack} if err := conn.srvhandshake(l.endpoint); err != nil { c.Close() return nil, err @@ -171,6 +172,8 @@ type Conn struct { *net.TCPConn id uint32 ack *Acknowledge + + closeOnce sync.Once } func NewConn(c *net.TCPConn, ack *Acknowledge) (*Conn, error) { @@ -203,7 +206,13 @@ func (c *Conn) MaxChunkCount() uint32 { return c.ack.MaxChunkCount } -func (c *Conn) Close() error { +func (c *Conn) Close() (err error) { + err = io.EOF + c.closeOnce.Do(func() { err = c.close() }) + return err +} + +func (c *Conn) close() error { debug.Printf("conn %d: close", c.id) return c.TCPConn.Close() } diff --git a/uasc/secure_channel.go b/uasc/secure_channel.go index 00395b62..75074e28 100644 --- a/uasc/secure_channel.go +++ b/uasc/secure_channel.go @@ -811,6 +811,8 @@ func (s *SecureChannel) nextRequestID() uint32 { // Close closes an existing secure channel func (s *SecureChannel) Close() (err error) { + // https://github.com/gopcua/opcua/pull/470 + // guard against double close until we found the root cause err = io.EOF s.closeOnce.Do(func() { err = s.close() }) return