MM-22044: Fix panic on web_conn send hello (#13799)

* MM-22044: Fix panic on web_conn send hello

(*Hub).Start is the central place for sending all web connection
related traffic. However, there was this one call to (*WebConn).Hello
which tried to send a message to a webconn separately.

This was a rare case, but it did occur under stress conditions generated
from a load test.

When the websocket send SEND_QUEUE_SIZE would get filled up and we would
attempt to make a broadcast, the non-blocking send would close the Send
channel of the web connection. During that time, if a web connection
would try to perform a broadcast, it would try to send to a closed channel
and cause a panic.

The solution is to bring back the sending of hello into the same goroutine
inside (*Hub).Start so that all state is centralised and we avoid
sending to a closed channel by sending the hello message inside the registering
code itself.

* Adding non-blocking send

* Simplify things

* Remove test

* Bring sendHello back

* Improve code further

Co-authored-by: mattermod <mattermod@users.noreply.github.com>
Этот коммит содержится в:
Agniva De Sarker
2020-03-30 18:12:40 +05:30
коммит произвёл GitHub
родитель f29d8ad098
Коммит 7ce68e89d7
2 изменённых файлов: 5 добавлений и 6 удалений

Просмотреть файл

@@ -291,10 +291,10 @@ func (wc *WebConn) IsAuthenticated() bool {
return true
}
func (wc *WebConn) SendHello() {
func (wc *WebConn) createHelloMessage() *model.WebSocketEvent {
msg := model.NewWebSocketEvent(model.WEBSOCKET_EVENT_HELLO, "", "", wc.UserId, nil)
msg.Add("server_version", fmt.Sprintf("%v.%v.%v.%v", model.CurrentVersion, model.BuildNumber, wc.App.ClientConfigHash(), wc.App.License() != nil))
wc.Send <- msg
return msg
}
func (wc *WebConn) shouldSendEventToGuest(msg *model.WebSocketEvent) bool {

Просмотреть файл

@@ -339,10 +339,6 @@ func (h *Hub) Register(webConn *WebConn) {
case h.register <- webConn:
case <-h.didStop:
}
if webConn.IsAuthenticated() {
webConn.SendHello()
}
}
func (h *Hub) Unregister(webConn *WebConn) {
@@ -410,6 +406,9 @@ func (h *Hub) Start() {
case webCon := <-h.register:
connections.Add(webCon)
atomic.StoreInt64(&h.connectionCount, int64(len(connections.All())))
if webCon.IsAuthenticated() {
webCon.Send <- webCon.createHelloMessage()
}
case webCon := <-h.unregister:
connections.Remove(webCon)
atomic.StoreInt64(&h.connectionCount, int64(len(connections.All())))