From c603d41d080d7c50159bcacdae19a26b72e3326b Mon Sep 17 00:00:00 2001 From: Shivaram Lingamneni Date: Tue, 3 May 2022 11:06:57 -0400 Subject: [PATCH 1/2] genericize atomic config changes --- irc/getters.go | 7 +------ irc/server.go | 5 ++--- irc/utils/config.go | 32 ++++++++++++++++++++++++++++++++ 3 files changed, 35 insertions(+), 9 deletions(-) create mode 100644 irc/utils/config.go diff --git a/irc/getters.go b/irc/getters.go index 761ece18..485c2d7e 100644 --- a/irc/getters.go +++ b/irc/getters.go @@ -7,7 +7,6 @@ import ( "net" "sync/atomic" "time" - "unsafe" "github.com/ergochat/ergo/irc/caps" "github.com/ergochat/ergo/irc/languages" @@ -16,11 +15,7 @@ import ( ) func (server *Server) Config() (config *Config) { - return (*Config)(atomic.LoadPointer(&server.config)) -} - -func (server *Server) SetConfig(config *Config) { - atomic.StorePointer(&server.config, unsafe.Pointer(config)) + return server.config.Get() } func (server *Server) ChannelRegistrationEnabled() bool { diff --git a/irc/server.go b/irc/server.go index 955a9337..823ab832 100644 --- a/irc/server.go +++ b/irc/server.go @@ -17,7 +17,6 @@ import ( "sync" "syscall" "time" - "unsafe" "github.com/ergochat/irc-go/ircfmt" "github.com/okzk/sdnotify" @@ -66,7 +65,7 @@ type Server struct { channels ChannelManager channelRegistry ChannelRegistry clients ClientManager - config unsafe.Pointer + config utils.ConfigStore[Config] configFilename string connectionLimiter connection_limits.Limiter ctime time.Time @@ -704,7 +703,7 @@ func (server *Server) applyConfig(config *Config) (err error) { config.Server.Cloaks.SetSecret(LoadCloakSecret(server.store)) // activate the new config - server.SetConfig(config) + server.config.Set(config) // load [dk]-lines, registered users and channels, etc. if initial { diff --git a/irc/utils/config.go b/irc/utils/config.go new file mode 100644 index 00000000..7159373d --- /dev/null +++ b/irc/utils/config.go @@ -0,0 +1,32 @@ +// Copyright (c) 2022 Shivaram Lingamneni +// released under the MIT license + +package utils + +import ( + "sync/atomic" + "unsafe" +) + +/* +This can be used to implement the following pattern: + +1. Load and munge a config (this can be arbitrarily expensive) +2. Use Set() to install the config +3. Use Get() to access the config +4. As long as any individual config is not modified (by any goroutine) + after the initial call to Set(), this is free of data races, and Get() + is extremely cheap (on amd64 it compiles down to plain MOV instructions). +*/ + +type ConfigStore[T any] struct { + ptr unsafe.Pointer +} + +func (c *ConfigStore[T]) Get() *T { + return (*T)(atomic.LoadPointer(&c.ptr)) +} + +func (c *ConfigStore[T]) Set(ptr *T) { + atomic.StorePointer(&c.ptr, unsafe.Pointer(ptr)) +} From 34ad3a2dc1faec63cb365561d49c8a563f66890a Mon Sep 17 00:00:00 2001 From: Shivaram Lingamneni Date: Tue, 3 May 2022 23:27:24 -0400 Subject: [PATCH 2/2] ConfigStore: clarify intended use --- irc/utils/config.go | 19 ++++++++++--------- 1 file changed, 10 insertions(+), 9 deletions(-) diff --git a/irc/utils/config.go b/irc/utils/config.go index 7159373d..73fd7165 100644 --- a/irc/utils/config.go +++ b/irc/utils/config.go @@ -11,22 +11,23 @@ import ( /* This can be used to implement the following pattern: -1. Load and munge a config (this can be arbitrarily expensive) -2. Use Set() to install the config -3. Use Get() to access the config -4. As long as any individual config is not modified (by any goroutine) - after the initial call to Set(), this is free of data races, and Get() +1. Prepare a config object (this can be arbitrarily expensive) +2. Take a pointer to the config object and use Set() to install it +3. Use Get() to access the config from any goroutine +4. To update the config, call Set() again with a new prepared config object +5. As long as any individual config object is not modified (by any goroutine) + after it is installed with Set(), this is free of data races, and Get() is extremely cheap (on amd64 it compiles down to plain MOV instructions). */ -type ConfigStore[T any] struct { +type ConfigStore[Config any] struct { ptr unsafe.Pointer } -func (c *ConfigStore[T]) Get() *T { - return (*T)(atomic.LoadPointer(&c.ptr)) +func (c *ConfigStore[Config]) Get() *Config { + return (*Config)(atomic.LoadPointer(&c.ptr)) } -func (c *ConfigStore[T]) Set(ptr *T) { +func (c *ConfigStore[Config]) Set(ptr *Config) { atomic.StorePointer(&c.ptr, unsafe.Pointer(ptr)) }