Fix race condition in concurrent artifact additions

This fixes a race condition where concurrent 'podman artifact add'
commands for different artifacts would result in only one artifact
being created, without any error messages.

The root cause was in the artifact store's Add() method. When the lock
was released during blob copying (for performance), concurrent additions
could overwrite each other's index entries:

- Process A: Lock → Read index → Unlock → Copy blobs → Lock → Commit
- Process B: Lock → Read index (missing A) → Unlock → Copy blobs → Lock → Commit
- Result: Process B's commit overwrites Process A's entry

The fix adds conflict detection after reacquiring the lock. Before
committing, we reload the artifact index and verify the artifact name
hasn't been added by another process. If a conflict is detected, we
return an error rather than silently overwriting the concurrent change.

A 'locked' boolean variable tracks lock state to prevent double-unlock
if an error occurs during blob copying (when the lock is released).
The deferred unlock only fires if 'locked' is true.

This preserves the performance benefits of concurrent blob copying
while preventing the race condition that caused artifacts to be lost.

Fixes: containers/podman#27569

Signed-off-by: Daniel J Walsh <dwalsh@redhat.com>

Author: rhatdan

common/pkg/libartifact/store/store.go

@@ -218,8 +218,8 @@ func (as ArtifactStore) Add(ctx context.Context, dest string, artifactBlobs []li
 		return nil, errors.New("append option is not compatible with type option")
 	}
 
-	locked := true
 	as.lock.Lock()
+	locked := true
 	defer func() {
 		if locked {
 			as.lock.Unlock()
@@ -297,10 +297,10 @@ func (as ArtifactStore) Add(ctx context.Context, dest string, artifactBlobs []li
 	}
 	defer imageDest.Close()
 
-	// Unlock around the actual pull of the blobs.
-	// This is ugly as hell, but should be safe.
-	locked = false
+	// Release the lock during blob copying to allow concurrent operations.
+	// We'll reacquire it later and check for conflicts before committing.
 	as.lock.Unlock()
+	locked = false
 
 	// ImageDestination, in general, requires the caller to write a full image; here we may write only the added layers.
 	// This works for the oci/layout transport we hard-code.
@@ -356,9 +356,24 @@ func (as ArtifactStore) Add(ctx context.Context, dest string, artifactBlobs []li
 		artifactManifest.Layers = append(artifactManifest.Layers, newLayer)
 	}
 
+	// Reacquire the lock before committing changes
 	as.lock.Lock()
 	locked = true
 
+	// Reload artifacts to check if another process added the same artifact
+	// while we were copying blobs (without the lock)
+	if !options.Append {
+		updatedArtifacts, err := as.getArtifacts(ctx, nil)
+		if err != nil {
+			return nil, err
+		}
+		// Check if artifact now exists (conflict with concurrent add)
+		_, _, err = updatedArtifacts.GetByNameOrDigest(dest)
+		if err == nil {
+			return nil, fmt.Errorf("%s: %w", dest, libartTypes.ErrArtifactAlreadyExists)
+		}
+	}
+
 	rawData, err := json.Marshal(artifactManifest)
 	if err != nil {
 		return nil, err