Refactor Status struct

pkg/instance/instance.go

@@ -16,23 +16,21 @@ import (
 
 // Instance represents a running instance of the llama server
 type Instance struct {
-	Name                   string                 `json:"name"`
-	options                *CreateInstanceOptions `json:"-"`
+	// Immutable identity (no locking needed after creation)
+	Name    string `json:"name"`
+	Created int64  `json:"created,omitempty"` // Unix timestamp when the instance was created
+
+	// Mutable state - each owns its own lock
+	status  *status                `json:"status"` // unexported - status owns its lock
+	options *CreateInstanceOptions `json:"-"`
+
+	// Global configuration (read-only, no lock needed)
 	globalInstanceSettings *config.InstancesConfig
 	globalBackendSettings  *config.BackendConfig
 
-	// Status
-	Status         Status `json:"status"`
-	onStatusChange func(oldStatus, newStatus Status)
-
-	// Creation time
-	Created int64 `json:"created,omitempty"` // Unix timestamp when the instance was created
-
-	// Logging file
-	logger *logger `json:"-"`
-
-	// Proxy component
-	proxy *proxy `json:"-"` // HTTP proxy and request tracking
+	// Components (can be nil for remote instances or when stopped)
+	logger *logger `json:"-"` // nil for remote instances
+	proxy  *proxy  `json:"-"` // nil for remote instances
 
 	// internal
 	cmd      *exec.Cmd          `json:"-"` // Command to run the instance
@@ -56,6 +54,10 @@ func NewInstance(name string, globalBackendSettings *config.BackendConfig, globa
 	// Create the instance logger
 	logger := NewLogger(name, globalInstanceSettings.LogsDir)
 
+	// Create status wrapper
+	status := newStatus(Stopped)
+	status.onStatusChange = onStatusChange
+
 	instance := &Instance{
 		Name:                   name,
 		options:                options,
@@ -63,8 +65,7 @@ func NewInstance(name string, globalBackendSettings *config.BackendConfig, globa
 		globalBackendSettings:  globalBackendSettings,
 		logger:                 logger,
 		Created:                time.Now().Unix(),
-		Status:                 Stopped,
-		onStatusChange:         onStatusChange,
+		status:                 status,
 	}
 
 	// Create Proxy component
@@ -79,6 +80,29 @@ func (i *Instance) GetOptions() *CreateInstanceOptions {
 	return i.options
 }
 
+// GetStatus returns the current status, delegating to status component
+func (i *Instance) GetStatus() Status {
+	if i.status == nil {
+		return Stopped
+	}
+	return i.status.Get()
+}
+
+// SetStatus sets the status, delegating to status component
+func (i *Instance) SetStatus(s Status) {
+	if i.status != nil {
+		i.status.Set(s)
+	}
+}
+
+// IsRunning returns true if the status is Running, delegating to status component
+func (i *Instance) IsRunning() bool {
+	if i.status == nil {
+		return false
+	}
+	return i.status.IsRunning()
+}
+
 func (i *Instance) GetPort() int {
 	i.mu.RLock()
 	defer i.mu.RUnlock()
@@ -182,14 +206,17 @@ func (i *Instance) MarshalJSON() ([]byte, error) {
 		}
 	}
 
-	// Use anonymous struct to avoid recursion
-	type Alias Instance
+	// Explicitly serialize to maintain backward compatible JSON format
 	return json.Marshal(&struct {
-		*Alias
+		Name          string                 `json:"name"`
+		Status        *status                `json:"status"`
+		Created       int64                  `json:"created,omitempty"`
 		Options       *CreateInstanceOptions `json:"options,omitempty"`
 		DockerEnabled bool                   `json:"docker_enabled,omitempty"`
 	}{
-		Alias:         (*Alias)(i),
+		Name:          i.Name,
+		Status:        i.status,
+		Created:       i.Created,
 		Options:       i.options,
 		DockerEnabled: dockerEnabled,
 	})
@@ -197,26 +224,33 @@ func (i *Instance) MarshalJSON() ([]byte, error) {
 
 // UnmarshalJSON implements json.Unmarshaler for Instance
 func (i *Instance) UnmarshalJSON(data []byte) error {
-	// Use anonymous struct to avoid recursion
-	type Alias Instance
+	// Explicitly deserialize to match MarshalJSON format
 	aux := &struct {
-		*Alias
+		Name    string                 `json:"name"`
+		Status  *status                `json:"status"`
+		Created int64                  `json:"created,omitempty"`
 		Options *CreateInstanceOptions `json:"options,omitempty"`
-	}{
-		Alias: (*Alias)(i),
-	}
+	}{}
 
 	if err := json.Unmarshal(data, aux); err != nil {
 		return err
 	}
 
+	// Set the fields
+	i.Name = aux.Name
+	i.Created = aux.Created
+	i.status = aux.Status
+
 	// Handle options with validation and defaults
 	if aux.Options != nil {
 		aux.Options.ValidateAndApplyDefaults(i.Name, i.globalInstanceSettings)
 		i.options = aux.Options
 	}
 
-	// Initialize fields that are not serialized
+	// Initialize fields that are not serialized or may be nil
+	if i.status == nil {
+		i.status = newStatus(Stopped)
+	}
 	if i.logger == nil && i.globalInstanceSettings != nil {
 		i.logger = NewLogger(i.Name, i.globalInstanceSettings.LogsDir)
 	}

pkg/instance/status.go

@@ -3,9 +3,10 @@ package instance
 import (
 	"encoding/json"
 	"log"
+	"sync"
 )
 
-// Enum for instance status
+// Status is the enum for status values (exported).
 type Status int
 
 const (
@@ -26,24 +27,7 @@ var statusToName = map[Status]string{
 	Failed:  "failed",
 }
 
-func (p *Instance) SetStatus(status Status) {
-	oldStatus := p.Status
-	p.Status = status
-
-	if p.onStatusChange != nil {
-		p.onStatusChange(oldStatus, status)
-	}
-}
-
-func (p *Instance) GetStatus() Status {
-	return p.Status
-}
-
-// IsRunning returns true if the status is Running
-func (p *Instance) IsRunning() bool {
-	return p.Status == Running
-}
-
+// Status enum JSON marshaling methods
 func (s Status) MarshalJSON() ([]byte, error) {
 	name, ok := statusToName[s]
 	if !ok {
@@ -52,7 +36,7 @@ func (s Status) MarshalJSON() ([]byte, error) {
 	return json.Marshal(name)
 }
 
-// UnmarshalJSON implements json.Unmarshaler
+// UnmarshalJSON implements json.Unmarshaler for Status enum
 func (s *Status) UnmarshalJSON(data []byte) error {
 	var str string
 	if err := json.Unmarshal(data, &str); err != nil {
@@ -68,3 +52,61 @@ func (s *Status) UnmarshalJSON(data []byte) error {
 	*s = status
 	return nil
 }
+
+// status represents the instance status with thread-safe access (unexported).
+type status struct {
+	mu sync.RWMutex
+	s  Status
+
+	// Callback for status changes
+	onStatusChange func(oldStatus, newStatus Status)
+}
+
+// newStatus creates a new status wrapper with the given initial status
+func newStatus(initial Status) *status {
+	return &status{
+		s: initial,
+	}
+}
+
+// Get returns the current status
+func (st *status) Get() Status {
+	st.mu.RLock()
+	defer st.mu.RUnlock()
+	return st.s
+}
+
+// Set updates the status and triggers the onStatusChange callback if set
+func (st *status) Set(newStatus Status) {
+	st.mu.Lock()
+	oldStatus := st.s
+	st.s = newStatus
+	callback := st.onStatusChange
+	st.mu.Unlock()
+
+	// Call the callback outside the lock to avoid potential deadlocks
+	if callback != nil {
+		callback(oldStatus, newStatus)
+	}
+}
+
+// IsRunning returns true if the status is Running
+func (st *status) IsRunning() bool {
+	st.mu.RLock()
+	defer st.mu.RUnlock()
+	return st.s == Running
+}
+
+// MarshalJSON implements json.Marshaler for status wrapper
+func (st *status) MarshalJSON() ([]byte, error) {
+	st.mu.RLock()
+	defer st.mu.RUnlock()
+	return st.s.MarshalJSON()
+}
+
+// UnmarshalJSON implements json.Unmarshaler for status wrapper
+func (st *status) UnmarshalJSON(data []byte) error {
+	st.mu.Lock()
+	defer st.mu.Unlock()
+	return st.s.UnmarshalJSON(data)
+}

pkg/manager/manager.go

@@ -289,7 +289,7 @@ func (im *instanceManager) loadInstance(name, path string) error {
 
 	// Restore persisted fields that NewInstance doesn't set
 	inst.Created = persistedInstance.Created
-	inst.SetStatus(persistedInstance.Status)
+	inst.SetStatus(persistedInstance.GetStatus())
 
 	// Handle remote instance mapping
 	if isRemote {

pkg/manager/operations.go

@@ -39,7 +39,7 @@ func (im *instanceManager) updateLocalInstanceFromRemote(localInst *instance.Ins
 
 	// Update the local instance with all remote data
 	localInst.SetOptions(&updatedOptions)
-	localInst.Status = remoteInst.Status
+	localInst.SetStatus(remoteInst.GetStatus())
 	localInst.Created = remoteInst.Created
 }