From 01b7a70a1f99c0ed55669b10083d9e0ce4f98577 Mon Sep 17 00:00:00 2001 From: Nikita Kryuchkov Date: Fri, 13 Dec 2019 22:54:19 +0300 Subject: [PATCH] Improve code quality --- pkg/app/client.go | 9 +++++++++ pkg/app/client_test.go | 10 ++++++++-- pkg/app/conn.go | 1 + pkg/app/listener.go | 3 +++ pkg/app/log_store.go | 2 ++ pkg/app/rpc_client_test.go | 1 + 6 files changed, 24 insertions(+), 2 deletions(-) diff --git a/pkg/app/client.go b/pkg/app/client.go index c4d2fe67ad..1b6e319a66 100644 --- a/pkg/app/client.go +++ b/pkg/app/client.go @@ -109,6 +109,7 @@ func (c *Client) Dial(remote appnet.Addr) (net.Conn, error) { } conn.freeConnMx.Lock() + free, err := c.cm.Add(connID, conn) if err != nil { @@ -120,7 +121,9 @@ func (c *Client) Dial(remote appnet.Addr) (net.Conn, error) { return nil, err } + conn.freeConn = free + conn.freeConnMx.Unlock() return conn, nil @@ -148,16 +151,20 @@ func (c *Client) Listen(n appnet.Type, port routing.Port) (net.Listener, error) } listener.freeLisMx.Lock() + freeLis, err := c.lm.Add(lisID, listener) if err != nil { listener.freeLisMx.Unlock() + if err := listener.Close(); err != nil { c.log.WithError(err).Error("error closing listener") } return nil, err } + listener.freeLis = freeLis + listener.freeLisMx.Unlock() return listener, nil @@ -167,6 +174,7 @@ func (c *Client) Listen(n appnet.Type, port routing.Port) (net.Listener, error) // listeners and connections. func (c *Client) Close() { var listeners []net.Listener + c.lm.DoRange(func(_ uint16, v interface{}) bool { lis, err := idmanager.AssertListener(v) if err != nil { @@ -179,6 +187,7 @@ func (c *Client) Close() { }) var conns []net.Conn + c.cm.DoRange(func(_ uint16, v interface{}) bool { conn, err := idmanager.AssertConn(v) if err != nil { diff --git a/pkg/app/client_test.go b/pkg/app/client_test.go index a4ad4fcb1f..497b3f8ac1 100644 --- a/pkg/app/client_test.go +++ b/pkg/app/client_test.go @@ -305,8 +305,10 @@ func TestClient_Close(t *testing.T) { l := logging.MustGetLogger("app2_client") visorPK, _ := cipher.GenerateKeyPair() - var closeNoErr error - closeErr := errors.New("close error") + var ( + closeNoErr error + closeErr = errors.New("close error") + ) rpc := &MockRPCClient{} @@ -321,11 +323,13 @@ func TestClient_Close(t *testing.T) { lis1 := &Listener{id: lisID1, rpc: rpc, cm: idmanager.New()} freeLis1, err := lm.Add(lisID1, lis1) require.NoError(t, err) + lis1.freeLis = freeLis1 lis2 := &Listener{id: lisID2, rpc: rpc, cm: idmanager.New()} freeLis2, err := lm.Add(lisID2, lis2) require.NoError(t, err) + lis2.freeLis = freeLis2 connID1 := uint16(1) @@ -339,11 +343,13 @@ func TestClient_Close(t *testing.T) { conn1 := &Conn{id: connID1, rpc: rpc} freeConn1, err := cm.Add(connID1, conn1) require.NoError(t, err) + conn1.freeConn = freeConn1 conn2 := &Conn{id: connID2, rpc: rpc} freeConn2, err := cm.Add(connID2, conn2) require.NoError(t, err) + conn2.freeConn = freeConn2 cl := prepClient(l, visorPK, rpc) diff --git a/pkg/app/conn.go b/pkg/app/conn.go index d2ba71c296..b7d8a15a86 100644 --- a/pkg/app/conn.go +++ b/pkg/app/conn.go @@ -39,6 +39,7 @@ func (c *Conn) Write(b []byte) (int, error) { func (c *Conn) Close() error { c.freeConnMx.RLock() defer c.freeConnMx.RUnlock() + if c.freeConn != nil { if freed := c.freeConn(); !freed { return errors.New("conn is already closed") diff --git a/pkg/app/listener.go b/pkg/app/listener.go index dc5734f75c..bdd3c4faf3 100644 --- a/pkg/app/listener.go +++ b/pkg/app/listener.go @@ -26,6 +26,7 @@ type Listener struct { // Accept accepts a connection from listener. func (l *Listener) Accept() (net.Conn, error) { l.log.Infoln("Calling app RPC Accept") + connID, remote, err := l.rpc.Accept(l.id) if err != nil { return nil, err @@ -47,6 +48,7 @@ func (l *Listener) Accept() (net.Conn, error) { // the conn without `freeConn` while the next few lines are running, // the panic may raise without this lock conn.freeConnMx.Lock() + free, err := l.cm.Add(connID, conn) if err != nil { conn.freeConnMx.Unlock() @@ -68,6 +70,7 @@ func (l *Listener) Accept() (net.Conn, error) { func (l *Listener) Close() error { l.freeLisMx.RLock() defer l.freeLisMx.RUnlock() + if l.freeLis != nil { if freed := l.freeLis(); !freed { return errors.New("listener is already closed") diff --git a/pkg/app/log_store.go b/pkg/app/log_store.go index e773fd3123..ff0a53287a 100644 --- a/pkg/app/log_store.go +++ b/pkg/app/log_store.go @@ -58,6 +58,7 @@ func newBoltDB(path, appName string) (_ LogStore, err error) { return nil }) + if err != nil && !strings.Contains(err.Error(), bbolt.ErrBucketExists.Error()) { return nil, err } @@ -125,6 +126,7 @@ func (l *boltDBappLogs) LogsSince(t time.Time) (logs []string, err error) { if err != nil { return nil, err } + defer func() { cErr := db.Close() err = cErr diff --git a/pkg/app/rpc_client_test.go b/pkg/app/rpc_client_test.go index 6b0cda1f9b..146bf87c9f 100644 --- a/pkg/app/rpc_client_test.go +++ b/pkg/app/rpc_client_test.go @@ -452,6 +452,7 @@ func prepNetworkerWithListener(t *testing.T, lis *appcommon.MockListener, local var noErr error appnet.ClearNetworkers() + n := &appnet.MockNetworker{} n.On("ListenContext", mock.Anything, local).Return(lis, noErr)