Copy configuration struct when sending Changed() events

Avoids data race. Copy() must be called with lock held.
This commit is contained in:
Jakob Borg 2015-04-05 17:36:52 +02:00
parent a5edb6807e
commit bf4eb4b269
4 changed files with 147 additions and 11 deletions

View File

@ -44,6 +44,30 @@ type Configuration struct {
OriginalVersion int `xml:"-" json:"-"` // The version we read from disk, before any conversion
}
func (orig Configuration) Copy() Configuration {
c := orig
// Deep copy FolderConfigurations
c.Folders = make([]FolderConfiguration, len(orig.Folders))
for i := range c.Folders {
c.Folders[i] = orig.Folders[i].Copy()
}
// Deep copy DeviceConfigurations
c.Devices = make([]DeviceConfiguration, len(orig.Devices))
for i := range c.Devices {
c.Devices[i] = orig.Devices[i].Copy()
}
c.Options = orig.Options.Copy()
// DeviceIDs are values
c.IgnoredDevices = make([]protocol.DeviceID, len(orig.IgnoredDevices))
copy(c.IgnoredDevices, orig.IgnoredDevices)
return c
}
type FolderConfiguration struct {
ID string `xml:"id,attr" json:"id"`
Path string `xml:"path,attr" json:"path"`
@ -63,6 +87,13 @@ type FolderConfiguration struct {
deviceIDs []protocol.DeviceID
}
func (orig FolderConfiguration) Copy() FolderConfiguration {
c := orig
c.Devices = make([]FolderDeviceConfiguration, len(orig.Devices))
copy(c.Devices, orig.Devices)
return c
}
func (f *FolderConfiguration) CreateMarker() error {
if !f.HasMarker() {
marker := filepath.Join(f.Path, ".stfolder")
@ -144,11 +175,15 @@ type DeviceConfiguration struct {
Introducer bool `xml:"introducer,attr" json:"introducer"`
}
func (orig DeviceConfiguration) Copy() DeviceConfiguration {
c := orig
c.Addresses = make([]string, len(orig.Addresses))
copy(c.Addresses, orig.Addresses)
return c
}
type FolderDeviceConfiguration struct {
DeviceID protocol.DeviceID `xml:"id,attr" json:"deviceID"`
Deprecated_Name string `xml:"name,attr,omitempty" json:"-"`
Deprecated_Addresses []string `xml:"address,omitempty" json:"-"`
}
type OptionsConfiguration struct {
@ -176,6 +211,15 @@ type OptionsConfiguration struct {
LimitBandwidthInLan bool `xml:"limitBandwidthInLan" json:"limitBandwidthInLan" default:"false"`
}
func (orig OptionsConfiguration) Copy() OptionsConfiguration {
c := orig
c.ListenAddress = make([]string, len(orig.ListenAddress))
copy(c.ListenAddress, orig.ListenAddress)
c.GlobalAnnServers = make([]string, len(orig.GlobalAnnServers))
copy(c.GlobalAnnServers, orig.GlobalAnnServers)
return c
}
type GUIConfiguration struct {
Enabled bool `xml:"enabled,attr" json:"enabled" default:"true"`
Address string `xml:"address" json:"address" default:"127.0.0.1:8384"`

View File

@ -7,6 +7,8 @@
package config
import (
"bytes"
"encoding/json"
"fmt"
"os"
"reflect"
@ -438,3 +440,42 @@ func TestRequiresRestart(t *testing.T) {
t.Error("Changing GUI options requires restart")
}
}
func TestCopy(t *testing.T) {
wrapper, err := Load("testdata/example.xml", device1)
if err != nil {
t.Fatal(err)
}
cfg := wrapper.Raw()
bsOrig, err := json.MarshalIndent(cfg, "", " ")
if err != nil {
t.Fatal(err)
}
copy := cfg.Copy()
cfg.Devices[0].Addresses[0] = "wrong"
cfg.Folders[0].Devices[0].DeviceID = protocol.DeviceID{0, 1, 2, 3}
cfg.Options.ListenAddress[0] = "wrong"
cfg.GUI.APIKey = "wrong"
bsChanged, err := json.MarshalIndent(cfg, "", " ")
if err != nil {
t.Fatal(err)
}
bsCopy, err := json.MarshalIndent(copy, "", " ")
if err != nil {
t.Fatal(err)
}
if bytes.Compare(bsOrig, bsChanged) == 0 {
t.Error("Config should have changed")
}
if bytes.Compare(bsOrig, bsCopy) != 0 {
//ioutil.WriteFile("a", bsOrig, 0644)
//ioutil.WriteFile("b", bsCopy, 0644)
t.Error("Copy should be unchanged")
}
}

50
internal/config/testdata/example.xml vendored Normal file
View File

@ -0,0 +1,50 @@
<configuration version="10">
<folder id="default" path="~/Sync" ro="false" rescanIntervalS="60" ignorePerms="false">
<device id="GYRZZQB-IRNPV4Z-T7TC52W-EQYJ3TT-FDQW6MW-DFLMU42-SSSU6EM-FBK2VAY"></device>
<device id="P56IOI7-MZJNU2Y-IQGDREY-DM2MGTI-MGL3BXN-PQ6W5BM-TBBZ4TJ-XZWICQ2"></device>
<device id="ZJCXQL7-M3NP4IC-4KQ7WFU-3NANYUX-AD74QRL-Q5LJ7BH-72KYZHK-GHTAOAK"></device>
<versioning></versioning>
<lenientMtimes>false</lenientMtimes>
<copiers>1</copiers>
<pullers>16</pullers>
<hashers>0</hashers>
</folder>
<device id="GYRZZQB-IRNPV4Z-T7TC52W-EQYJ3TT-FDQW6MW-DFLMU42-SSSU6EM-FBK2VAY" name="win7" compression="metadata" introducer="false">
<address>dynamic</address>
</device>
<device id="P56IOI7-MZJNU2Y-IQGDREY-DM2MGTI-MGL3BXN-PQ6W5BM-TBBZ4TJ-XZWICQ2" name="jborg-mbp" compression="metadata" introducer="false">
<address>dynamic</address>
</device>
<device id="ZJCXQL7-M3NP4IC-4KQ7WFU-3NANYUX-AD74QRL-Q5LJ7BH-72KYZHK-GHTAOAK" name="anto-syncer" compression="never" introducer="false">
<address>dynamic</address>
</device>
<gui enabled="true" tls="false">
<address>0.0.0.0:8080</address>
<apikey>136020D511BF136020D511BF136020D511BF</apikey>
</gui>
<options>
<listenAddress>0.0.0.0:22000</listenAddress>
<globalAnnounceServer>udp4://announce.syncthing.net:22026</globalAnnounceServer>
<globalAnnounceServer>udp6://announce-v6.syncthing.net:22026</globalAnnounceServer>
<globalAnnounceEnabled>true</globalAnnounceEnabled>
<localAnnounceEnabled>true</localAnnounceEnabled>
<localAnnouncePort>21025</localAnnouncePort>
<localAnnounceMCAddr>[ff32::5222]:21026</localAnnounceMCAddr>
<maxSendKbps>0</maxSendKbps>
<maxRecvKbps>0</maxRecvKbps>
<reconnectionIntervalS>60</reconnectionIntervalS>
<startBrowser>false</startBrowser>
<upnpEnabled>true</upnpEnabled>
<upnpLeaseMinutes>0</upnpLeaseMinutes>
<upnpRenewalMinutes>30</upnpRenewalMinutes>
<urAccepted>-1</urAccepted>
<urUniqueID></urUniqueID>
<restartOnWakeup>true</restartOnWakeup>
<autoUpgradeIntervalH>0</autoUpgradeIntervalH>
<keepTemporariesH>24</keepTemporariesH>
<cacheIgnoredFiles>true</cacheIgnoredFiles>
<progressUpdateIntervalS>5</progressUpdateIntervalS>
<symlinksEnabled>true</symlinksEnabled>
<limitBandwidthInLan>false</limitBandwidthInLan>
</options>
</configuration>

View File

@ -80,6 +80,7 @@ func (w *Wrapper) Serve() {
w.sMut.Lock()
subs := w.subs
w.sMut.Unlock()
for _, h := range subs {
h.Changed(cfg)
}
@ -113,7 +114,7 @@ func (w *Wrapper) Replace(cfg Configuration) {
w.cfg = cfg
w.deviceMap = nil
w.folderMap = nil
w.replaces <- cfg
w.replaces <- cfg.Copy()
}
// Devices returns a map of devices. Device structures should not be changed,
@ -141,13 +142,13 @@ func (w *Wrapper) SetDevice(dev DeviceConfiguration) {
for i := range w.cfg.Devices {
if w.cfg.Devices[i].DeviceID == dev.DeviceID {
w.cfg.Devices[i] = dev
w.replaces <- w.cfg
w.replaces <- w.cfg.Copy()
return
}
}
w.cfg.Devices = append(w.cfg.Devices, dev)
w.replaces <- w.cfg
w.replaces <- w.cfg.Copy()
}
// Devices returns a map of folders. Folder structures should not be changed,
@ -181,13 +182,13 @@ func (w *Wrapper) SetFolder(fld FolderConfiguration) {
for i := range w.cfg.Folders {
if w.cfg.Folders[i].ID == fld.ID {
w.cfg.Folders[i] = fld
w.replaces <- w.cfg
w.replaces <- w.cfg.Copy()
return
}
}
w.cfg.Folders = append(w.cfg.Folders, fld)
w.replaces <- w.cfg
w.replaces <- w.cfg.Copy()
}
// Options returns the current options configuration object.
@ -202,7 +203,7 @@ func (w *Wrapper) SetOptions(opts OptionsConfiguration) {
w.mut.Lock()
defer w.mut.Unlock()
w.cfg.Options = opts
w.replaces <- w.cfg
w.replaces <- w.cfg.Copy()
}
// GUI returns the current GUI configuration object.
@ -217,7 +218,7 @@ func (w *Wrapper) SetGUI(gui GUIConfiguration) {
w.mut.Lock()
defer w.mut.Unlock()
w.cfg.GUI = gui
w.replaces <- w.cfg
w.replaces <- w.cfg.Copy()
}
// Sets the folder error state. Emits ConfigSaved to cause a GUI refresh.
@ -236,7 +237,7 @@ func (w *Wrapper) SetFolderError(id string, err error) {
if errstr != w.cfg.Folders[i].Invalid {
w.cfg.Folders[i].Invalid = errstr
events.Default.Log(events.ConfigSaved, w.cfg)
w.replaces <- w.cfg
w.replaces <- w.cfg.Copy()
}
return
}