From a68bb655f0963aeb6060bfd3606f7a9db8258ce3 Mon Sep 17 00:00:00 2001 From: Thomas Stevens Date: Mon, 30 Mar 2020 08:48:33 -0400 Subject: [PATCH 01/10] Make shutdown method public --- electrum/network.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/electrum/network.go b/electrum/network.go index d7e8114..d950231 100644 --- a/electrum/network.go +++ b/electrum/network.go @@ -224,7 +224,7 @@ func (s *Server) listen() { select { case err := <-s.transport.Errors(): s.Error <- err - s.shutdown() + s.Shutdown() case bytes := <-s.transport.Responses(): result := &container{ content: bytes, @@ -336,7 +336,7 @@ func (s *Server) request(method string, params []interface{}, v interface{}) err return nil } -func (s *Server) shutdown() { +func (s *Server) Shutdown() { close(s.quit) s.transport = nil From 98a789fc3048e75a7a8538014e30280317156298 Mon Sep 17 00:00:00 2001 From: Thomas Stevens Date: Mon, 30 Mar 2020 09:16:46 -0400 Subject: [PATCH 02/10] Improve shutdown method --- electrum/network.go | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/electrum/network.go b/electrum/network.go index d950231..05cd3eb 100644 --- a/electrum/network.go +++ b/electrum/network.go @@ -50,6 +50,7 @@ type Transport interface { SendMessage([]byte) error Responses() <-chan []byte Errors() <-chan error + Close() error } // TCPTransport store informations about the TCP transport. @@ -136,6 +137,10 @@ func (t *TCPTransport) Errors() <-chan error { return t.errors } +func (t *TCPTransport) Close() error { + return t.conn.Close() +} + type container struct { content []byte err error @@ -338,7 +343,7 @@ func (s *Server) request(method string, params []interface{}, v interface{}) err func (s *Server) Shutdown() { close(s.quit) - + _ = s.transport.Close() s.transport = nil s.handlers = nil s.pushHandlers = nil From bab54e341282bd6de15bcbf76aa3c8c199343da2 Mon Sep 17 00:00:00 2001 From: Thomas Stevens Date: Mon, 30 Mar 2020 20:07:45 -0400 Subject: [PATCH 03/10] Make timeouts configurable. Fix race condition --- electrum/network.go | 21 +++++++++++++++------ 1 file changed, 15 insertions(+), 6 deletions(-) diff --git a/electrum/network.go b/electrum/network.go index 05cd3eb..a76b36c 100644 --- a/electrum/network.go +++ b/electrum/network.go @@ -20,15 +20,19 @@ const ( // ProtocolVersion identifies the support protocol version to the remote server ProtocolVersion = "1.4" - // TCP connection and server request timeout duration. - connTimeout = 30 * time.Second - nl = byte('\n') + nl = byte('\n') ) var ( // DebugMode provides debug output on communications with the remote server if enabled. DebugMode bool + // Timeout for connecting to the server + ConnTimeout = time.Second * 30 + + // Timeout for requests + ReqTimeout = time.Second * 30 + // ErrServerConnected throws an error if remote server is already connected. ErrServerConnected = errors.New("server is already connected") @@ -62,7 +66,7 @@ type TCPTransport struct { // NewTCPTransport opens a new TCP connection to the remote server. func NewTCPTransport(addr string) (*TCPTransport, error) { - conn, err := net.DialTimeout("tcp", addr, connTimeout) + conn, err := net.DialTimeout("tcp", addr, ConnTimeout) if err != nil { return nil, err } @@ -81,7 +85,7 @@ func NewTCPTransport(addr string) (*TCPTransport, error) { // NewSSLTransport opens a new SSL connection to the remote server. func NewSSLTransport(addr string, config *tls.Config) (*TCPTransport, error) { dialer := net.Dialer{ - Timeout: connTimeout, + Timeout: ConnTimeout, } conn, err := tls.DialWithDialer(&dialer, "tcp", addr, config) if err != nil { @@ -226,7 +230,12 @@ type response struct { func (s *Server) listen() { for { + if s.transport == nil { + break + } select { + case <-s.quit: + break case err := <-s.transport.Errors(): s.Error <- err s.Shutdown() @@ -319,7 +328,7 @@ func (s *Server) request(method string, params []interface{}, v interface{}) err var resp *container select { case resp = <-c: - case <-time.After(connTimeout): + case <-time.After(ReqTimeout): return ErrTimeout } From d5adb659d85826b46a0b5cfb46936767bb221ee9 Mon Sep 17 00:00:00 2001 From: Thomas Stevens Date: Mon, 30 Mar 2020 20:26:58 -0400 Subject: [PATCH 04/10] Improve request timeout config --- electrum/network.go | 32 +++++++++++++++++--------------- 1 file changed, 17 insertions(+), 15 deletions(-) diff --git a/electrum/network.go b/electrum/network.go index a76b36c..f0e99c4 100644 --- a/electrum/network.go +++ b/electrum/network.go @@ -27,12 +27,6 @@ var ( // DebugMode provides debug output on communications with the remote server if enabled. DebugMode bool - // Timeout for connecting to the server - ConnTimeout = time.Second * 30 - - // Timeout for requests - ReqTimeout = time.Second * 30 - // ErrServerConnected throws an error if remote server is already connected. ErrServerConnected = errors.New("server is already connected") @@ -65,8 +59,8 @@ type TCPTransport struct { } // NewTCPTransport opens a new TCP connection to the remote server. -func NewTCPTransport(addr string) (*TCPTransport, error) { - conn, err := net.DialTimeout("tcp", addr, ConnTimeout) +func NewTCPTransport(addr string, timeout time.Duration) (*TCPTransport, error) { + conn, err := net.DialTimeout("tcp", addr, timeout) if err != nil { return nil, err } @@ -83,9 +77,9 @@ func NewTCPTransport(addr string) (*TCPTransport, error) { } // NewSSLTransport opens a new SSL connection to the remote server. -func NewSSLTransport(addr string, config *tls.Config) (*TCPTransport, error) { +func NewSSLTransport(addr string, config *tls.Config, timeout time.Duration) (*TCPTransport, error) { dialer := net.Dialer{ - Timeout: ConnTimeout, + Timeout: timeout, } conn, err := tls.DialWithDialer(&dialer, "tcp", addr, config) if err != nil { @@ -150,9 +144,15 @@ type container struct { err error } +type ServerOptions struct { + ConnTimeout time.Duration + ReqTimeout time.Duration +} + // Server stores information about the remote server. type Server struct { transport Transport + opts ServerOptions handlers map[uint64]chan *container handlersLock sync.RWMutex @@ -167,25 +167,27 @@ type Server struct { } // NewServer initialize a new remote server. -func NewServer() *Server { +func NewServer(opts ServerOptions) *Server { s := &Server{ handlers: make(map[uint64]chan *container), pushHandlers: make(map[string][]chan *container), Error: make(chan error), quit: make(chan struct{}), + + opts: opts, } return s } // ConnectTCP connects to the remote server using TCP. -func (s *Server) ConnectTCP(addr string) error { +func (s *Server) ConnectTCP(addr string, ) error { if s.transport != nil { return ErrServerConnected } - transport, err := NewTCPTransport(addr) + transport, err := NewTCPTransport(addr, s.opts.ConnTimeout) if err != nil { return err } @@ -202,7 +204,7 @@ func (s *Server) ConnectSSL(addr string, config *tls.Config) error { return ErrServerConnected } - transport, err := NewSSLTransport(addr, config) + transport, err := NewSSLTransport(addr, config, s.opts.ConnTimeout) if err != nil { return err } @@ -328,7 +330,7 @@ func (s *Server) request(method string, params []interface{}, v interface{}) err var resp *container select { case resp = <-c: - case <-time.After(ReqTimeout): + case <-time.After(s.opts.ReqTimeout): return ErrTimeout } From 3cdc5126f813039887d7ead50039268e1a226f91 Mon Sep 17 00:00:00 2001 From: Thomas Stevens Date: Mon, 30 Mar 2020 20:29:54 -0400 Subject: [PATCH 05/10] Use a pointer for ServerOptions --- electrum/network.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/electrum/network.go b/electrum/network.go index f0e99c4..e4c3394 100644 --- a/electrum/network.go +++ b/electrum/network.go @@ -152,7 +152,7 @@ type ServerOptions struct { // Server stores information about the remote server. type Server struct { transport Transport - opts ServerOptions + opts *ServerOptions handlers map[uint64]chan *container handlersLock sync.RWMutex @@ -167,7 +167,7 @@ type Server struct { } // NewServer initialize a new remote server. -func NewServer(opts ServerOptions) *Server { +func NewServer(opts *ServerOptions) *Server { s := &Server{ handlers: make(map[uint64]chan *container), pushHandlers: make(map[string][]chan *container), From c53cf2aea4b1734c4bda1279c09a2c3613686d50 Mon Sep 17 00:00:00 2001 From: Thomas Stevens Date: Tue, 31 Mar 2020 09:41:31 -0400 Subject: [PATCH 06/10] Fix race condition with shutdown. Add IsShutdown() --- electrum/network.go | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-) diff --git a/electrum/network.go b/electrum/network.go index e4c3394..3ceeff5 100644 --- a/electrum/network.go +++ b/electrum/network.go @@ -354,8 +354,19 @@ func (s *Server) request(method string, params []interface{}, v interface{}) err func (s *Server) Shutdown() { close(s.quit) - _ = s.transport.Close() + if s.transport != nil { + _ = s.transport.Close() + } s.transport = nil s.handlers = nil s.pushHandlers = nil } + +func (s *Server) IsShutdown() bool { + select { + case <-s.quit: + return true + default: + } + return false +} \ No newline at end of file From 0bf5e6c53602f6063c8677877609c91b00e8cb67 Mon Sep 17 00:00:00 2001 From: Thomas Stevens Date: Tue, 31 Mar 2020 09:52:15 -0400 Subject: [PATCH 07/10] Transport errors should make connection shutdown --- electrum/network.go | 1 + 1 file changed, 1 insertion(+) diff --git a/electrum/network.go b/electrum/network.go index 3ceeff5..7e7fd0b 100644 --- a/electrum/network.go +++ b/electrum/network.go @@ -318,6 +318,7 @@ func (s *Server) request(method string, params []interface{}, v interface{}) err err = s.transport.SendMessage(bytes) if err != nil { + s.Shutdown() return err } From e42fd232156b7f65461c15bd97b944917ed951c2 Mon Sep 17 00:00:00 2001 From: Thomas Stevens Date: Tue, 31 Mar 2020 10:13:52 -0400 Subject: [PATCH 08/10] Make shutdown safer --- electrum/network.go | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/electrum/network.go b/electrum/network.go index 7e7fd0b..7000caf 100644 --- a/electrum/network.go +++ b/electrum/network.go @@ -354,7 +354,9 @@ func (s *Server) request(method string, params []interface{}, v interface{}) err } func (s *Server) Shutdown() { - close(s.quit) + if !s.IsShutdown() { + close(s.quit) + } if s.transport != nil { _ = s.transport.Close() } From ee72946254f23420370693d6ca4f286be5936e8b Mon Sep 17 00:00:00 2001 From: Thomas Stevens Date: Tue, 7 Apr 2020 09:06:47 -0400 Subject: [PATCH 09/10] Remove extraneous comma. Run gofmt --- electrum/network.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/electrum/network.go b/electrum/network.go index 7000caf..98bb512 100644 --- a/electrum/network.go +++ b/electrum/network.go @@ -182,7 +182,7 @@ func NewServer(opts *ServerOptions) *Server { } // ConnectTCP connects to the remote server using TCP. -func (s *Server) ConnectTCP(addr string, ) error { +func (s *Server) ConnectTCP(addr string) error { if s.transport != nil { return ErrServerConnected } @@ -372,4 +372,4 @@ func (s *Server) IsShutdown() bool { default: } return false -} \ No newline at end of file +} From d79d10eb1d192cc111d759622f7408c0f031ecfd Mon Sep 17 00:00:00 2001 From: Vladimir Li Date: Fri, 10 Jul 2020 09:51:44 -0700 Subject: [PATCH 10/10] make an extra shutdown check in the listen routine --- electrum/network.go | 3 +++ 1 file changed, 3 insertions(+) diff --git a/electrum/network.go b/electrum/network.go index 98bb512..49c4002 100644 --- a/electrum/network.go +++ b/electrum/network.go @@ -232,6 +232,9 @@ type response struct { func (s *Server) listen() { for { + if s.IsShutdown() { + break + } if s.transport == nil { break }