Skip to content

Commit

Permalink
fix a goroutine leak issue caused by Login plugin timeout (fatedier#3547
Browse files Browse the repository at this point in the history
)
  • Loading branch information
fatedier authored Jul 25, 2023
1 parent 3235add commit 6430afc
Show file tree
Hide file tree
Showing 3 changed files with 30 additions and 12 deletions.
4 changes: 4 additions & 0 deletions Release.md
Original file line number Diff line number Diff line change
@@ -1,3 +1,7 @@
### Features

* Adds a `completion` command for shell completions.

### Fixes

* fix a goroutine leak issue caused by Login plugin timeout.
36 changes: 25 additions & 11 deletions server/control.go
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,6 @@ import (

"github.com/fatedier/frp/pkg/auth"
"github.com/fatedier/frp/pkg/config"
"github.com/fatedier/frp/pkg/consts"
pkgerr "github.com/fatedier/frp/pkg/errors"
"github.com/fatedier/frp/pkg/msg"
plugin "github.com/fatedier/frp/pkg/plugin/server"
Expand All @@ -55,13 +54,14 @@ func NewControlManager() *ControlManager {
}
}

func (cm *ControlManager) Add(runID string, ctl *Control) (oldCtl *Control) {
func (cm *ControlManager) Add(runID string, ctl *Control) (old *Control) {
cm.mu.Lock()
defer cm.mu.Unlock()

oldCtl, ok := cm.ctlsByRunID[runID]
var ok bool
old, ok = cm.ctlsByRunID[runID]
if ok {
oldCtl.Replaced(ctl)
old.Replaced(ctl)
}
cm.ctlsByRunID[runID] = ctl
return
Expand Down Expand Up @@ -141,14 +141,13 @@ type Control struct {
// replace old controller instantly.
runID string

// control status
status string

readerShutdown *shutdown.Shutdown
writerShutdown *shutdown.Shutdown
managerShutdown *shutdown.Shutdown
allShutdown *shutdown.Shutdown

started bool

mu sync.RWMutex

// Server configuration information
Expand Down Expand Up @@ -187,7 +186,6 @@ func NewControl(
portsUsedNum: 0,
lastPing: time.Now(),
runID: loginMsg.RunID,
status: consts.Working,
readerShutdown: shutdown.New(),
writerShutdown: shutdown.New(),
managerShutdown: shutdown.New(),
Expand All @@ -208,11 +206,19 @@ func (ctl *Control) Start() {
Error: "",
}
_ = msg.WriteMsg(ctl.conn, loginRespMsg)
ctl.mu.Lock()
ctl.started = true
ctl.mu.Unlock()

go ctl.writer()
for i := 0; i < ctl.poolCount; i++ {
ctl.sendCh <- &msg.ReqWorkConn{}
}
go func() {
for i := 0; i < ctl.poolCount; i++ {
// ignore error here, that means that this control is closed
_ = errors.PanicToError(func() {
ctl.sendCh <- &msg.ReqWorkConn{}
})
}
}()

go ctl.manager()
go ctl.reader()
Expand Down Expand Up @@ -418,6 +424,14 @@ func (ctl *Control) stoper() {

// block until Control closed
func (ctl *Control) WaitClosed() {
ctl.mu.RLock()
started := ctl.started
ctl.mu.RUnlock()

if !started {
ctl.allShutdown.Done()
return
}
ctl.allShutdown.WaitDone()
}

Expand Down
2 changes: 1 addition & 1 deletion server/service.go
Original file line number Diff line number Diff line change
Expand Up @@ -552,7 +552,7 @@ func (svr *Service) RegisterControl(ctlConn net.Conn, loginMsg *msg.Login) (err

ctl := NewControl(ctx, svr.rc, svr.pxyManager, svr.pluginManager, svr.authVerifier, ctlConn, loginMsg, svr.cfg)
if oldCtl := svr.ctlManager.Add(loginMsg.RunID, ctl); oldCtl != nil {
oldCtl.allShutdown.WaitDone()
oldCtl.WaitClosed()
}

ctl.Start()
Expand Down

0 comments on commit 6430afc

Please sign in to comment.