Fix: lock

[xav-developer]

Description

The Spiral\RoadRunner\KeyValue\Cache class will set the value regardless of whether it exists or not - it always resolves the lock, you need to check for the existence of the key using the has method

Checklist

• My code follows the style guidelines of this project
• I have performed a self-review of my own code
• I have commented my code, particularly in hard-to-understand areas
• I wrote unit tests for my code (if tests is required for my changes)
• I have made changes in CHANGELOG.md file

Summary by CodeRabbit

• Bug Fixes

  • Improved lock acquisition to prevent overwriting existing locks by failing fast when a lock already exists.
  • Made cache set operations consistently write and always release associated locks, reducing potential deadlocks.
  • Clarified that key existence should be checked before setting values.

• Documentation

  • Updated changelog to reflect revised cache write behavior and lock handling.

[coderabbitai]

Walkthrough

Introduces an early-exit check in RoadRunnerLock::acquire to return false when the lock key already exists. Updates CHANGELOG to note that Spiral\RoadRunner\KeyValue\Cache now sets values unconditionally and always resolves the lock, requiring callers to check key existence via has().

Changes

Cohort / File(s)	Summary of Changes
Lock acquisition logic src/Cache/RoadRunnerLock.php	Added a pre-check in acquire: if lock key exists, return false before attempting to set; otherwise proceed with set logic. No public signatures changed.
Documentation / Changelog CHANGELOG.md	Documented behavior update: Cache sets value unconditionally and always resolves the lock; callers must use has() to check existence.

Sequence Diagram(s)

sequenceDiagram
  autonumber
  actor Caller
  participant Lock as RoadRunnerLock
  participant Store as KeyValue Storage

  Caller->>Lock: acquire(key, ttl)
  Lock->>Store: has(key)?
  alt Key exists (early exit)
    Store-->>Lock: true
    Lock-->>Caller: false
  else Key absent
    Store-->>Lock: false
    Lock->>Store: set(key, lockToken, ttl)
    Store-->>Lock: set result (ok/fail)
    alt set ok
      Lock-->>Caller: true
    else set failed
      Lock-->>Caller: false
    end
  end

Loading

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Suggested labels

type:bug

Suggested reviewers

• butschster

Poem

A nibble of logic, a hop to the lock,
I peek at the burrow before I knock.
If somebun’s in, I’ll tiptoe away—
If not, I’ll settle and softly stay.
Changelog carrots lined in a row,
Cache now sets—has() lets you know. 🥕🐇

Pre-merge checks and finishing touches

❌ Failed checks (1 inconclusive)

Check name	Status	Explanation	Resolution
Title Check	❓ Inconclusive	The title “Fix: lock” is overly generic and does not clearly convey the specific change of adding a pre-check to prevent reacquiring an existing lock in the RoadRunner cache implementation. It fails to summarize the primary behavior adjustment introduced by this pull request.	Consider revising the title to explicitly describe the key change, for example “Prevent reacquiring existing locks in RoadRunnerLock::acquire,” so that reviewers immediately understand the nature of the fix.

✅ Passed checks (2 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Docstring Coverage	✅ Passed	Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.

✨ Finishing touches

• 📝 Generate Docstrings

🧪 Generate unit tests

• Create PR with unit tests
• Post copyable unit tests in a comment

---

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between f7b7c23 and 8e07bdd.

📒 Files selected for processing (2)

• CHANGELOG.md (1 hunks)
• src/Cache/RoadRunnerLock.php (1 hunks)

🔇 Additional comments (2)

CHANGELOG.md (1)

24-24: LGTM!

The CHANGELOG entry clearly documents the behavior change and provides the necessary guidance for callers to check key existence using the has method.

src/Cache/RoadRunnerLock.php (1)

28-36: Lock acquisition isn’t atomic—storage.has() followed by storage.set() is a TOCTOU race. I didn’t find any atomic “set if not exists” or CAS methods on StorageInterface; verify whether the Spiral RoadRunner key-value store supports atomic lock ops or switch to a backend/algorithm that does.

Warning

There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure.

🔧 PHPStan (2.1.29)

Path /fixes does not exist

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}