Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(cache): prevent concurrent map read/writes in memory cache Get method #49

Open
wants to merge 8 commits into
base: develop
Choose a base branch
from
2 changes: 1 addition & 1 deletion internal/api/handlers/health.go
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ func NewHealthHandler(db DatabaseService, health *services.HealthService, creato

func (h *HealthHandler) CheckHealth(c *gin.Context) {
// Create a context with timeout for the entire health check operation
ctx, cancel := context.WithTimeout(c.Request.Context(), 10*time.Second)
ctx, cancel := context.WithTimeout(c.Request.Context(), 15*time.Second)
defer cancel()

serviceID := c.Param("service")
Expand Down
62 changes: 51 additions & 11 deletions internal/api/handlers/plex.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ import (
"github.com/autobrr/dashbrr/internal/services/resilience"
"github.com/autobrr/dashbrr/internal/types"
"github.com/autobrr/dashbrr/internal/utils"
"encoding/json"
)

const (
Expand Down Expand Up @@ -114,18 +115,57 @@ func (h *PlexHandler) fetchSessionsWithCache(ctx context.Context, cacheKey strin
return nil, err
}

// Convert the cached data to PlexSessionsResponse
converted, err := utils.SafeStructConvert[types.PlexSessionsResponse](data)
if err != nil {
log.Error().
Err(err).
Str("cache_key", cacheKey).
Str("type", utils.GetTypeString(data)).
Msg("[Plex] Failed to convert cached data")
return nil, fmt.Errorf("failed to convert cached data: %w", err)
// Handle conversion based on the type of data
var sessions types.PlexSessionsResponse
switch v := data.(type) {
case *types.PlexSessionsResponse:
// If it's already the correct type, return it
return v, nil
case map[string]interface{}, []byte, string:
// Convert to JSON bytes if it's a map or already JSON
var jsonBytes []byte
var err error

switch vt := v.(type) {
case []byte:
jsonBytes = vt
case string:
jsonBytes = []byte(vt)
default:
jsonBytes, err = json.Marshal(v)
if err != nil {
log.Error().
Err(err).
Str("cache_key", cacheKey).
Str("type", utils.GetTypeString(data)).
Msg("[Plex] Failed to marshal cached data to JSON")
return nil, fmt.Errorf("failed to marshal cached data: %w", err)
}
}

// Unmarshal into the target struct
if err := json.Unmarshal(jsonBytes, &sessions); err != nil {
log.Error().
Err(err).
Str("cache_key", cacheKey).
Str("type", utils.GetTypeString(data)).
Msg("[Plex] Failed to unmarshal cached data")
return nil, fmt.Errorf("failed to unmarshal cached data: %w", err)
}
return &sessions, nil
default:
// For any other type, try struct conversion
converted, err := utils.SafeStructConvert[types.PlexSessionsResponse](data)
if err != nil {
log.Error().
Err(err).
Str("cache_key", cacheKey).
Str("type", utils.GetTypeString(data)).
Msg("[Plex] Failed to convert cached data")
return nil, fmt.Errorf("failed to convert cached data: %w", err)
}
return &converted, nil
}

return &converted, nil
}

func (h *PlexHandler) GetPlexSessions(c *gin.Context) {
Expand Down
22 changes: 8 additions & 14 deletions internal/api/handlers/settings.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@ import (
"github.com/autobrr/dashbrr/internal/models"
"github.com/autobrr/dashbrr/internal/services"
"github.com/autobrr/dashbrr/internal/services/cache"
"github.com/autobrr/dashbrr/internal/services/manager"
"github.com/autobrr/dashbrr/internal/types"
)

Expand All @@ -28,20 +27,18 @@ const (
)

type SettingsHandler struct {
db *database.DB
health *services.HealthService
cache cache.Store
serviceManager *manager.ServiceManager
lastDebugLog time.Time
db *database.DB
health *services.HealthService
cache cache.Store
lastDebugLog time.Time
}

func NewSettingsHandler(db *database.DB, health *services.HealthService, cache cache.Store) *SettingsHandler {
return &SettingsHandler{
db: db,
health: health,
cache: cache,
serviceManager: manager.NewServiceManager(db, cache),
lastDebugLog: time.Now().Add(-configDebugLogTTL), // Initialize to ensure first log happens
db: db,
health: health,
cache: cache,
lastDebugLog: time.Now().Add(-configDebugLogTTL), // Initialize to ensure first log happens
}
}

Expand Down Expand Up @@ -150,9 +147,6 @@ func (h *SettingsHandler) SaveSettings(c *gin.Context) {
return
}

// Initialize service data
h.serviceManager.InitializeService(c.Request.Context(), &config)

// Invalidate cache
if err := h.cache.Delete(context.Background(), configCacheKey); err != nil {
log.Warn().Err(err).Msg("Failed to delete configuration cache")
Expand Down
18 changes: 15 additions & 3 deletions internal/services/cache/memory.go
Original file line number Diff line number Diff line change
Expand Up @@ -164,13 +164,25 @@ func (s *MemoryStore) Get(ctx context.Context, key string, value interface{}) er
s.local.RLock()
item, exists := s.local.items[key]
if exists && time.Now().Before(item.expiration) {
// Make a copy of the value while holding the read lock
data := make([]byte, len(item.value))
copy(data, item.value)
s.local.RUnlock()
return json.Unmarshal(item.value, value)
return json.Unmarshal(data, value)
}
s.local.RUnlock()

if exists {
delete(s.local.items, key)
// Need write lock for deletion
s.local.Lock()
// Check again after acquiring write lock as state might have changed
if item, stillExists := s.local.items[key]; stillExists {
if time.Now().After(item.expiration) {
delete(s.local.items, key)
}
}
s.local.Unlock()
}
s.local.RUnlock()

return ErrKeyNotFound
}
Expand Down
148 changes: 0 additions & 148 deletions internal/services/manager/service_manager.go

This file was deleted.

2 changes: 1 addition & 1 deletion internal/services/overseerr/overseerr.go
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ func NewOverseerrService() models.ServiceHealthChecker {
service.Description = "Monitor and manage your Overseerr instance"
service.DefaultURL = "http://localhost:5055"
service.HealthEndpoint = "/api/v1/status"
service.SetTimeout(core.DefaultTimeout)
service.SetTimeout(core.DefaultLongTimeout) // Increased timeout for Overseerr
return service
}

Expand Down
2 changes: 1 addition & 1 deletion web/src/utils/api.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ const SERVICE_TIMEOUTS: Record<string, number> = {
'/api/autobrr/releases': 60000, // 1 minute for autobrr releases
'/api/plex/sessions': 5000, // 5 seconds for plex sessions
'/api/maintainerr': 600000, // 10 minutes for maintainerr
'/api/overseerr': 30000, // 30 seconds for overseerr
'/api/overseerr': 60000, // 1 minute for overseerr (increased from 30s)
'/api/radarr': 60000, // 1 minute for radarr
'/api/sonarr': 60000, // 1 minute for sonarr
'/api/prowlarr': 60000, // 1 minute for prowlarr
Expand Down