Going through the source, a few things stood out worth discussing before a PR.
1. Semaphore backed property hooks add no value over asymmetric visibility
$available and $capacity use backed property hooks that do nothing but return the backing store:
public int $available {
get => $this->available;
}
PHP 8.4 public private(set) is the idiomatic way to express this - publicly readable, privately writable - without hook dispatch overhead on every read:
public private(set) int $available;
public private(set) int $capacity;
Mutex::$queueLength is different since its hook computes from $this->queue, so the hook there is justified.
2. Semaphore::release() does not guard against over-release when the queue is non-empty
The capacity check only fires in the no-queue path:
if (\count($this->queue) === 0) {
if ($this->available >= $this->capacity) {
throw new \LogicException(...);
}
// ...
}
// queue non-empty path: available++ with no capacity guard
If a caller releases more permits than they acquired while waiters are queued, $available temporarily exceeds capacity. The exception eventually fires once the queue drains, but the semaphore is in an inconsistent state between those calls. The guard should apply unconditionally before the increment:
if ($this->available >= $this->capacity) {
throw new \LogicException('Cannot release more permits than semaphore capacity');
}
3. Missing final on both concrete classes
Both Mutex and Semaphore are subclassable. Their synchronization guarantees depend entirely on private state ($locked, $queue, $available). A subclass cannot override the queueing logic, so subclassing either class can only produce broken variants. Marking them final closes that door and is consistent with how other internal Hibla classes are treated.
4. tryAcquire() bypasses the waiter queue without documenting the starvation risk
tryAcquire() intentionally skips the queue, which is fine for its use case. But if a high-frequency caller loops on tryAcquire(), waiters in the queue can starve indefinitely. Worth noting in the docblock so callers understand when it is and is not appropriate.
5. No tryAcquireMany() to pair with acquireMany()
The interface has acquire() / tryAcquire() and acquireMany() but no tryAcquireMany(int $permits). If non-blocking multi-permit acquisition is out of scope that is fine, but the asymmetry is noticeable.
Happy to put together a PR for any or all of these if the direction looks right.
Going through the source, a few things stood out worth discussing before a PR.
1. Semaphore backed property hooks add no value over asymmetric visibility
$availableand$capacityuse backed property hooks that do nothing but return the backing store:PHP 8.4
public private(set)is the idiomatic way to express this - publicly readable, privately writable - without hook dispatch overhead on every read:Mutex::$queueLengthis different since its hook computes from$this->queue, so the hook there is justified.2. Semaphore::release() does not guard against over-release when the queue is non-empty
The capacity check only fires in the no-queue path:
If a caller releases more permits than they acquired while waiters are queued,
$availabletemporarily exceeds capacity. The exception eventually fires once the queue drains, but the semaphore is in an inconsistent state between those calls. The guard should apply unconditionally before the increment:3. Missing final on both concrete classes
Both
MutexandSemaphoreare subclassable. Their synchronization guarantees depend entirely on private state ($locked,$queue,$available). A subclass cannot override the queueing logic, so subclassing either class can only produce broken variants. Marking themfinalcloses that door and is consistent with how other internal Hibla classes are treated.4. tryAcquire() bypasses the waiter queue without documenting the starvation risk
tryAcquire()intentionally skips the queue, which is fine for its use case. But if a high-frequency caller loops ontryAcquire(), waiters in the queue can starve indefinitely. Worth noting in the docblock so callers understand when it is and is not appropriate.5. No tryAcquireMany() to pair with acquireMany()
The interface has
acquire()/tryAcquire()andacquireMany()but notryAcquireMany(int $permits). If non-blocking multi-permit acquisition is out of scope that is fine, but the asymmetry is noticeable.Happy to put together a PR for any or all of these if the direction looks right.