fix: keep write locks alive while request data is flowing - #2217
Conversation
Sorry, something went wrong.
Renew expiring write locks whenever incoming representation data is read. If a held lock expires mid-write, restore the stream hook and destroy the stream so the source cannot continue writing without protection.
|
I confirm this is ready for review @joachimvh - I have already reviewed and applied my fixes jeswr#55 |
Sorry, something went wrong.
There was a problem hiding this comment.
Pull request overview
This PR extends LockingResourceStore’s expiring-lock renewal behavior to write operations by keeping write locks alive while incoming request body streams are being read, and aborting the incoming stream if the write lock expires mid-write (addressing #2215).
Changes:
- Add lockedRepresentationWrite to wrap incoming representation streams so each read() call renews the write lock, and destroy the stream on lock-expiry mid-write.
- Route addResource, setRepresentation, and modifyResource through the new write-lock streaming helper.
- Add/extend unit and integration tests to cover lock renewal on reads and stream destruction on write-timeouts.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| src/storage/LockingResourceStore.ts | Introduces a write-path stream adaptation to renew write locks on read() and destroy the incoming stream if the lock expires mid-write. |
| test/unit/storage/LockingResourceStore.test.ts | Adds unit tests verifying lock renewal per read(), restoration of the original read, and stream destruction behavior on lock expiry vs. other errors. |
| test/integration/LockingResourceStore.test.ts | Adds integration coverage ensuring incoming streams are destroyed on write-timeouts (no reads / long pauses). |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Sorry, something went wrong.
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
|
This has been reviewed by me and is ready for maintainer review |
Sorry, something went wrong.
📁 Related issues
Closes #2215.
✍️ Description
LockingResourceStore already renews expiring locks while response streams are read. This change applies the same
protection to incoming representations used by addResource, setRepresentation, and modifyResource.
The write helper adapts the existing stream in place so representation and patch-specific properties are preserved.
Every read renews the held write lock. If that lock expires before the source call completes, the helper first restores
the exact original read function and then destroys the stream, preventing the source store from continuing an
unprotected write.
Lock-acquisition failures and source write errors do not destroy the request stream. The original read function is
restored after both successful and failed writes.
Validation:
✅ PR check list
Before this pull request can be merged, a core maintainer will check whether