diff --git a/pkg/cookiebanner/tracker_mapping_worker.go b/pkg/cookiebanner/tracker_mapping_worker.go index fd151a2a8..e069ff5c4 100644 --- a/pkg/cookiebanner/tracker_mapping_worker.go +++ b/pkg/cookiebanner/tracker_mapping_worker.go @@ -289,7 +289,9 @@ func (h *trackerMappingHandler) Process(ctx context.Context, tp coredata.Tracker // Phase 4: persist the pattern mapping in a short transaction. The // unmatched fallback keeps catalog coverage complete even when no // vendor was resolved. - return h.pg.WithTx( + mapped := true + + if err := h.pg.WithTx( ctx, func(ctx context.Context, tx pg.Tx) error { if commonPatternID == nil { @@ -331,6 +333,8 @@ func (h *trackerMappingHandler) Process(ctx context.Context, tp coredata.Tracker log.String("tracker_pattern_id", tp.ID.String()), ) + mapped = false + return nil } @@ -344,20 +348,39 @@ func (h *trackerMappingHandler) Process(ctx context.Context, tp coredata.Tracker log.String("tracker_pattern_id", tp.ID.String()), ) - // This run newly resolved a catalog third party, so - // same-banner siblings that share an initiator domain but - // were processed earlier and left unmatched can now match - // against it. Re-arm their mapping so the worker revisits - // them; the guards keep already-mapped siblings untouched. - if commonThirdPartyID != nil && !det.commonThirdPartyPreexisted { - if err := h.reenqueueUnmappedSiblings(ctx, tx, tp, det.domains); err != nil { - return err - } - } - return nil }, - ) + ); err != nil { + return err + } + + // Phase 5: re-arm same-banner siblings in a separate short + // transaction, after the mapping above has committed. This run newly + // resolved a catalog third party, so siblings that share an initiator + // domain but were processed earlier and left unmatched can now match + // against it. Re-arm their mapping so the worker revisits them; the + // guards keep already-mapped siblings untouched. + // + // The re-enqueue must not run inside the Phase 4 transaction: that + // transaction holds the row lock on tp, and the sibling UPDATE then + // takes locks on other tracker_patterns rows while holding it. Two + // workers mapping sibling patterns on the same banner would acquire + // those row locks in opposite orders and deadlock. Committing Phase 4 + // first releases tp's lock, and RequestMappingForUnmappedSiblings + // takes its locks in a deterministic id order, so the two can no + // longer cycle. + if mapped && commonThirdPartyID != nil && !det.commonThirdPartyPreexisted { + if err := h.pg.WithTx( + ctx, + func(ctx context.Context, tx pg.Tx) error { + return h.reenqueueUnmappedSiblings(ctx, tx, tp, det.domains) + }, + ); err != nil { + return err + } + } + + return nil } // deterministicResult carries the outcome of the pure-SQL catalog diff --git a/pkg/coredata/tracker_pattern.go b/pkg/coredata/tracker_pattern.go index 8206cbe93..3ed26767d 100644 --- a/pkg/coredata/tracker_pattern.go +++ b/pkg/coredata/tracker_pattern.go @@ -1286,26 +1286,37 @@ func (tps *TrackerPatterns) RequestMappingForUnmappedSiblings( return 0, nil } + // The target rows are locked through an ORDER BY id ... FOR UPDATE + // subquery so concurrent re-enqueues over overlapping sibling sets + // always acquire their row locks in the same ascending id order. Two + // workers mapping sibling patterns on the same banner would otherwise + // lock the shared rows in opposite orders and deadlock (40P01). q := ` UPDATE tracker_patterns SET mapping_requested_at = NOW(), updated_at = NOW() -WHERE - %[1]s - AND cookie_banner_id = @cookie_banner_id - AND id != @exclude_pattern_id - AND third_party_id IS NULL - AND mapping_requested_at IS NULL - AND (source IS NULL OR source != @extension_source) - AND id IN ( - SELECT DISTINCT tracker_pattern_id - FROM detected_trackers - WHERE %[1]s - AND cookie_banner_id = @cookie_banner_id - AND initiator_domain = ANY(@domains) - AND tracker_pattern_id IS NOT NULL - ) +WHERE id IN ( + SELECT id + FROM tracker_patterns + WHERE + %[1]s + AND cookie_banner_id = @cookie_banner_id + AND id != @exclude_pattern_id + AND third_party_id IS NULL + AND mapping_requested_at IS NULL + AND (source IS NULL OR source != @extension_source) + AND id IN ( + SELECT DISTINCT tracker_pattern_id + FROM detected_trackers + WHERE %[1]s + AND cookie_banner_id = @cookie_banner_id + AND initiator_domain = ANY(@domains) + AND tracker_pattern_id IS NOT NULL + ) + ORDER BY id + FOR UPDATE +) ` q = fmt.Sprintf(q, scope.SQLFragment())