Skip to content

Commit de90d97

Browse files
fix(pipeline): nil-deref in ProcessSingle on multi-Del-per-uid + strip stale READING debug prints
Two bugs surfaced by graphql/e2e/auth's TestOrderAndOffset (cleanup mutation \`mutation DelTask { deleteTask(filter: {}) }\` crashed the alpha mid-request, causing the test client to see EOF on POST): 1. posting/index.go ProcessSingle SIGSEGV at line 675. GraphQL deleteTask cleanup expands into multiple Del edges per entity (one per predicate the entity has — uid, type, list edges, etc.). When two Del edges to the same uid land in one transaction's batch, the second iteration through ProcessSingle's per-edge loop does: pl, exists := postings[uid] if exists { if edge.Op == DEL { oldVal = findSingleValueInPostingList(pl) if string(edge.Value) == string(oldVal.Value) { ... } ^^^^^^ nil deref } } findSingleValueInPostingList only returns Set postings; if the accumulated list holds only Dels (from the prior iteration), it returns nil and we panic dereferencing oldVal.Value. Two fixes here: - Guard the deref: \`if oldVal != nil && string(...) == ...\`. - Move \`var oldVal *pb.Posting\` inside the loop. It was declared at function scope, so a stale value from one edge could bleed into the nil-guarded branch for a different uid on a later iteration. Per-edge scope makes the intent explicit. 2. worker/task.go: two leftover \`fmt.Println("READING SINGLE", ...)\` and \`fmt.Println("READING", ...)\` calls in the value-postings read path. Same class of debug spew Phase 1B stripped from posting/, missed because that sweep didn't include worker/. Removed both. Safe — they were unconditional prints on every query value read. The graphql/e2e/auth and graphql/e2e/auth/debug_off packages now both pass in 30s. \`./posting/\` and \`./worker/\` unit tests still green. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1 parent 8ccdae6 commit de90d97

2 files changed

Lines changed: 9 additions & 6 deletions

File tree

posting/index.go

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -634,7 +634,6 @@ func (mp *MutationPipeline) ProcessSingle(ctx context.Context, pipeline *Predica
634634
dataKey := x.DataKey(pipeline.attr, 0)
635635
insertDeleteAllEdge := !(info.index || info.reverse || info.count) // nolint
636636

637-
var oldVal *pb.Posting
638637
for edge := range pipeline.edges {
639638
if edge.Op != pb.DirectedEdge_DEL && !schemaExists {
640639
return errors.Errorf("runMutation: Unable to find schema for %s", edge.Attr)
@@ -644,6 +643,9 @@ func (mp *MutationPipeline) ProcessSingle(ctx context.Context, pipeline *Predica
644643
return err
645644
}
646645

646+
// oldVal is reset per edge so a stale value from a previous iteration
647+
// can't bleed into the nil-guarded branch below.
648+
var oldVal *pb.Posting
647649
uid := edge.Entity
648650
pl, exists := postings[uid]
649651

@@ -671,8 +673,13 @@ func (mp *MutationPipeline) ProcessSingle(ctx context.Context, pipeline *Predica
671673

672674
if exists {
673675
if edge.Op == pb.DirectedEdge_DEL {
676+
// findSingleValueInPostingList returns nil when the
677+
// accumulated postings for this uid hold only Del entries
678+
// (no Set), which happens when the same uid receives
679+
// multiple Del edges in one batch (e.g. GraphQL
680+
// deleteTask cleanup deleting many predicates per entity).
674681
oldVal = findSingleValueInPostingList(pl)
675-
if string(edge.Value) == string(oldVal.Value) {
682+
if oldVal != nil && string(edge.Value) == string(oldVal.Value) {
676683
setPosting()
677684
}
678685
} else {

worker/task.go

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -487,8 +487,6 @@ func (qs *queryState) handleValuePostings(ctx context.Context, args funcArgs) er
487487
Value: p.Value,
488488
}
489489
}
490-
pk, _ := x.Parse(key)
491-
fmt.Println("READING SINGLE", pk, vals, pl)
492490
} else {
493491
pl, err := qs.cache.Get(key)
494492
if err != nil {
@@ -508,8 +506,6 @@ func (qs *queryState) handleValuePostings(ctx context.Context, args funcArgs) er
508506
}
509507

510508
vals, fcs, err = retrieveValuesAndFacets(args, pl, facetsTree, listType)
511-
pk, _ := x.Parse(key)
512-
fmt.Println("READING", pk, vals, fcs, pl.Print())
513509

514510
switch {
515511
case err == posting.ErrNoValue || (err == nil && len(vals) == 0):

0 commit comments

Comments
 (0)