Breaking: reject lost-lock renewal and incomplete lock acquisition

.agents/skills/foundatio/SKILL.md

@@ -316,6 +316,7 @@ public class OrderServiceTests : TestLoggerBase
 - **Failure semantics depend on job type**: `JobResult` only has `IsSuccess` -- there is no separate "failed but don't retry" status. For **queue-processed jobs** (`QueueJobBase<T>.ProcessQueueEntryAsync`, or setting `context.Result` in a `WorkItemJob` handler), a non-success result triggers `AbandonAsync`, which re-queues the entry and eventually dead-letters it after `Retries` is exhausted -- reserve `FailedWithMessage`/`FromException` for transient errors you want retried, and log + return `JobResult.Success`/`SuccessWithMessage(...)` for permanent errors to avoid a pointless retry loop. For **standalone/manual jobs** (`JobBase`, a one-off `RunAsync()`/`RunInConsoleAsync()` run, or scheduled/cron jobs via `Foundatio.Extensions.Hosting`), there is no built-in retry or dead letter queue -- a failed result just produces an error-level log, a non-zero exit code from `RunInConsoleAsync`, or a failed entry in the job run history. Returning `FailedWithMessage`/`FromException` there is correct even for permanent errors, since nothing inside Foundatio will retry it.
 - **JobWithLockBase vs manual locking**: Use `JobWithLockBase` when the entire run must be single-instance (leader election). Use manual `ILockProvider.AcquireAsync` inside `JobBase` for finer-grained locking within a job.
 - **JobContext.RenewLockAsync**: Call in long-running jobs (both `JobBase` and `QueueJobBase`) to prevent lock expiration mid-processing.
+- **Cache-backed renewal failure means ownership is unconfirmed**: `CacheLockProvider.RenewAsync` throws `LockException` when conditional renewal fails. After a pause, only a live lease with the same owner can renew; an expired handle requires a new acquisition and revalidation of the work. Stop cooperative work and observe background renewal failures; with `AutoRenewLockOnProgress`, `WorkItemJob` cancels `WorkItemContext.CancellationToken` when the work item lock is lost and abandons the entry instead of completing it, even if the handler returns normally (other renewal errors, including queue entry renewal, are logged). Durations below 5ms throw before mutation. Throttling and empty locks have no-op renewal. Ownership checks are not destination-enforced fencing. Multi-resource acquisition is all-or-nothing even for suffix-overlapping names; successful renewals reset their per-lock interval. After slow renewals, every acquired lease must still fit its conservative lifetime; an elapsed lifetime returns null and releases the partial set (omitted durations use the schedule's one-minute assumption). Lock-set operations attempt every member; `LockException` takes precedence over provider errors (which are logged), and partial-set cleanup failures are logged so callers get `null` or the original error.
 - **In-memory expired entries**: Writes treat an expired entry as missing before maintenance sweeps it (as Redis does): increments and `SetIfHigher`/`SetIfLower` start fresh, `ReplaceAsync`, `SetExpirationAsync`, `RemoveIfEqualAsync` and `ReplaceIfEqualAsync` miss. The write reclaims the expired entry and raises `ItemExpired` once. Successful live conditional removal and empty-list removal free memory without that event. Size-rejected updates preserve value, TTL, and tracked size.
 - **In-memory numeric TTLs**: `SetIfHigherAsync` and `SetIfLowerAsync` apply the requested expiration even when the numeric value is unchanged. A zero return value does not mean the TTL was preserved; `null` clears it. Size-rejected updates remain unchanged.
 - **In-memory lists and cloning**: List writes preserve supported dictionary types and comparers; copying an unsupported `IDictionary` throws `NotSupportedException` without changing the entry. Batch large updates because cloning, custom sizing, and raw dictionary access can require whole-list copies. `Items` honors `CloneValues` without affecting LRU. Collection snapshots do not isolate shared mutable elements when cloning is disabled. See the [caching guide](https://foundatio.readthedocs.io/guide/caching.html#in-memory-update-behavior) for details.

docs/guide/jobs.md

@@ -70,7 +70,7 @@ public class CleanupJob : JobBase
 |--------|-------------|
 | `CancellationToken` | Signals that the job should stop gracefully |
 | `Lock` | The distributed lock held by the job (`null` unless using `JobWithLockBase`) |
-| `RenewLockAsync()` | Extends the lock lease — call this in long-running loops to prevent expiration. In `QueueEntryContext`, also renews the queue entry's visibility timeout so the message isn't redelivered to another consumer. |
+| `RenewLockAsync()` | Extends the lock lease — call this in long-running loops to prevent expiration. In `QueueEntryContext`, also renews the queue entry's visibility timeout so the message isn't redelivered to another consumer. With a cache-backed lock, it throws `LockException` if the lock was lost; let it propagate (or stop the work) because another process may now own it. |
 
 ```csharp
 protected override async Task<JobResult> RunInternalAsync(JobContext context)
@@ -441,10 +441,10 @@ public record DeleteEntityWorkItem
 | `Data` | The raw work item payload (use `GetData<T>()` instead) |
 | `JobId` | Unique identifier for this job run |
 | `WorkItemLock` | Optional distributed lock for the work item |
-| `CancellationToken` | Signals that processing should stop |
+| `CancellationToken` | Signals that processing should stop, including when `AutoRenewLockOnProgress` detects that the work item lock was lost. After a lost lock the entry is abandoned and retried, even if the handler returns normally |
 | `Result` | Set to `JobResult.FailedWithMessage(...)` to indicate failure without throwing. As with `QueueJobBase`, a non-success `Result` abandons and retries the entry -- see [Retry vs Permanent Failure](#retry-vs-permanent-failure) |
-| `ReportProgressAsync(progress, message)` | Publishes `WorkItemStatus` updates via `IMessageBus` |
-| `RenewLockAsync()` | Extends the work item lock lease |
+| `ReportProgressAsync(progress, message)` | Publishes `WorkItemStatus` updates via `IMessageBus`. With `AutoRenewLockOnProgress`, it also renews the queue entry and work item lock |
+| `RenewLockAsync()` | Extends the work item lock lease. Throws `LockException` if a cache-backed lock was lost; stop the work |
 
 ### WorkItemHandlers
 

docs/guide/locks.md

@@ -190,10 +190,10 @@ try
 
     await DoWorkAsync();
 }
-catch (LockAcquisitionTimeoutException)
+catch (LockAcquisitionTimeoutException ex)
 {
     // Lock could not be acquired before the timeout elapsed.
-    _logger.LogWarning("Timed out waiting for lock");
+    _logger.LogWarning(ex, "Timed out waiting for lock: {Message}", ex.Message);
 }
 ```
 
@@ -250,36 +250,77 @@ if (lck is not null)
 }
 ```
 
+`CacheLockProvider.RenewAsync` only extends a lock you still hold. If its cache rejects renewal, including when the lock expired, was released, or was taken by another owner, it throws `LockException`. Ownership is no longer assured: stop the protected work, since another process may already be doing it. Custom cache providers must implement atomic conditional replacement and treat expired entries as missing.
+
+After a process pause or network outage, renewal can succeed if the lease is still live and its owner ID matches. Once it expires, the old handle cannot renew it, even if nobody else has acquired the resource. Acquire a new lock and revalidate the work before continuing; acquiring again does not restore uninterrupted ownership. This follows the [conditional renewal rule documented by Redis](https://redis.io/docs/latest/develop/clients/patterns/distributed-locks/#making-the-algorithm-more-reliable-extending-the-lock).
+
+A supplied renewal duration must be at least 5 milliseconds; a shorter duration throws `ArgumentOutOfRangeException` without changing the cache. Omitting the duration uses 20 minutes. Renewal is a no-op for `ThrottlingLockProvider` and `EmptyLock`; these do not provide renewable exclusive leases.
+
+::: danger Breaking changes
+- `CacheLockProvider.RenewAsync` throws `LockException` when renewal fails. Earlier versions returned successfully, and could revive an expired lock. Renewal called from an `async void` method or timer callback that doesn't catch it will crash the process; observe renewal in an awaited task as shown below.
+- `RenewAsync` throws `ArgumentOutOfRangeException` for durations under 5 milliseconds. The cache already enforced this minimum, but `RenewAsync(TimeSpan.Zero)` used to delete the lock before ownership was checked; it now throws and leaves the lock unchanged. A multi-resource acquisition with `timeUntilExpires` under 5 milliseconds can throw the same exception when it renews earlier locks.
+- Multi-resource `AcquireAsync` throws `LockAcquisitionTimeoutException` when an earlier lease is lost while waiting for the remaining resources; `TryAcquireAsync` returns `null`.
+- `ScopedLockProvider.RenewAsync` must be passed the held lock's `ILock.Resource`, which includes the scope, and `ILock.LockId`. Renewing with the unscoped resource name used to succeed silently without renewing anything; it now throws `LockException`.
+- `WorkItemContext.CancellationToken` is a token linked to the job's token, cancelled also when `AutoRenewLockOnProgress` detects a lost work item lock. Compare it by cancellation state, not by identity with the job token.
+:::
+
+A lock is not a fencing token. Losing it cannot cancel work you already sent to another system. Checking ownership before a write leaves a race between the check and the write; only validation enforced by the destination can fence a stale owner. Make operations idempotent where appropriate, but do not treat idempotency as exclusive ownership.
+
 ### Automatic Renewal
 
-For very long operations, set up automatic renewal:
+For very long operations, observe renewal failures and cancel cooperative work before releasing the lock:
 
 ```csharp
-await using var lck = await locker.TryAcquireAsync("my-resource");
+await using var lck = await locker.TryAcquireAsync(
+    "my-resource", timeUntilExpires: TimeSpan.FromMinutes(1));
 if (lck is null) return;
 
 using var cts = new CancellationTokenSource();
-
-// Start renewal task
-var renewTask = Task.Run(async () =>
-{
-    while (!cts.Token.IsCancellationRequested)
-    {
-        await Task.Delay(TimeSpan.FromSeconds(30), cts.Token);
-        await lck.RenewAsync(TimeSpan.FromMinutes(1));
-    }
-});
+var renewTask = RenewPeriodicallyAsync();
 
 try
 {
-    await VeryLongRunningOperationAsync();
+    await VeryLongRunningOperationAsync(cts.Token);
 }
 finally
 {
-    cts.Cancel();
+    await cts.CancelAsync();
+    await renewTask;
+}
+
+async Task RenewPeriodicallyAsync()
+{
+    try
+    {
+        while (true)
+        {
+            await Task.Delay(TimeSpan.FromSeconds(30), cts.Token);
+            await lck.RenewAsync(TimeSpan.FromMinutes(1));
+        }
+    }
+    catch (OperationCanceledException) when (cts.IsCancellationRequested)
+    {
+        // Normal shutdown after work completes or is cancelled.
+    }
+    catch (Exception ex)
+    {
+        _logger.LogWarning(ex, "Lock renewal failed: {Message}", ex.Message);
+        await cts.CancelAsync();
+        throw;
+    }
 }
 ```
 
+The local async function starts immediately and yields at the delay; it does not need `Task.Run`. The work must honor the cancellation token. The `finally` block observes renewal failure and waits for renewal to stop before `await using` releases the lock. An in-flight renewal has no cancellation-token parameter, so shutdown waits for that cache operation; configure provider timeouts.
+
+Choose an interval shorter than the lease duration, leaving time for cache latency and retries. A pause can still let the lease expire before renewal detects the loss, and cancellation cannot revoke an operation already dispatched to another system.
+
+`WorkItemJob` applies this pattern when a handler sets `AutoRenewLockOnProgress`. Each `ReportProgressAsync` call renews the queue entry and the work item lock. Renewal errors are logged and progress reporting continues. If the work item lock is lost (`LockException`), `WorkItemContext.CancellationToken` is cancelled, and the job abandons the queue entry for retry instead of completing it, whether the handler throws, returns early, or swallows the cancellation, and even if the job itself is cancelled afterward. A handler that ignores the token keeps running unprotected until it returns. A failed queue entry renewal is only logged: queues don't report a definite loss, and while the work item lock is held a redelivered entry can't be processed concurrently.
+
+When acquiring multiple resources, success means every requested lock was acquired. If any resource is unavailable, the provider releases the partial set and returns `null`, including when resource names share a suffix. While acquisition is in progress, earlier locks are renewed once their renewal interval elapses. After waiting for renewal, acquisition checks every member's conservative lease lifetime, including the newest lock. If that lifetime elapsed, acquisition returns `null` and releases the partial set. With no explicit duration, this check uses the renewal schedule's conservative one-minute lifetime, which may be shorter than a provider's default. A lost lease (`LockException`) makes acquisition return `null`, and it takes precedence over a provider error when both occur, because the loss is definite; the provider error is logged. Otherwise a provider error is propagated. Either way, release is attempted for every acquired lock. Cleanup failures are logged rather than thrown, so callers still receive `null` or the original error; any lease that could not be released remains until it expires.
+
+Renewing a multi-resource lock renews every member, even if one fails. If a member's lease was lost, `RenewAsync` throws `LockException`, even when another member also hit a provider error (that error is logged). `RenewalCount` increases only when every member renews.
+
 ## Common Patterns
 
 ### Singleton Processing