Skip to content

Incorrect lock condition: should be == locked, not != locked #25

@nntien155

Description

@nntien155

Bug Report: Incorrect Lock Condition

Date: 2025-08-07
Title: Incorrect lock condition: should be == locked, not != locked

🐞 Description

There appears to be a logic bug in the for loop condition that checks whether the cache is locked by another process.

Current Code

for err == nil && r[0] == nil && r[1].(string) != locked {
    debugf("empty result for %s locked by other, so sleep %s", key, c.Options.LockSleep.String())
    select {
    case <-ctx.Done():
        return "", ctx.Err()
    case <-time.After(c.Options.LockSleep):
    }
    r, err = c.luaGet(ctx, key, owner)
}

🔍 Problem

The condition r[1].(string) != locked is inverted. It causes the retry logic to run when the cache is not locked, which is not the intended behavior.

This results in unnecessary sleeping and retrying when the cache is actually free, and may prevent the fetchNew() logic from executing in a timely manner.

✅ Suggested Fix

Change the condition to check for == locked:

for err == nil && r[0] == nil && r[1].(string) == locked {
    debugf("empty result for %s locked by other, so sleep %s", key, c.Options.LockSleep.String())
    select {
    case <-ctx.Done():
        return "", ctx.Err()
    case <-time.After(c.Options.LockSleep):
    }
    r, err = c.luaGet(ctx, key, owner)
}

✅ Why It Matters

  • Ensures the retry loop only happens while the key is locked by another process and no data is yet available.
  • Avoids unnecessary sleeping.
  • Makes the logic more correct and predictable.

Let me know if you’d like a PR created with this fix.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions