server: fix bgp state transition race

Make sure that all Go routines finishes before moving to another bgp
state.

Signed-off-by: FUJITA Tomonori <fujita.tomonori@lab.ntt.co.jp>

2 files changed, 34 insertions, 23 deletions

diff --git a/server/fsm.go b/server/fsm.go
index 6adfc800..ffba35f1 100644
--- a/server/fsm.go
+++ b/server/fsm.go
@@ -266,15 +266,15 @@ type FSMHandler struct {
 }
 
 func NewFSMHandler(fsm *FSM, incoming chan *fsmMsg, outgoing chan *bgp.BGPMessage) *FSMHandler {
-	f := &FSMHandler{
+	h := &FSMHandler{
 		fsm:              fsm,
 		errorCh:          make(chan bool, 2),
 		incoming:         incoming,
 		outgoing:         outgoing,
 		holdTimerResetCh: make(chan bool, 2),
 	}
-	f.t.Go(f.loop)
-	return f
+	fsm.t.Go(h.loop)
+	return h
 }
 
 func (h *FSMHandler) idle() bgp.FSMState {
@@ -830,23 +830,34 @@ func (h *FSMHandler) established() bgp.FSMState {
 
 func (h *FSMHandler) loop() error {
 	fsm := h.fsm
-	nextState := bgp.FSMState(0)
+	ch := make(chan bgp.FSMState)
 	oldState := fsm.state
-	switch fsm.state {
-	case bgp.BGP_FSM_IDLE:
-		nextState = h.idle()
-	// case bgp.BGP_FSM_CONNECT:
-	// 	nextState = h.connect()
-	case bgp.BGP_FSM_ACTIVE:
-		nextState = h.active()
-	case bgp.BGP_FSM_OPENSENT:
-		nextState = h.opensent()
-	case bgp.BGP_FSM_OPENCONFIRM:
-		nextState = h.openconfirm()
-	case bgp.BGP_FSM_ESTABLISHED:
-		nextState = h.established()
+
+	f := func() error {
+		nextState := bgp.FSMState(0)
+		switch fsm.state {
+		case bgp.BGP_FSM_IDLE:
+			nextState = h.idle()
+			// case bgp.BGP_FSM_CONNECT:
+			// 	nextState = h.connect()
+		case bgp.BGP_FSM_ACTIVE:
+			nextState = h.active()
+		case bgp.BGP_FSM_OPENSENT:
+			nextState = h.opensent()
+		case bgp.BGP_FSM_OPENCONFIRM:
+			nextState = h.openconfirm()
+		case bgp.BGP_FSM_ESTABLISHED:
+			nextState = h.established()
+		}
+
+		ch <- nextState
+		return nil
 	}
 
+	h.t.Go(f)
+
+	nextState := <-ch
+
 	if nextState == bgp.BGP_FSM_ESTABLISHED && oldState == bgp.BGP_FSM_OPENCONFIRM {
 		log.WithFields(log.Fields{
 			"Topic": "Peer",
@@ -862,6 +873,12 @@ func (h *FSMHandler) loop() error {
 		}).Info("Peer Down")
 	}
 
+	e := time.AfterFunc(time.Second*120, func() {
+		log.Fatal("failed to free the fsm.h.t for ", fsm.pConf.NeighborConfig.NeighborAddress, oldState, nextState)
+	})
+	h.t.Wait()
+	e.Stop()
+
 	// zero means that tomb.Dying()
 	if nextState >= bgp.BGP_FSM_IDLE {
 		e := &fsmMsg{
diff --git a/server/server.go b/server/server.go
index d45e7bff..65253a70 100644
--- a/server/server.go
+++ b/server/server.go
@@ -24,7 +24,6 @@ import (
 	"github.com/osrg/gobgp/policy"
 	"github.com/osrg/gobgp/table"
 	zebra "github.com/osrg/gozebra"
-	"gopkg.in/tomb.v2"
 	"net"
 	"os"
 	"strconv"
@@ -603,11 +602,6 @@ func (server *BgpServer) handleFSMMessage(peer *Peer, e *fsmMsg, incoming chan *
 	case FSM_MSG_STATE_CHANGE:
 		nextState := e.MsgData.(bgp.FSMState)
 		oldState := bgp.FSMState(peer.conf.NeighborState.SessionState)
-		go func(t *tomb.Tomb, addr string, oldState, newState bgp.FSMState) {
-			e := time.AfterFunc(time.Second*30, func() { log.Fatal("failed to free the fsm.h.t for ", addr, oldState, newState) })
-			t.Wait()
-			e.Stop()
-		}(&peer.fsm.h.t, peer.conf.NeighborConfig.NeighborAddress.String(), oldState, nextState)
 		peer.conf.NeighborState.SessionState = uint32(nextState)
 		peer.fsm.StateChange(nextState)
 		globalRib := server.localRibMap[GLOBAL_RIB_NAME]