Use a RWMutex for server config locking instead of routing config gets and sets through Server#handler.

ctlrpc.go

@@ -30,7 +30,7 @@ type ConfigValue struct {
 
 // Set a config value
 func (c *ControlRPC) SetConfig(in *ConfigValue, out *ConfigValue) os.Error {
-	servers[in.Id].SetConfig(in.Key, in.Value)
+	servers[in.Id].cfg.Set(in.Key, in.Value)
 	out.Id = in.Id
 	out.Key = in.Key
 	out.Value = in.Value
@@ -41,6 +41,6 @@ func (c *ControlRPC) SetConfig(in *ConfigValue, out *ConfigValue) os.Error {
 func (c *ControlRPC) GetConfig(in *ConfigValue, out *ConfigValue) os.Error {
 	out.Id = in.Id
 	out.Key = in.Key
-	out.Value = servers[in.Id].GetConfig(in.Key)
+	out.Value = servers[in.Id].cfg.StringValue(in.Key)
 	return nil
 }

freeze.go

@@ -8,6 +8,7 @@ import (
 	"compress/gzip"
 	"fmt"
 	"gob"
+	"grumble/serverconf"
 	"io"
 	"io/ioutil"
 	"os"
@@ -98,7 +99,7 @@ func (server *Server) FreezeToFile(filename string) (err os.Error) {
 // Freeze a server
 func (server *Server) Freeze() (fs frozenServer, err os.Error) {
 	fs.Id = int(server.Id)
-	fs.Config = server.cfg
+	fs.Config = server.cfg.GetAll()
 
 	channels := []frozenChannel{}
 	for _, c := range server.Channels {
@@ -225,7 +226,7 @@ func NewServerFromFrozen(filename string) (s *Server, err os.Error) {
 	}
 
 	if fs.Config != nil {
-		s.cfg = fs.Config
+		s.cfg = serverconf.New(fs.Config)
 	}
 
 	// Add all channels, but don't hook up parent/child relationships

pkg/serverconf/config.go

@@ -6,6 +6,7 @@ package serverconf
 
 import (
 	"strconv"
+	"sync"
 )
 
 var defaultCfg = map[string]string{
@@ -21,14 +22,52 @@ var defaultCfg = map[string]string{
 	"SendVersion":           "true",
 }
 
-type Config map[string]string
-
-func (cfg Config) Set(key string, value string) {
-	cfg[key] = value
+type Config struct {
+	cfgMap map[string]string
+	mutex  sync.RWMutex
 }
 
-func (cfg Config) StringValue(key string) (value string) {
-	value, exists := cfg[key]
+// Create a new Config using cfgMap as the intial internal config map.
+// If cfgMap is nil, ConfigWithMap will create a new config map.
+func New(cfgMap map[string]string) *Config {
+	if cfgMap == nil {
+		cfgMap = make(map[string]string)
+	}
+	return &Config{cfgMap: cfgMap}
+}
+
+// Get a copy of the Config's internal config map
+func (cfg *Config) GetAll() (all map[string]string) {
+	cfg.mutex.RLock()
+	defer cfg.mutex.RUnlock()
+
+	all = make(map[string]string)
+	for k,v := range cfg.cfgMap {
+		all[k] = v
+	}
+	return
+}
+
+// Set a new value for a config key
+func (cfg *Config) Set(key string, value string) {
+	cfg.mutex.Lock()
+	defer cfg.mutex.Unlock()
+	cfg.cfgMap[key] = value
+}
+
+// Reset the value of a config key
+func (cfg *Config) Reset(key string) {
+	cfg.mutex.Lock()
+	defer cfg.mutex.Unlock()
+	cfg.cfgMap[key] = "", false
+}
+
+// Get the value of a specific config key encoded as a string
+func (cfg *Config) StringValue(key string) (value string) {
+	cfg.mutex.RLock()
+	defer cfg.mutex.RUnlock()
+
+	value, exists := cfg.cfgMap[key]
 	if exists {
 		return value
 	}
@@ -41,24 +80,23 @@ func (cfg Config) StringValue(key string) (value string) {
 	return ""
 }
 
-func (cfg Config) IntValue(key string) (intval int) {
+// Get the value of a speific config key as an int
+func (cfg *Config) IntValue(key string) (intval int) {
 	str := cfg.StringValue(key)
 	intval, _ = strconv.Atoi(str)
 	return
 }
 
-func (cfg Config) Uint32Value(key string) (uint32val uint32) {
+// Get the value of a specific config key as a uint32
+func (cfg *Config) Uint32Value(key string) (uint32val uint32) {
 	str := cfg.StringValue(key)
 	uintval, _ := strconv.Atoui(str)
 	return uint32(uintval)
 }
 
-func (cfg Config) BoolValue(key string) (boolval bool) {
+// Get the value fo a sepcific config key as a bool
+func (cfg *Config) BoolValue(key string) (boolval bool) {
 	str := cfg.StringValue(key)
 	boolval, _ = strconv.Atob(str)
 	return
 }
-
-func (cfg Config) Reset(key string) {
-	cfg[key] = "", false
-}

pkg/serverconf/config_test.go

@@ -9,7 +9,7 @@ import (
 )
 
 func TestIntValue(t *testing.T) {
-	cfg := make(Config)
+	cfg := New(nil)
 	cfg.Set("Test", "13")
 	if cfg.IntValue("Test") != 13 {
 		t.Errorf("Expected 13")
@@ -17,7 +17,7 @@ func TestIntValue(t *testing.T) {
 }
 
 func TestFloatAsInt(t *testing.T) {
-	cfg := make(Config)
+	cfg := New(nil)
 	cfg.Set("Test", "13.4")
 	if cfg.IntValue("Test") != 0 {
 		t.Errorf("Expected 0")
@@ -25,14 +25,14 @@ func TestFloatAsInt(t *testing.T) {
 }
 
 func TestDefaultValue(t *testing.T) {
-	cfg := make(Config)
+	cfg := New(nil)
 	if cfg.IntValue("MaxBandwidth") != 72000 {
 		t.Errorf("Expected 72000")
 	}
 }
 
 func TestBoolValue(t *testing.T) {
-	cfg := make(Config)
+	cfg := New(nil)
 	cfg.Set("DoStuffOnStartup", "true")
 	if cfg.BoolValue("DoStuffOnStartup") != true {
 		t.Errorf("Expected true")

server.go

@@ -58,14 +58,13 @@ type Server struct {
 	udpsend        chan *Message
 	voicebroadcast chan *VoiceBroadcast
 	freezeRequest  chan *freezeRequest
-	configRequest  chan *configRequest
 
 	// Signals to the server that a client has been successfully
 	// authenticated.
 	clientAuthenticated chan *Client
 
 	// Server configuration
-	cfg serverconf.Config
+	cfg *serverconf.Config
 
 	// Clients
 	clients map[uint32]*Client
@@ -124,13 +123,6 @@ type freezeRequest struct {
 	readCloser io.ReadCloser
 }
 
-type configRequest struct {
-	done  chan bool
-	set   bool
-	key   string
-	value string
-}
-
 // Allocate a new Murmur instance
 func NewServer(id int64, addr string, port int) (s *Server, err os.Error) {
 	s = new(Server)
@@ -140,7 +132,7 @@ func NewServer(id int64, addr string, port int) (s *Server, err os.Error) {
 	s.port = port
 	s.running = false
 
-	s.cfg = make(map[string]string)
+	s.cfg = serverconf.New(nil)
 
 	s.sessions = make(map[uint32]bool)
 	s.clients = make(map[uint32]*Client)
@@ -155,7 +147,6 @@ func NewServer(id int64, addr string, port int) (s *Server, err os.Error) {
 	s.udpsend = make(chan *Message)
 	s.voicebroadcast = make(chan *VoiceBroadcast)
 	s.freezeRequest = make(chan *freezeRequest)
-	s.configRequest = make(chan *configRequest)
 	s.clientAuthenticated = make(chan *Client)
 
 	s.Channels = make(map[int]*Channel)
@@ -394,10 +385,6 @@ func (server *Server) handler() {
 			}
 			go server.handleFreezeRequest(req, &fs)
 
-		// Synchronzied config get/set
-		case req := <-server.configRequest:
-			server.handleConfigRequest(req)
-
 		// Server registration update
 		// Tick every hour + a minute offset based on the server id.
 		case <-time.Tick((3600 + ((server.Id * 60) % 600)) * 1e9):
@@ -433,15 +420,6 @@ func (server *Server) handleFreezeRequest(freq *freezeRequest, fs *frozenServer)
 	}
 }
 
-func (server *Server) handleConfigRequest(cfgReq *configRequest) {
-	if cfgReq.set {
-		server.cfg.Set(cfgReq.key, cfgReq.value)
-	} else {
-		cfgReq.value = server.cfg.StringValue(cfgReq.key)
-	}
-	cfgReq.done <- true
-}
-
 // Handle an Authenticate protobuf message.  This is handled in a separate
 // goroutine to allow for remote authenticators that are slow to respond.
 //
@@ -1075,22 +1053,6 @@ func (s *Server) FreezeServer() io.ReadCloser {
 	return fr.readCloser
 }
 
-// Set the value of a config key
-func (s *Server) SetConfig(key string, value string) {
-	cfgReq := &configRequest{make(chan bool), true, key, value}
-	s.configRequest <- cfgReq
-	<-cfgReq.done
-	return
-}
-
-// Get the value of a config key
-func (s *Server) GetConfig(key string) (value string) {
-	cfgReq := &configRequest{make(chan bool), false, key, ""}
-	s.configRequest <- cfgReq
-	<-cfgReq.done
-	return cfgReq.value
-}
-
 // Register a client on the server.
 func (s *Server) RegisterClient(client *Client) (uid uint32) {
 	// Increment nextUserId only if registration succeeded.