From e39cceace6a64bfd503baa2910ce677d98019f8b Mon Sep 17 00:00:00 2001 From: "Stanislav N. aka pztrn" Date: Tue, 3 Oct 2017 02:03:43 +0500 Subject: [PATCH 1/2] Added eventsMutex and wrap all events calls with it. Due to some "golangish" code this library have possibility to run into data race when application is working with callbacks. This commit adds eventsMutex (which is a sync.Mutex), removed all "golangish" ifs-map reads, and wrap events map read with sync.Mutex to avoid data races. --- irc_callback.go | 36 +++++++++++++++++++++++++++++------- irc_struct.go | 1 + 2 files changed, 30 insertions(+), 7 deletions(-) diff --git a/irc_callback.go b/irc_callback.go index d389f73..0c98947 100644 --- a/irc_callback.go +++ b/irc_callback.go @@ -13,13 +13,17 @@ import ( func (irc *Connection) AddCallback(eventcode string, callback func(*Event)) int { eventcode = strings.ToUpper(eventcode) id := 0 - if _, ok := irc.events[eventcode]; !ok { + + irc.eventsMutex.Lock() + _, ok := irc.events[eventcode] + if !ok { irc.events[eventcode] = make(map[int]func(*Event)) id = 0 } else { id = len(irc.events[eventcode]) } irc.events[eventcode][id] = callback + irc.eventsMutex.Unlock() return id } @@ -28,15 +32,20 @@ func (irc *Connection) AddCallback(eventcode string, callback func(*Event)) int func (irc *Connection) RemoveCallback(eventcode string, i int) bool { eventcode = strings.ToUpper(eventcode) - if event, ok := irc.events[eventcode]; ok { + irc.eventsMutex.Lock() + event, ok := irc.events[eventcode] + if ok { if _, ok := event[i]; ok { delete(irc.events[eventcode], i) + irc.eventsMutex.Unlock() return true } irc.Log.Printf("Event found, but no callback found at id %d\n", i) + irc.eventsMutex.Unlock() return false } + irc.eventsMutex.Unlock() irc.Log.Println("Event not found") return false } @@ -46,10 +55,14 @@ func (irc *Connection) RemoveCallback(eventcode string, i int) bool { func (irc *Connection) ClearCallback(eventcode string) bool { eventcode = strings.ToUpper(eventcode) - if _, ok := irc.events[eventcode]; ok { + irc.eventsMutex.Lock() + _, ok := irc.events[eventcode] + if ok { irc.events[eventcode] = make(map[int]func(*Event)) + irc.eventsMutex.Unlock() return true } + irc.eventsMutex.Unlock() irc.Log.Println("Event not found") return false @@ -59,7 +72,10 @@ func (irc *Connection) ClearCallback(eventcode string) bool { func (irc *Connection) ReplaceCallback(eventcode string, i int, callback func(*Event)) { eventcode = strings.ToUpper(eventcode) - if event, ok := irc.events[eventcode]; ok { + irc.eventsMutex.Lock() + event, ok := irc.events[eventcode] + irc.eventsMutex.Unlock() + if ok { if _, ok := event[i]; ok { event[i] = callback return @@ -109,7 +125,10 @@ func (irc *Connection) RunCallbacks(event *Event) { event.Arguments[len(event.Arguments)-1] = msg } - if callbacks, ok := irc.events[event.Code]; ok { + irc.eventsMutex.Lock() + callbacks, ok := irc.events[event.Code] + irc.eventsMutex.Unlock() + if ok { if irc.VerboseCallbackHandler { irc.Log.Printf("%v (%v) >> %#v\n", event.Code, len(callbacks), event) } @@ -121,12 +140,15 @@ func (irc *Connection) RunCallbacks(event *Event) { irc.Log.Printf("%v (0) >> %#v\n", event.Code, event) } - if callbacks, ok := irc.events["*"]; ok { + irc.eventsMutex.Lock() + allcallbacks, ok := irc.events["*"] + irc.eventsMutex.Unlock() + if ok { if irc.VerboseCallbackHandler { irc.Log.Printf("%v (0) >> %#v\n", event.Code, event) } - for _, callback := range callbacks { + for _, callback := range allcallbacks { callback(event) } } diff --git a/irc_struct.go b/irc_struct.go index a188d9d..f161a18 100644 --- a/irc_struct.go +++ b/irc_struct.go @@ -39,6 +39,7 @@ type Connection struct { user string registered bool events map[string]map[int]func(*Event) + eventsMutex sync.Mutex QuitMessage string lastMessage time.Time From 5a0a9009952b97fb28dc2b1f37799936db0a9439 Mon Sep 17 00:00:00 2001 From: "Stanislav N. aka pztrn" Date: Tue, 3 Oct 2017 02:19:13 +0500 Subject: [PATCH 2/2] Pings are also wrapped with own sync.Mutex. --- irc.go | 6 ++++-- irc_struct.go | 1 + 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/irc.go b/irc.go index eb9174d..1b52f34 100644 --- a/irc.go +++ b/irc.go @@ -74,9 +74,9 @@ func (irc *Connection) readLoop() { irc.Log.Printf("<-- %s\n", strings.TrimSpace(msg)) } - irc.Lock() + irc.lastMessageMutex.Lock() irc.lastMessage = time.Now() - irc.Unlock() + irc.lastMessageMutex.Unlock() event, err := parseToEvent(msg) event.Connection = irc if err == nil { @@ -166,9 +166,11 @@ func (irc *Connection) pingLoop() { select { case <-ticker.C: //Ping if we haven't received anything from the server within the keep alive period + irc.lastMessageMutex.Lock() if time.Since(irc.lastMessage) >= irc.KeepAlive { irc.SendRawf("PING %d", time.Now().UnixNano()) } + irc.lastMessageMutex.Unlock() case <-ticker2.C: //Ping at the ping frequency irc.SendRawf("PING %d", time.Now().UnixNano()) diff --git a/irc_struct.go b/irc_struct.go index f161a18..22d08d8 100644 --- a/irc_struct.go +++ b/irc_struct.go @@ -43,6 +43,7 @@ type Connection struct { QuitMessage string lastMessage time.Time + lastMessageMutex sync.Mutex VerboseCallbackHandler bool Log *log.Logger