unlockActivity in WithoutOverlappingMiddleware not using expiresAfter property

[janwehner]

Hi,

is there a reason that the unlockActivity method in the WithoutOverlappingMiddleware class does not take the $expiresAfter property into account when setting the value for the activity semaphore key?

The compareAndSet method is setting a TTL when passed:

if ($expiresAfter) {
    $this->cache->put($key, $newValue, $expiresAfter);
} else {
    $this->cache->put($key, $newValue);
}

The lock method is calling compareAndSet for the activity semaphore key in line 112 and passing the class property $expiresAfter to it. But when the unlockActivity method is running it will override the activity semaphore key in line 180 without a TTL:

$this->cache->put($this->getActivitySemaphoreKey(), $remaining);

I think this does result in many non-expiring keys in the Redis DB. Since I started testing my workflows in an staging environment the non-expiring keys in the Redis DB grew from 10000 to over 1 million. Nearly all keys are of the activity semaphore type laravel-workflow-overlap:{WORKFLOW_ID}:activity with a value of "a:0:{}".

Would it be possible to use the $expiresAfter property there too or maybe use a fixed and longer TTL?

[janwehner]

Maybe the Signal and Exception classes also have the potential to cause the same problem since they don't pass a TTL to the WithoutOverlappingMiddleware class and the default for $expiresAfter is 0, Redis does not support 0 as valid expiry and treats it as non-expiring unless the key is overwritten with a TTL.

[rmcdaniel]

Fixed in 1.0.62. Please feel to reopen this or create a new issue if you find anything else. Thank you for your insightful comments!