remove multiconn.LastUsage + fix peer gc

peer: gc active sub conn

Makefile

@@ -25,6 +25,7 @@ deps:
 	go mod download
 	go build -o deps storj.io/drpc/cmd/protoc-gen-go-drpc
 	go build -o deps github.com/gogo/protobuf/protoc-gen-gogofaster
+	go build -o deps github.com/golang/mock/mockgen
 
 test:
 	go test ./... --cover

net/peer/peer.go

@@ -35,6 +35,7 @@ func NewPeer(mc transport.MultiConn, ctrl connCtrl) (p Peer, err error) {
 		active:    map[*subConn]struct{}{},
 		MultiConn: mc,
 		ctrl:      ctrl,
+		created:   time.Now(),
 	}
 	if pr.id, err = CtxPeerId(ctx); err != nil {
 		return
@@ -74,6 +75,8 @@ type peer struct {
 
 	mu sync.Mutex
 
+	created time.Time
+
 	transport.MultiConn
 }
 
@@ -95,6 +98,12 @@ func (p *peer) AcquireDrpcConn(ctx context.Context) (drpc.Conn, error) {
 	idx := len(p.inactive) - 1
 	res := p.inactive[idx]
 	p.inactive = p.inactive[:idx]
+	select {
+	case <-res.Closed():
+		p.mu.Unlock()
+		return p.AcquireDrpcConn(ctx)
+	default:
+	}
 	p.active[res] = struct{}{}
 	p.mu.Unlock()
 	return res, nil
@@ -184,14 +193,14 @@ func (p *peer) serve(conn net.Conn) (err error) {
 }
 
 func (p *peer) TryClose(objectTTL time.Duration) (res bool, err error) {
-	p.gc(objectTTL)
-	if time.Now().Sub(p.LastUsage()) < objectTTL {
-		return false, nil
+	aliveCount := p.gc(objectTTL)
+	if aliveCount == 0 && p.created.Add(time.Minute).Before(time.Now()) {
+		return true, p.Close()
 	}
-	return true, p.Close()
+	return false, nil
 }
 
-func (p *peer) gc(ttl time.Duration) {
+func (p *peer) gc(ttl time.Duration) (aliveCount int) {
 	p.mu.Lock()
 	defer p.mu.Unlock()
 	minLastUsage := time.Now().Add(-ttl)
@@ -222,9 +231,17 @@ func (p *peer) gc(ttl time.Duration) {
 		select {
 		case <-act.Closed():
 			delete(p.active, act)
+			continue
 		default:
 		}
+		if act.LastUsage().Before(minLastUsage) {
+			log.Warn("close active connection because no activity", zap.String("peerId", p.id), zap.String("addr", p.Addr()))
+			_ = act.Close()
+			delete(p.active, act)
+			continue
+		}
 	}
+	return len(p.active) + len(p.inactive)
 }
 
 func (p *peer) Close() (err error) {

net/peer/peer_test.go

@@ -64,11 +64,9 @@ func TestPeerAccept(t *testing.T) {
 }
 
 func TestPeer_TryClose(t *testing.T) {
-	t.Run("ttl", func(t *testing.T) {
+	t.Run("not close in first minute", func(t *testing.T) {
 		fx := newFixture(t, "p1")
 		defer fx.finish()
-		lu := time.Now()
-		fx.mc.EXPECT().LastUsage().Return(lu)
 		res, err := fx.TryClose(time.Second)
 		require.NoError(t, err)
 		assert.False(t, res)
@@ -76,8 +74,7 @@ func TestPeer_TryClose(t *testing.T) {
 	t.Run("close", func(t *testing.T) {
 		fx := newFixture(t, "p1")
 		defer fx.finish()
-		lu := time.Now().Add(-time.Second * 2)
-		fx.mc.EXPECT().LastUsage().Return(lu)
+		fx.peer.created = fx.peer.created.Add(-time.Minute * 2)
 		res, err := fx.TryClose(time.Second)
 		require.NoError(t, err)
 		assert.True(t, res)
@@ -85,8 +82,7 @@ func TestPeer_TryClose(t *testing.T) {
 	t.Run("gc", func(t *testing.T) {
 		fx := newFixture(t, "p1")
 		defer fx.finish()
-		now := time.Now()
-		fx.mc.EXPECT().LastUsage().Return(now.Add(time.Millisecond * 100))
+		//now := time.Now()
 
 		// make one inactive
 		in, out := net.Pipe()
@@ -105,6 +101,7 @@ func TestPeer_TryClose(t *testing.T) {
 		}()
 		defer out2.Close()
 		fx.mc.EXPECT().Open(gomock.Any()).Return(in2, nil)
+		fx.mc.EXPECT().Addr().Return("")
 		dc2, err := fx.AcquireDrpcConn(ctx)
 		require.NoError(t, err)
 		_ = dc2.Close()
@@ -118,6 +115,18 @@ func TestPeer_TryClose(t *testing.T) {
 		fx.mc.EXPECT().Open(gomock.Any()).Return(in3, nil)
 		dc3, err := fx.AcquireDrpcConn(ctx)
 		require.NoError(t, err)
+
+		// make another active, should be removed by ttl
+		in4, out4 := net.Pipe()
+		go func() {
+			handshake.IncomingProtoHandshake(ctx, out4, defaultProtoChecker)
+		}()
+		defer out4.Close()
+		fx.mc.EXPECT().Open(gomock.Any()).Return(in4, nil)
+		dc4, err := fx.AcquireDrpcConn(ctx)
+		require.NoError(t, err)
+		defer dc4.Close()
+
 		fx.ReleaseDrpcConn(dc3)
 		_ = dc3.Close()
 		fx.ReleaseDrpcConn(dc)

net/secureservice/secureservice.go

@@ -53,7 +53,9 @@ type secureService struct {
 }
 
 func (s *secureService) Init(a *app.App) (err error) {
-	s.protoVersion = ProtoVersion
+	if s.protoVersion == 0 {
+		s.protoVersion = ProtoVersion
+	}
 	account := a.MustComponent(commonaccount.CName).(commonaccount.Service)
 	peerKey, err := account.Account().PeerKey.Raw()
 	if err != nil {

net/secureservice/secureservice_test.go

@@ -57,7 +57,7 @@ func TestHandshake(t *testing.T) {
 
 func TestHandshakeIncompatibleVersion(t *testing.T) {
 	nc := testnodeconf.GenNodeConfig(2)
-	fxS := newFixture(t, nc, nc.GetAccountService(0), 0)
+	fxS := newFixture(t, nc, nc.GetAccountService(0), 1)
 	defer fxS.Finish(t)
 	sc, cc := net.Pipe()
 
@@ -72,7 +72,7 @@ func TestHandshakeIncompatibleVersion(t *testing.T) {
 		ar.ctx, ar.err = fxS.SecureInbound(ctx, sc)
 		resCh <- ar
 	}()
-	fxC := newFixture(t, nc, nc.GetAccountService(1), 1)
+	fxC := newFixture(t, nc, nc.GetAccountService(1), 2)
 	defer fxC.Finish(t)
 	_, err := fxC.SecureOutbound(ctx, cc)
 	require.Equal(t, handshake.ErrIncompatibleVersion, err)

net/transport/mock_transport/mock_transport.go

@@ -8,7 +8,6 @@ import (
 	context "context"
 	net "net"
 	reflect "reflect"
-	time "time"
 
 	transport "github.com/anyproto/any-sync/net/transport"
 	gomock "github.com/golang/mock/gomock"
@@ -158,20 +157,6 @@ func (mr *MockMultiConnMockRecorder) IsClosed() *gomock.Call {
 	return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "IsClosed", reflect.TypeOf((*MockMultiConn)(nil).IsClosed))
 }
 
-// LastUsage mocks base method.
-func (m *MockMultiConn) LastUsage() time.Time {
-	m.ctrl.T.Helper()
-	ret := m.ctrl.Call(m, "LastUsage")
-	ret0, _ := ret[0].(time.Time)
-	return ret0
-}
-
-// LastUsage indicates an expected call of LastUsage.
-func (mr *MockMultiConnMockRecorder) LastUsage() *gomock.Call {
-	mr.mock.ctrl.T.Helper()
-	return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "LastUsage", reflect.TypeOf((*MockMultiConn)(nil).LastUsage))
-}
-
 // Open mocks base method.
 func (m *MockMultiConn) Open(arg0 context.Context) (net.Conn, error) {
 	m.ctrl.T.Helper()

net/transport/transport.go

@@ -5,7 +5,6 @@ import (
 	"context"
 	"errors"
 	"net"
-	"time"
 )
 
 var (
@@ -29,8 +28,6 @@ type MultiConn interface {
 	Accept() (conn net.Conn, err error)
 	// Open opens new sub connection
 	Open(ctx context.Context) (conn net.Conn, err error)
-	// LastUsage returns the time of the last connection activity
-	LastUsage() time.Time
 	// Addr returns remote peer address
 	Addr() string
 	// IsClosed returns true when connection is closed

net/transport/yamux/config.go

@@ -5,7 +5,8 @@ type configGetter interface {
 }
 
 type Config struct {
-	ListenAddrs     []string `yaml:"listenAddrs"`
-	WriteTimeoutSec int      `yaml:"writeTimeoutSec"`
-	DialTimeoutSec  int      `yaml:"dialTimeoutSec"`
+	ListenAddrs        []string `yaml:"listenAddrs"`
+	WriteTimeoutSec    int      `yaml:"writeTimeoutSec"`
+	DialTimeoutSec     int      `yaml:"dialTimeoutSec"`
+	KeepAlivePeriodSec int      `yaml:"keepAlivePeriodSec"`
 }

net/transport/yamux/yamux.go

@@ -51,8 +51,16 @@ func (y *yamuxTransport) Init(a *app.App) (err error) {
 	if y.conf.WriteTimeoutSec <= 0 {
 		y.conf.WriteTimeoutSec = 10
 	}
+
 	y.yamuxConf = yamux.DefaultConfig()
-	y.yamuxConf.EnableKeepAlive = false
+	if y.conf.KeepAlivePeriodSec < 0 {
+		y.yamuxConf.EnableKeepAlive = false
+	} else {
+		y.yamuxConf.EnableKeepAlive = true
+		if y.conf.KeepAlivePeriodSec != 0 {
+			y.yamuxConf.KeepAliveInterval = time.Duration(y.conf.KeepAlivePeriodSec) * time.Second
+		}
+	}
 	y.yamuxConf.StreamOpenTimeout = time.Duration(y.conf.DialTimeoutSec) * time.Second
 	y.yamuxConf.ConnectionWriteTimeout = time.Duration(y.conf.WriteTimeoutSec) * time.Second
 	y.listCtx, y.listCtxCancel = context.WithCancel(context.Background())
@@ -106,7 +114,7 @@ func (y *yamuxTransport) Dial(ctx context.Context, addr string) (mc transport.Mu
 		_ = conn.Close()
 		return nil, err
 	}
-	luc := connutil.NewLastUsageConn(conn)
+	luc := connutil.NewLastUsageConn(connutil.NewTimeout(conn, time.Duration(y.conf.WriteTimeoutSec)*time.Second))
 	sess, err := yamux.Client(luc, y.yamuxConf)
 	if err != nil {
 		return
@@ -152,7 +160,7 @@ func (y *yamuxTransport) accept(conn net.Conn) {
 		log.Warn("incoming connection handshake error", zap.Error(err))
 		return
 	}
-	luc := connutil.NewLastUsageConn(conn)
+	luc := connutil.NewLastUsageConn(connutil.NewTimeout(conn, time.Duration(y.conf.WriteTimeoutSec)*time.Second))
 	sess, err := yamux.Server(luc, y.yamuxConf)
 	if err != nil {
 		log.Warn("incoming connection yamux session error", zap.Error(err))