diff options
| author | FUJITA Tomonori <fujita.tomonori@lab.ntt.co.jp> | 2015-01-18 23:16:40 +0900 |
|---|---|---|
| committer | FUJITA Tomonori <fujita.tomonori@lab.ntt.co.jp> | 2015-01-18 23:16:40 +0900 |
| commit | 97c124246d0b2e714d57eca814f95c0a82d97113 (patch) | |
| tree | 3f877e9b9093df59c7a2e2eb6a2e8f04e115f976 | |
| parent | ec278710f10c2cae7d6b3e393aa957edfd536181 (diff) | |
server: fix 'deleted config' race
When a config is deleted, goroutine for FSMHandler.h.Dying becomes
active. sendMessageloop must send notifcation before dying. So it
should not check Dying(). After sending notificaiton, it closes the
conneciton and return. It makes sure that rx goroutine will die too.
Signed-off-by: FUJITA Tomonori <fujita.tomonori@lab.ntt.co.jp>
| -rw-r--r-- | server/fsm.go | 7 | ||||
| -rw-r--r-- | server/peer.go | 5 |
2 files changed, 8 insertions, 4 deletions
diff --git a/server/fsm.go b/server/fsm.go index 5c93fce6..b70f0860 100644 --- a/server/fsm.go +++ b/server/fsm.go @@ -388,9 +388,10 @@ func (h *FSMHandler) sendMessageloop() error { conn := h.conn fsm := h.fsm for { + // this function doesn't check Dying() because we + // can't die before sending notificaiton. After + // sending notification, we'll die. select { - case <-h.t.Dying(): - return nil case m := <-h.outgoing: b, _ := m.Serialize() _, err := conn.Write(b) @@ -407,6 +408,7 @@ func (h *FSMHandler) sendMessageloop() error { if m.Header.Type == bgp.BGP_MSG_NOTIFICATION { h.errorCh <- true + conn.Close() return nil } case <-fsm.keepaliveTicker.C: @@ -445,7 +447,6 @@ func (h *FSMHandler) established() bgp.FSMState { h.t.Kill(nil) return bgp.BGP_FSM_IDLE case <-h.t.Dying(): - h.conn.Close() return 0 } } diff --git a/server/peer.go b/server/peer.go index c86aabb5..3b03046b 100644 --- a/server/peer.go +++ b/server/peer.go @@ -267,7 +267,10 @@ func (peer *Peer) loop() error { case <-peer.t.Dying(): close(peer.acceptedConnCh) peer.outgoing <- bgp.NewBGPNotificationMessage(bgp.BGP_ERROR_CEASE, bgp.BGP_ERROR_SUB_PEER_DECONFIGURED, nil) - h.Wait() + // h.t.Kill(nil) will be called + // internall so even goroutines in + // non-established will be killed. + h.Stop() return nil case e := <-incoming: switch e.MsgType { |