Repository navigation
fix(libkernel): make close wait for a positioned transfer on the same descriptor - #5938
Conversation
|
Title run with this change (Cat Quest III, PPSA15065), update to the "not repeated with this change" line in the description. Tree: Result ( For comparison, without this change on AI-assisted (Claude Code). |
|
Some comparisons could not be completed because a pull request conflicts with the base branch. Review these shared files manually before concluding that there is no overlap.
|
|
@adriadam10 a short request for review of this PR. Head AI-assisted (Claude Code). |
… descriptor Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
…ansfer Its callers (NativePositioned_nid_no_patch, the vectored transfer) already hold the non-recursive per-descriptor lock, so locking again deadlocked every positioned transfer. Co-Authored-By: Claude <noreply@anthropic.com>
|
Thanks for the fix. One thing added on top of your commit:
Checked: rebase onto current |
abe6ba8 to
5484500
Compare
What
Cat Quest III (PPSA15065) aborts about 15 s after start with:
(
PPSA15065_rN2/run_175915.errline 10; the bracketed fields come from a local debug build that extends the message, not from this change.) The read itself succeeded (readOk=1) and the restore failed withrestoreError=6(ERROR_INVALID_HANDLE), so the handle was released while the transfer was in flight.On Windows
sceKernelPread/sceKernelPwriterun asSetFilePointerEx+ReadFile/WriteFile+SetFilePointerExback under the per-descriptor positioned lock (File::LockPositionedTransfer).sceKernelCloseandclose_nid_postfixdid not take that lock, so a close on another thread could free the handle in the middle of a transfer.NativePositionedalso fetched the handle before it took the lock, so a transfer that began after a finished close used a stale handle and failed with ESPIPE instead of EBADF.sceKernelCloseandclose_nid_postfixtake the descriptor's positioned lock before they release anything (Windows only), so a transfer in flight finishes and the close follows.NativePositionedtakes the lock before_get_osfhandle, so a transfer after a close sees EBADF.guest_positioned_close(Windows): 60 rounds of a 32 MiBsceKernelPreadin one thread andsceKernelClosefrom the main thread about 300 us later; it requires no exception,close == 0, and the read either complete or EBADF, and at least one round must overlap.Title evidence: a trace run on the same build (
PPSA15065_rN2/run_180943.err, 120 s, no crash) has 539 lines[fd0trace] close fd=0from 7 distinct thread ids (callerapp.exe+0x1401327) and 138[fd0trace] positioned enter fd=0lines (counted with Python); the[fd0trace]lines come from a local trace build, not from this change. So the title closes descriptor 0 repeatedly while it reads from it.Tested
Windows 10 (19045), MinGW-w64 GCC 15.2.0, Release, on
maine34c47921+ this commit:guest_positioned_closefails without the fix in 4 of 4 runs (3 xwhat(): NativePositioned: cannot restore the file offset, 1 x result -2147352547 = ESPIPE,check failed at line 67) and passes with the fix in 5 of 5 runs.python tools/check_conventions.py --base upstream/mainwith this title and body passes.maina453108cfwith a local debug build); the effect on the title is therefore not shown by a run with the fix.Overlaps: #5892 (Cat Quest III, open) refuses to close the standard descriptors (#4909), which removes the fd 0 reuse that produced this crash; with it the title may no longer reach this path. This change is independent of it (it fixes close against a running transfer for any descriptor) and conflicts textually with #5892 in
Open.cpp(merge-tree). #3295 (open) keeps the host standard streams open on close and edits the same close functions; merge-tree conflicts only indocs/dev/TechnicalDebt.md. How the console behaves when a close races a positioned read is not verified; this change assumes the transfer completes first.Checklist
main; overlaps with perf(agc): compare sampled snapshots outside the write watch only after a write-watch mark #5892 and fix(libkernel): keep host standard streams open on guest close #3295 are listed aboveNotImplemented_nid_no_patch); silent stubs are listed in TechnicalDebt.mdfiles or images added to the repositoryAI-assisted (Claude Code).
🤖 Generated with Claude Code