From 87132fe9a69c4d6c24c5072c1e2a1672114a56d3 Mon Sep 17 00:00:00 2001 From: Gisle Aune Date: Sun, 5 Aug 2018 11:27:11 +0200 Subject: [PATCH] Removed locking of global handlers, and updated documentation to highlight thread unsafety. This will help concurrency. The pattern of adding and removing handlers is discouraged and made impossible --- handle.go | 37 +++++++------------------------------ handle_test.go | 4 ---- 2 files changed, 7 insertions(+), 34 deletions(-) diff --git a/handle.go b/handle.go index a6b39f8..821d497 100644 --- a/handle.go +++ b/handle.go @@ -1,49 +1,26 @@ package irc -import ( - "sync" -) - // A Handler is a function that is part of the irc event loop. It will receive all -// events that haven't been killed up to that point. +// events. type Handler func(event *Event, client *Client) var eventHandler struct { - mutex sync.RWMutex handlers []Handler } func emit(event *Event, client *Client) { - eventHandler.mutex.RLock() for _, handler := range eventHandler.handlers { handler(event, client) } - eventHandler.mutex.RUnlock() } -// Handle adds a new handler to the irc handling. It returns a pointer that can be passed to RemoveHandler -// later on to unsubscribe. -func Handle(handler Handler) *Handler { - eventHandler.mutex.Lock() - defer eventHandler.mutex.Unlock() - +// Handle adds a new handler to the irc handling. The handler may be called from multiple threads at the same +// time, so external resources should be locked if there are multiple clients. Adding handlers is not thread +// safe and should be done prior to clients being created.Handle. Also, this handler will block the individual +// client's event loop, so long operations that include network requests and the like should be done in a +// goroutine with the needed data **copied** from the handler function. +func Handle(handler Handler) { eventHandler.handlers = append(eventHandler.handlers, handler) - return &eventHandler.handlers[len(eventHandler.handlers)-1] -} - -// RemoveHandler unregisters a handler. -func RemoveHandler(handlerPtr *Handler) (ok bool) { - eventHandler.mutex.Lock() - defer eventHandler.mutex.Unlock() - - for i := range eventHandler.handlers { - if &eventHandler.handlers[i] == handlerPtr { - eventHandler.handlers = append(eventHandler.handlers[:i], eventHandler.handlers[i+1:]...) - return true - } - } - - return false } func init() { diff --git a/handle_test.go b/handle_test.go index 057b65b..d67b533 100644 --- a/handle_test.go +++ b/handle_test.go @@ -31,10 +31,6 @@ func TestHandle(t *testing.T) { t.Error("Event wasn't handled") } - if !irc.RemoveHandler(handle) { - t.Error("Couldn't remove handler") - } - handled = false client.EmitSync(context.Background(), event)