From 3495fb1c76763ec9f3ba9273e2eb52005da86743 Mon Sep 17 00:00:00 2001 From: Dane Everitt Date: Wed, 16 Dec 2020 22:03:35 -0800 Subject: [PATCH] Less racey logic for resource usage --- environment/docker/stats.go | 2 +- environment/stats.go | 16 -------------- router/router_server.go | 2 +- server/resources.go | 42 ++++++++++++++++--------------------- server/state.go | 9 +++----- 5 files changed, 23 insertions(+), 48 deletions(-) diff --git a/environment/docker/stats.go b/environment/docker/stats.go index 5637ba7..00fe5a1 100644 --- a/environment/docker/stats.go +++ b/environment/docker/stats.go @@ -61,7 +61,7 @@ func (e *Environment) pollResources(ctx context.Context) error { atomic.AddUint64(&tx, nw.RxBytes) } - st := &environment.Stats{ + st := environment.Stats{ Memory: calculateDockerMemory(v.MemoryStats), MemoryLimit: v.MemoryStats.Limit, CpuAbsolute: calculateDockerAbsoluteCpu(&v.PreCPUStats, &v.CPUStats), diff --git a/environment/stats.go b/environment/stats.go index c4e25cb..8dbff47 100644 --- a/environment/stats.go +++ b/environment/stats.go @@ -1,13 +1,9 @@ package environment -import "sync" - // Defines the current resource usage for a given server instance. If a server is offline you // should obviously expect memory and CPU usage to be 0. However, disk will always be returned // since that is not dependent on the server being running to collect that data. type Stats struct { - mu sync.RWMutex - // The total amount of memory, in bytes, that this server instance is consuming. This is // calculated slightly differently than just using the raw Memory field that the stats // return from the container, so please check the code setting this value for how that @@ -33,15 +29,3 @@ type Stats struct { TxBytes uint64 `json:"tx_bytes"` } `json:"network"` } - -// Resets the usages values to zero, used when a server is stopped to ensure we don't hold -// onto any values incorrectly. -func (s *Stats) Empty() { - s.mu.Lock() - defer s.mu.Unlock() - - s.Memory = 0 - s.CpuAbsolute = 0 - s.Network.TxBytes = 0 - s.Network.RxBytes = 0 -} diff --git a/router/router_server.go b/router/router_server.go index b65f5e6..04497aa 100644 --- a/router/router_server.go +++ b/router/router_server.go @@ -23,7 +23,7 @@ func getServer(c *gin.Context) { s := GetServer(c.Param("server")) c.JSON(http.StatusOK, serverProcData{ - ResourceUsage: *s.Proc(), + ResourceUsage: s.Proc(), Suspended: s.IsSuspended(), }) } diff --git a/server/resources.go b/server/resources.go index d6072b9..45c4292 100644 --- a/server/resources.go +++ b/server/resources.go @@ -1,7 +1,6 @@ package server import ( - "encoding/json" "github.com/pterodactyl/wings/environment" "github.com/pterodactyl/wings/system" "sync" @@ -26,32 +25,27 @@ type ResourceUsage struct { Disk int64 `json:"disk_bytes"` } -// Custom marshaler to ensure that the object is locked when we're converting it to JSON in -// order to avoid race conditions. -func (ru *ResourceUsage) MarshalJSON() ([]byte, error) { - ru.mu.Lock() - defer ru.mu.Unlock() - - // Alias the resource usage so that we don't infinitely recurse when marshaling the struct. - type alias ResourceUsage - - return json.Marshal((*alias)(ru)) -} - -// Returns the resource usage stats for the server instance. If the server is not running, only the -// disk space currently used will be returned. When the server is running all of the other stats will -// be returned. -// -// When a process is stopped all of the stats are zeroed out except for the disk. -func (s *Server) Proc() *ResourceUsage { +// Returns the current resource usage stats for the server instance. This returns +// a copy of the tracked resources, so making any changes to the response will not +// have the desired outcome for you most likely. +func (s *Server) Proc() ResourceUsage { + s.resources.mu.Lock() + defer s.resources.mu.Unlock() // Store the updated disk usage when requesting process usage. atomic.StoreInt64(&s.resources.Disk, s.Filesystem().CachedUsage()) + //goland:noinspection GoVetCopyLock + return s.resources +} - // Acquire a lock before attempting to return the value of resources. - s.resources.mu.RLock() - defer s.resources.mu.RUnlock() - - return &s.resources +// Resets the usages values to zero, used when a server is stopped to ensure we don't hold +// onto any values incorrectly. +func (ru *ResourceUsage) Reset() { + ru.mu.Lock() + defer ru.mu.Unlock() + ru.Memory = 0 + ru.CpuAbsolute = 0 + ru.Network.TxBytes = 0 + ru.Network.RxBytes = 0 } func (s *Server) emitProcUsage() { diff --git a/server/state.go b/server/state.go index 52573f9..c845e46 100644 --- a/server/state.go +++ b/server/state.go @@ -62,11 +62,11 @@ func saveServerStates() error { // Sets the state of the server internally. This function handles crash detection as // well as reporting to event listeners for the server. func (s *Server) OnStateChange() { - prevState := s.Proc().State.Load() + prevState := s.resources.State.Load() st := s.Environment.State() // Update the currently tracked state for the server. - s.Proc().State.Store(st) + s.resources.State.Store(st) // Emit the event to any listeners that are currently registered. if prevState != s.Environment.State() { @@ -91,10 +91,7 @@ func (s *Server) OnStateChange() { // Reset the resource usage to 0 when the process fully stops so that all of the UI // views in the Panel correctly display 0. if st == environment.ProcessOfflineState { - s.resources.mu.Lock() - s.resources.Empty() - s.resources.mu.Unlock() - + s.resources.Reset() s.emitProcUsage() }