Completely re-work the server configuration to be separated out better

server/update.go

@@ -15,7 +15,7 @@ import (
 // it is up to the specific environment to determine what needs to happen when
 // that is the case.
 func (s *Server) UpdateDataStructure(data []byte, background bool) error {
-	src := new(Server)
+	src := new(Configuration)
 	if err := json.Unmarshal(data, src); err != nil {
 		return errors.WithStack(err)
 	}
@@ -27,26 +27,30 @@ func (s *Server) UpdateDataStructure(data []byte, background bool) error {
 		return errors.New("attempting to merge a data stack with an invalid UUID")
 	}
 
-	s.Lock()
+	// Grab a copy of the configuration to work on.
+	c := s.Config()
+
+	// Lock the server configuration while we're doing this merge to avoid anything
+	// trying to overwrite it or make modifications while we're sorting out what we
+	// need to do.
+	s.cfg.mu.Lock()
+	defer s.cfg.mu.Unlock()
+
 	// Merge the new data object that we have received with the existing server data object
 	// and then save it to the disk so it is persistent.
-	if err := mergo.Merge(s, src, mergo.WithOverride); err != nil {
+	if err := mergo.Merge(c, src, mergo.WithOverride); err != nil {
 		return errors.WithStack(err)
 	}
 
-	// Mergo makes that previous lock disappear. Handle that by just re-locking the object.
-	s.Lock()
-	defer s.Unlock()
-
 	// Don't explode if we're setting CPU limits to 0. Mergo sees that as an empty value
 	// so it won't override the value we've passed through in the API call. However, we can
 	// safely assume that we're passing through valid data structures here. I foresee this
 	// backfiring at some point, but until then...
 	//
 	// We'll go ahead and do this with swap as well.
-	s.Build.CpuLimit = src.Build.CpuLimit
-	s.Build.Swap = src.Build.Swap
-	s.Build.DiskSpace = src.Build.DiskSpace
+	c.Build.CpuLimit = src.Build.CpuLimit
+	c.Build.Swap = src.Build.Swap
+	c.Build.DiskSpace = src.Build.DiskSpace
 
 	// Mergo can't quite handle this boolean value correctly, so for now we'll just
 	// handle this edge case manually since none of the other data passed through in this
@@ -56,7 +60,7 @@ func (s *Server) UpdateDataStructure(data []byte, background bool) error {
 			return errors.WithStack(err)
 		}
 	} else {
-		s.Container.OomDisabled = v
+		c.Container.OomDisabled = v
 	}
 
 	// Mergo also cannot handle this boolean value.
@@ -65,25 +69,29 @@ func (s *Server) UpdateDataStructure(data []byte, background bool) error {
 			return errors.WithStack(err)
 		}
 	} else {
-		s.Suspended = v
+		c.Suspended = v
 	}
 
 	// Environment and Mappings should be treated as a full update at all times, never a
 	// true patch, otherwise we can't know what we're passing along.
 	if src.EnvVars != nil && len(src.EnvVars) > 0 {
-		s.EnvVars = src.EnvVars
+		c.EnvVars = src.EnvVars
 	}
 
 	if src.Allocations.Mappings != nil && len(src.Allocations.Mappings) > 0 {
-		s.Allocations.Mappings = src.Allocations.Mappings
+		c.Allocations.Mappings = src.Allocations.Mappings
 	}
 
 	if src.Mounts != nil && len(src.Mounts) > 0 {
-		s.Mounts = src.Mounts
+		c.Mounts = src.Mounts
 	}
 
+	// Update the configuration once we have a lock on the configuration object.
+	s.cfg = *c
+	s.Uuid = c.Uuid
+
 	if background {
-		s.runBackgroundActions()
+		go s.runBackgroundActions()
 	}
 
 	return nil
@@ -96,24 +104,22 @@ func (s *Server) UpdateDataStructure(data []byte, background bool) error {
 // These tasks run in independent threads where relevant to speed up any updates
 // that need to happen.
 func (s *Server) runBackgroundActions() {
-	// Update the environment in place, allowing memory and CPU usage to be adjusted
-	// on the fly without the user needing to reboot (theoretically).
-	go func(server *Server) {
-		server.Log().Info("performing server limit modification on-the-fly")
-		if err := server.Environment.InSituUpdate(); err != nil {
-			server.Log().WithField("error", err).Warn("failed to perform on-the-fly update of the server environment")
-		}
-	}(s)
-
-	// Check if the server is now suspended, and if so and the process is not terminated
+	// Check if the s is now suspended, and if so and the process is not terminated
 	// yet, do it immediately.
-	go func(server *Server) {
-		if server.Suspended && server.GetState() != ProcessOfflineState {
-			server.Log().Info("server suspended with running process state, terminating now")
+	if s.IsSuspended() && s.GetState() != ProcessOfflineState {
+		s.Log().Info("server suspended with running process state, terminating now")
 
-			if err := server.Environment.WaitForStop(10, true); err != nil {
-				server.Log().WithField("error", err).Warn("failed to terminate server environment after suspension")
-			}
+		if err := s.Environment.WaitForStop(10, true); err != nil {
+			s.Log().WithField("error", err).Warn("failed to terminate server environment after suspension")
 		}
-	}(s)
+	}
+
+	if !s.IsSuspended() {
+		// Update the environment in place, allowing memory and CPU usage to be adjusted
+		// on the fly without the user needing to reboot (theoretically).
+		s.Log().Info("performing server limit modification on-the-fly")
+		if err := s.Environment.InSituUpdate(); err != nil {
+			s.Log().WithField("error", err).Warn("failed to perform on-the-fly update of the server environment")
+		}
+	}
 }