Skip to content

Conversation

Appla
Copy link
Contributor

@Appla Appla commented Sep 26, 2025

This PR introduces timeout-based locking support for the RWLock type in Swoole\\Lock, aligning its behavior with Mutex based.

Changes introduced:

  • lock_read()lock_read(?float $timeout = null): Enables acquiring a read lock with an optional timeout.
  • lockwait(float $timeout): Extended to support RWLock in addition to Mutex.

These PHP-level API changes introduce new semantics around lock acquisition timing and may warrant further discussion regarding coroutine compatibility and expected behavior.

int unlock() override;
int trylock_rd() override;
int trylock() override;
int lock_wait(int timeout_msec);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It should be a member method of RWLock not SpinLock

public function locakwait(float $timeout = 1.0): bool {}
public function trylock(): bool {}
public function lock_read(): bool {}
public function lock_read(?float $timeout = null): bool {}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do not modify lock_rd() method, but modify lock_wait(), add an int type parameter, and specify whether the lock type is wrlock or rdlock.

@Appla
Copy link
Contributor Author

Appla commented Sep 28, 2025

updated as suggestions with additional stub definition typo fixed

Copy link

codecov bot commented Sep 28, 2025

Codecov Report

❌ Patch coverage is 0% with 8 lines in your changes missing coverage. Please review.
✅ Project coverage is 86.13%. Comparing base (6184d5c) to head (e9323da).
⚠️ Report is 15 commits behind head on master.

Files with missing lines Patch % Lines
src/lock/rw_lock.cc 0.00% 8 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master    #5866      +/-   ##
==========================================
+ Coverage   86.11%   86.13%   +0.01%     
==========================================
  Files         109      109              
  Lines       16820    16836      +16     
  Branches     2975     2977       +2     
==========================================
+ Hits        14485    14502      +17     
+ Misses       2335     2334       -1     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@matyhtf matyhtf merged commit 1e0100e into swoole:master Sep 28, 2025
8 of 49 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants