From 24d6b49481e7564e7308e1becf4281fb6ad837aa Mon Sep 17 00:00:00 2001
From: Evan Jarrett <evan@evanjarrett.com>
Date: Fri, 26 Dec 2025 09:48:25 -0600
Subject: [PATCH] clean up unused locks

---
 pkg/appview/storage/manifest_store.go     | 20 ++-----------
 pkg/appview/storage/routing_repository.go | 36 +++++++----------------
 2 files changed, 12 insertions(+), 44 deletions(-)

diff --git a/pkg/appview/storage/manifest_store.go b/pkg/appview/storage/manifest_store.go
index be6d3ce..76b6392 100644
--- a/pkg/appview/storage/manifest_store.go
+++ b/pkg/appview/storage/manifest_store.go
@@ -10,7 +10,6 @@ import (
 	"log/slog"
 	"net/http"
 	"strings"
-	"sync"
 	"time"
 
 	"atcr.io/pkg/appview/readme"
@@ -22,10 +21,8 @@ import (
 // ManifestStore implements distribution.ManifestService
 // It stores manifests in ATProto as records
 type ManifestStore struct {
-	ctx                *RegistryContext       // Context with user/hold info
-	mu                 sync.RWMutex           // Protects lastFetchedHoldDID
-	lastFetchedHoldDID string                 // Hold DID from most recently fetched manifest (for pull)
-	blobStore          distribution.BlobStore // Blob store for fetching config during push
+	ctx       *RegistryContext       // Context with user/hold info
+	blobStore distribution.BlobStore // Blob store for fetching config during push
 }
 
 // NewManifestStore creates a new ATProto-backed manifest store
@@ -66,19 +63,6 @@ func (s *ManifestStore) Get(ctx context.Context, dgst digest.Digest, options ...
 		return nil, fmt.Errorf("failed to unmarshal manifest record: %w", err)
 	}
 
-	// Store the hold DID for subsequent blob requests during pull
-	// Prefer HoldDID (new format) with fallback to HoldEndpoint (legacy URL format)
-	// The routing repository will cache this for concurrent blob fetches
-	s.mu.Lock()
-	if manifestRecord.HoldDID != "" {
-		// New format: DID reference (preferred)
-		s.lastFetchedHoldDID = manifestRecord.HoldDID
-	} else if manifestRecord.HoldEndpoint != "" {
-		// Legacy format: URL reference - convert to DID
-		s.lastFetchedHoldDID = atproto.ResolveHoldDIDFromURL(manifestRecord.HoldEndpoint)
-	}
-	s.mu.Unlock()
-
 	var ociManifest []byte
 
 	// New records: Download blob from ATProto blob storage
diff --git a/pkg/appview/storage/routing_repository.go b/pkg/appview/storage/routing_repository.go
index b990d3a..dc51be4 100644
--- a/pkg/appview/storage/routing_repository.go
+++ b/pkg/appview/storage/routing_repository.go
@@ -7,19 +7,19 @@ package storage
 import (
 	"context"
 	"log/slog"
-	"sync"
 
 	"github.com/distribution/distribution/v3"
 )
 
 // RoutingRepository routes manifests to ATProto and blobs to external hold service
 // The registry (AppView) is stateless and NEVER stores blobs locally
+// NOTE: A fresh instance is created per-request (see middleware/registry.go)
+// so no mutex is needed - each request has its own instance
 type RoutingRepository struct {
 	distribution.Repository
 	Ctx           *RegistryContext // All context and services (exported for token updates)
-	mu            sync.Mutex       // Protects manifestStore and blobStore
-	manifestStore *ManifestStore   // Cached manifest store instance
-	blobStore     *ProxyBlobStore  // Cached blob store instance
+	manifestStore *ManifestStore   // Manifest store instance (lazy-initialized)
+	blobStore     *ProxyBlobStore  // Blob store instance (lazy-initialized)
 }
 
 // NewRoutingRepository creates a new routing repository
@@ -32,36 +32,22 @@ func NewRoutingRepository(baseRepo distribution.Repository, ctx *RegistryContext
 
 // Manifests returns the ATProto-backed manifest service
 func (r *RoutingRepository) Manifests(ctx context.Context, options ...distribution.ManifestServiceOption) (distribution.ManifestService, error) {
-	r.mu.Lock()
-	// Create or return cached manifest store
+	// Lazy-initialize manifest store (no mutex needed - one instance per request)
 	if r.manifestStore == nil {
 		// Ensure blob store is created first (needed for label extraction during push)
-		// Release lock while calling Blobs to avoid deadlock
-		r.mu.Unlock()
 		blobStore := r.Blobs(ctx)
-		r.mu.Lock()
-
-		// Double-check after reacquiring lock (another goroutine might have set it)
-		if r.manifestStore == nil {
-			r.manifestStore = NewManifestStore(r.Ctx, blobStore)
-		}
+		r.manifestStore = NewManifestStore(r.Ctx, blobStore)
 	}
-	manifestStore := r.manifestStore
-	r.mu.Unlock()
-
-	return manifestStore, nil
+	return r.manifestStore, nil
 }
 
 // Blobs returns a proxy blob store that routes to external hold service
 // The registry (AppView) NEVER stores blobs locally - all blobs go through hold service
 func (r *RoutingRepository) Blobs(ctx context.Context) distribution.BlobStore {
-	r.mu.Lock()
-	// Return cached blob store if available
+	// Return cached blob store if available (no mutex needed - one instance per request)
 	if r.blobStore != nil {
-		blobStore := r.blobStore
-		r.mu.Unlock()
 		slog.Debug("Returning cached blob store", "component", "storage/blobs", "did", r.Ctx.DID, "repo", r.Ctx.Repository)
-		return blobStore
+		return r.blobStore
 	}
 
 	// Determine if this is a pull (GET/HEAD) or push (PUT/POST/etc) operation
@@ -103,9 +89,7 @@ func (r *RoutingRepository) Blobs(ctx context.Context) distribution.BlobStore {
 
 	// Create and cache proxy blob store
 	r.blobStore = NewProxyBlobStore(r.Ctx)
-	blobStore := r.blobStore
-	r.mu.Unlock()
-	return blobStore
+	return r.blobStore
 }
 
 // Tags returns the tag service