refactor [dk]linemanagers

* Synchronize map accesses
* Be more aggressive about cleaning up expired lines
* CIDR dlines did not work (nil dereference)

irc/dline.go

@@ -8,6 +8,7 @@ import (
 	"fmt"
 	"net"
 	"sort"
+	"sync"
 	"time"
 
 	"strings"
@@ -79,6 +80,7 @@ type dLineNet struct {
 
 // DLineManager manages and dlines.
 type DLineManager struct {
+	sync.RWMutex
 	// addresses that are dlined
 	addresses map[string]*dLineAddr
 	// networks that are dlined
@@ -97,6 +99,9 @@ func NewDLineManager() *DLineManager {
 func (dm *DLineManager) AllBans() map[string]IPBanInfo {
 	allb := make(map[string]IPBanInfo)
 
+	dm.RLock()
+	defer dm.RUnlock()
+
 	for name, info := range dm.addresses {
 		allb[name] = info.Info
 	}
@@ -118,13 +123,17 @@ func (dm *DLineManager) AddNetwork(network net.IPNet, length *IPRestrictTime, re
 			OperReason: operReason,
 		},
 	}
+	dm.Lock()
 	dm.networks[netString] = &dln
+	dm.Unlock()
 }
 
 // RemoveNetwork removes a network from the blocked list.
 func (dm *DLineManager) RemoveNetwork(network net.IPNet) {
 	netString := network.String()
+	dm.Lock()
 	delete(dm.networks, netString)
+	dm.Unlock()
 }
 
 // AddIP adds an IP address to the blocked list.
@@ -138,21 +147,27 @@ func (dm *DLineManager) AddIP(addr net.IP, length *IPRestrictTime, reason string
 			OperReason: operReason,
 		},
 	}
+	dm.Lock()
 	dm.addresses[addrString] = &dla
+	dm.Unlock()
 }
 
 // RemoveIP removes an IP from the blocked list.
 func (dm *DLineManager) RemoveIP(addr net.IP) {
 	addrString := addr.String()
+	dm.Lock()
 	delete(dm.addresses, addrString)
+	dm.Unlock()
 }
 
 // CheckIP returns whether or not an IP address was banned, and how long it is banned for.
 func (dm *DLineManager) CheckIP(addr net.IP) (isBanned bool, info *IPBanInfo) {
 	// check IP addr
 	addrString := addr.String()
-
+	dm.RLock()
 	addrInfo := dm.addresses[addrString]
+	dm.RUnlock()
+
 	if addrInfo != nil {
 		if addrInfo.Info.Time != nil {
 			if addrInfo.Info.Time.IsExpired() {
@@ -167,30 +182,32 @@ func (dm *DLineManager) CheckIP(addr net.IP) (isBanned bool, info *IPBanInfo) {
 	}
 
 	// check networks
-	var netsToRemove []net.IPNet
-
-	for _, netInfo := range dm.networks {
-		if !netInfo.Network.Contains(addr) {
-			continue
+	doCleanup := false
+	defer func() {
+		if doCleanup {
+			go func() {
+				dm.Lock()
+				defer dm.Unlock()
+				for key, netInfo := range dm.networks {
+					if netInfo.Info.Time.IsExpired() {
+						delete(dm.networks, key)
+					}
+				}
+			}()
 		}
+	}()
 
-		if netInfo.Info.Time != nil {
-			if netInfo.Info.Time.IsExpired() {
-				// ban on network has expired, remove it from our blocked list
-				netsToRemove = append(netsToRemove, netInfo.Network)
-			} else {
-				return true, &addrInfo.Info
-			}
-		} else {
-			return true, &addrInfo.Info
-		}
-	}
+	dm.RLock()
+	defer dm.RUnlock()
 
-	// remove expired networks
-	for _, expiredNet := range netsToRemove {
-		dm.RemoveNetwork(expiredNet)
+	for _, netInfo := range dm.networks {
+		if netInfo.Info.Time != nil && netInfo.Info.Time.IsExpired() {
+			// expired ban, ignore and clean up later
+			doCleanup = true
+		} else if netInfo.Network.Contains(addr) {
+			return true, &netInfo.Info
+		}
 	}
-
 	// no matches!
 	return false, nil
 }

irc/kline.go

@@ -8,6 +8,7 @@ import (
 	"fmt"
 	"sort"
 	"strings"
+	"sync"
 	"time"
 
 	"github.com/goshuirc/irc-go/ircfmt"
@@ -34,6 +35,7 @@ type KLineInfo struct {
 
 // KLineManager manages and klines.
 type KLineManager struct {
+	sync.RWMutex
 	// kline'd entries
 	entries map[string]*KLineInfo
 }
@@ -49,6 +51,8 @@ func NewKLineManager() *KLineManager {
 func (km *KLineManager) AllBans() map[string]IPBanInfo {
 	allb := make(map[string]IPBanInfo)
 
+	km.RLock()
+	defer km.RUnlock()
 	for name, info := range km.entries {
 		allb[name] = info.Info
 	}
@@ -67,48 +71,58 @@ func (km *KLineManager) AddMask(mask string, length *IPRestrictTime, reason stri
 			OperReason: operReason,
 		},
 	}
+	km.Lock()
 	km.entries[mask] = &kln
+	km.Unlock()
 }
 
 // RemoveMask removes a mask from the blocked list.
 func (km *KLineManager) RemoveMask(mask string) {
+	km.Lock()
 	delete(km.entries, mask)
+	km.Unlock()
 }
 
 // CheckMasks returns whether or not the hostmask(s) are banned, and how long they are banned for.
 func (km *KLineManager) CheckMasks(masks ...string) (isBanned bool, info *IPBanInfo) {
-	// check networks
-	var masksToRemove []string
+	doCleanup := false
+	defer func() {
+		// asynchronously remove expired bans
+		if doCleanup {
+			go func() {
+				km.Lock()
+				defer km.Unlock()
+				for key, entry := range km.entries {
+					if entry.Info.Time.IsExpired() {
+						delete(km.entries, key)
+					}
+				}
+			}()
+		}
+	}()
+
+
+	km.RLock()
+	defer km.RUnlock()
 
 	for _, entryInfo := range km.entries {
-		var matches bool
+		if entryInfo.Info.Time != nil && entryInfo.Info.Time.IsExpired() {
+			doCleanup = true
+			continue
+		}
+
+		matches := false
 		for _, mask := range masks {
 			if entryInfo.Matcher.Match(mask) {
 				matches = true
 				break
 			}
 		}
-		if !matches {
-			continue
-		}
-
-		if entryInfo.Info.Time != nil {
-			if entryInfo.Info.Time.IsExpired() {
-				// ban on network has expired, remove it from our blocked list
-				masksToRemove = append(masksToRemove, entryInfo.Mask)
-			} else {
-				return true, &entryInfo.Info
-			}
-		} else {
+		if matches {
 			return true, &entryInfo.Info
 		}
 	}
 
-	// remove expired networks
-	for _, expiredMask := range masksToRemove {
-		km.RemoveMask(expiredMask)
-	}
-
 	// no matches!
 	return false, nil
 }