Skip to content

Completer can panic if completion races with job deletion #1303

Description

@logical-misha

Completer can panic if completion races with job deletion

Summary

JobSetStateIfRunningMany is allowed to ignore an unknown job without returning
an error. The shared driver test comments describe this as intentional because
a job can be deleted while a completer is trying to finalize it.

The inline and async completers call JobSetStateIfRunningMany, then
unconditionally publish jobs[0] to subscribers. If the driver returns no rows
for the ignored/deleted job, the inline path panics with an index-out-of-range
error. Backend paths that return a nil row slot can also lead to a subscriber
update whose Job is nil.

Confirmed against River master at
4aa884e927fc07635605967729a288ac9416ebf1 on 2026-07-10 with Go 1.26.5
on Linux. I did not find an exact existing public report during a targeted
search.

Why this seems wrong

  • The driver layer already treats missing completion rows as an expected race.
  • A tolerated storage race should not become a process panic in the completer.
  • Subscriber updates should be published only for rows that were actually
    updated.
  • The same guard should apply to inline, async, and batch completion paths.

Relevant code

In internal/jobcompleter/job_completer.go, the inline completer publishes the
first returned row without checking whether any row was returned:

jobs, err := c.pilot.JobSetStateIfRunningMany(ctx, c.exec, setStateParamsToMany(c.Time.NowOrNil(), c.schema, params))
if err != nil {
    return err
}

stats.CompleteDuration = c.Time.Now().Sub(start)
c.subscribeCh <- []CompleterJobUpdated{{
    Job:      jobs[0],
    JobStats: stats,
    Snoozed:  params.Snoozed,
}}

The async completer has the same jobs[0] publication shape. The shared driver
test UnknownJobIgnored documents why the lower layer allows an unknown job to
complete without error: the job may have been deleted while the completer was
trying to finalize it.

Reproduction

Add a focused no-DB test to internal/jobcompleter/job_completer_test.go:

func TestInlineJobCompleter_UnknownJobIgnored(t *testing.T) {
    t.Parallel()

    ctx := context.Background()

    execMock := &partialExecutorMock{
        JobSetStateIfRunningManyFunc: func(ctx context.Context, params *riverdriver.JobSetStateIfRunningManyParams) ([]*rivertype.JobRow, error) {
            require.Len(t, params.ID, 1)
            return nil, nil
        },
    }

    subscribeCh := make(chan []CompleterJobUpdated, 10)
    t.Cleanup(riverinternaltest.DiscardContinuously(subscribeCh))

    completer := NewInlineCompleter(riversharedtest.BaseServiceArchetype(t), "", execMock, &riverpilot.StandardPilot{}, subscribeCh)
    completer.disableSleep = true

    require.NotPanics(t, func() {
        err := completer.JobSetStateIfRunning(ctx, &jobstats.JobStatistics{}, riverdriver.JobSetStateCompleted(1, time.Now(), nil))
        require.NoError(t, err)
    })
}

Run:

go test ./internal/jobcompleter -run TestInlineJobCompleter_UnknownJobIgnored -count=1 -v

Current result:

Error:       func (assert.PanicTestFunc)(...) should not panic
Panic value: runtime error: index out of range [0] with length 0

The panic comes from InlineCompleter.JobSetStateIfRunning when it reads
jobs[0].

Expected behavior

If no row was updated, the completer should return without publishing a
completion update. That matches the driver behavior where a missing row is
treated as an already-resolved race.

Suggested fix

Guard event publication in the inline and async completers:

if len(jobs) < 1 || jobs[0] == nil {
    return nil
}

For the batch completer, skip nil returned rows while constructing subscriber
events, and publish only when at least one actual updated row exists.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Fields

    No fields configured for issues without a type.

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions