Skip to content

A failed read-lock restore in the two legacy migration paths leaves an unmatched RUnlock #463

Open

Description

@blacksoxx

Description

Two legacy-migration paths drop a read lock to take the write lock and restore it afterwards, both with the request context:

  • appendGobObjectFromFile (pkg/registry/file/storage.go): s.locks.RUnlock(key), then defer s.locks.RLock(ctx, key) whose error is discarded, while the function's outer defer s.locks.RUnlock(key) still runs.
  • get with hasReadLock: after a failed Lock, the restore s.locks.RLock(ctx, key) is best effort; on failure it logs and returns, and the caller's deferred RUnlock runs anyway.

MapMutex.RLock succeeds on its fast path without looking at the context, so the restore fails only when a writer holds or waits for the key and the context is done, which is the same condition that made the Lock fail: a client that gave up on a long LIST during a migration under write contention. Then RUnlock runs for a lock this goroutine does not hold. If the key's state is still there, readers goes to -1 and Lock's readers == 0 check never passes again, so every write to that key times out from then on. If the writer released in between and the state was evicted, RUnlock dereferences a nil keyState.

GuaranteedUpdate already avoids this for its own relock with relockCtx := context.WithoutCancel(ctx).

Expected behavior

A restore that cannot fail on cancellation, or an RUnlock that cannot corrupt the count.

@matthyx, would you take the context.WithoutCancel restore in both paths (with the error surfaced instead of deferred), a tolerant RUnlock, or both? I can take it once decided.

Environment

storage main at 4794d89.

Activity

  1. matthyx commented on Oct 26, 2026

    @matthyx
    Contributor

    I'd take the context.WithoutCancel like in GuaranteedUpdate

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions