diff --git a/pkg/atproto/atproto.go b/pkg/atproto/atproto.go index 21555023..fb28b0de 100644 --- a/pkg/atproto/atproto.go +++ b/pkg/atproto/atproto.go @@ -63,16 +63,19 @@ func (atsync *ATProtoSynchronizer) SyncBlueskyRepo(ctx context.Context, handle s // back, and a row can vanish mid-walk — an operator deleting it to force // a resync is a long-standing habit. With no row, the walk's own record // visitor falls through to here and would lock the mutex its caller - // already holds. An in-flight DID never wants a second sync started - // anyway; the record this call was serving is re-indexed by whatever - // sync runs next. - if syncInFlight(ident.DID.String()) { - return nil, fmt.Errorf("sync already in flight for %s", ident.DID.String()) + // already holds. The ctx marker identifies exactly that call tree and + // nothing else: an unrelated caller racing this DID's first sync still + // waits on the lock like it always has, instead of getting an error for + // a microsecond coincidence. The record the refused call was serving is + // re-indexed by whatever sync runs next. + if walkingDID(ctx) == ident.DID.String() { + return nil, fmt.Errorf("refusing re-entrant sync of %s inside its own walk", ident.DID.String()) } handleLock := handleLocks.GetLock(ident.DID.String()) handleLock.Lock() defer handleLock.Unlock() + ctx = markWalking(ctx, ident.DID.String()) // Tell re-entrant callers (handleCreateUpdate syncs the repos it sees // records from) that this DID's placeholder row is being filled in right @@ -214,6 +217,7 @@ func (atsync *ATProtoSynchronizer) DeepenRepo(ctx context.Context, did string) ( handleLock.Lock() defer handleLock.Unlock() defer markSyncInFlight(did)() + ctx = markWalking(ctx, did) ident, err := atsync.resolveIdent(ctx, did, true) if err != nil { @@ -263,6 +267,24 @@ func (atsync *ATProtoSynchronizer) DeepenRepo(ctx context.Context, did string) ( return window.Genesis, window.Lo, nil } +// walkingDIDKey carries, on a ctx, the DID whose repo walk this call tree is +// performing. It is how a re-entrant sync attempt for that DID — a record +// visitor calling back into SyncBlueskyRepo after the row vanished out from +// under its walk — can be refused instead of deadlocking on the per-DID lock +// its own caller holds. A ctx value rather than a registry on purpose: it +// names one call tree, so unrelated concurrent callers are never mistaken for +// re-entry. +type walkingDIDKey struct{} + +func markWalking(ctx context.Context, did string) context.Context { + return context.WithValue(ctx, walkingDIDKey{}, did) +} + +func walkingDID(ctx context.Context) string { + s, _ := ctx.Value(walkingDIDKey{}).(string) + return s +} + // syncsInFlight holds the DIDs whose backfill is running in this process right // now. A placeholder repo row (empty Version) otherwise means "incomplete, // re-sync me", which would be wrong -- and, since indexing a record can call