-
Notifications
You must be signed in to change notification settings - Fork 70
Open
Description
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
Labels
No labels