Repository navigation
feat(libkernel): implement fcntl record locks on Windows - #4480
oneandonlydean merged 1 commit into
Conversation
ProsperoRadio (PPSA99001) stops at 33.7 s on Windows, with boykopovar#3649 applied so that it gets past poll, on the throw from boykopovar#3840: what(): fcntl: record locks are not implemented on Windows, command=12, fd=3 F_GETLK, F_SETLK, F_SETLKW and the F_OGETLK, F_OSETLK and F_OSETLKW forms now follow FreeBSD 11.4's kern_fcntl and lf_advlockasync (8c639945): an SEEK_CUR start is resolved against the descriptor offset (EOVERFLOW past OFF_MAX), F_RDLCK needs read access and F_WRLCK write access (EBADF), another type is EINVAL, and the range is checked against the file size for SEEK_END (EINVAL for a negative start or an unknown whence, EOVERFLOW past OFF_MAX). The guest is one process, and lf_blocks never lets locks of one owner conflict, so whichever descriptor or thread asks, F_SETLK and F_SETLKW succeed without waiting and F_GETLK reports F_UNLCK, as on the console. Nothing is held on the host: Windows byte-range locks are mandatory and would fail the guest's own reads and writes through other descriptors. A lock from the end of a directory throws. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
b8e52d2 to
43ffba8
Compare
|
The PR body says the Windows path wasn't run for this carry, so I ran it. Windows 11 (26300), WinLibs GCC 15.2.0, Ninja, Release, Tests
ProsperoRadio (PPSA99001, catalog 01.000.010), 60 s runs with
The title's log redirect doesn't keep the ProsperoTV (PPSA99003) exits after 10 s both with and without this PR. It needs #4525 first, and #4525 doesn't merge together with #3649: both change AI-assisted: yes (Claude Code) |
Dextroid17
left a comment
There was a problem hiding this comment.
Review of #4480 (fcntl record locks on Windows), head 43ffba8
Verdict: approve, relying on LeandroLP23's Windows run for the host path. One non-blocking note.
Scope first: I ran Linux only. On Linux the #else branch uses the host's POSIX locks, so the Windows ProcessRecordLock is not compiled there. What I could check is the range/ordering/errno logic, by copying it into a stub harness.
What I ran (PR head merged onto upstream/main ee49621, clean env):
- guest_kernel_fcntl_tests (Linux build): 10/10 runs exit 0.
- Differential check of the range/ordering logic against my own transcription of FreeBSD fcntl() + lf_advlockasync() (freebsd-src 8c639945a): 2,352,000 cases (F_GETLK/F_SETLK, lock types incl. unknown, whence incl. invalid, start/length at int64 extremes, open mode, file size, file offset >= 0 only), built with UBSan. 0 mismatches. LockRangeError and the ProcessRecordLock checks are copied from the PR with the host calls stubbed. The reference is only as independent as my reading of FreeBSD, and it covers range and error order only, not F_SETLKW blocking, F_UNLCKSYS or the HostWrite write-back.
- Mutation controls: dropping the
length - 1overflow term gives 6,912 mismatches; dropping the F_GETLK unknown-type check gives 241,704; dropping thestart == 0guard gives 0 (when length < 0,start + length < 0already catches it, so the guard is redundant but harmless). git merge-treeof this PR with #4555 (also edits DescriptorControl.cpp InitialEntry) is clean.
From LeandroLP23's comment, not run by me: Windows 554/554, guest_kernel_fcntl 3/3, and the failure message with main's DescriptorControl.cpp restored. In their ProsperoRadio runs the title exits 0xc0000409 both before and after this PR; with the PR the log gets past the database open to the HTTP code.
Not checked: the "SEEK_END on a directory still throws" case (the test asserts it under _WIN32 only).
Note: the start == 0 guard in LockRangeError is redundant when length < 0. Fine to leave.
|
A title that needs this: ProsperoTV (PPSA99003) on Windows 11 (WinLibs GCC 15.2, RTX 3080). On That's F_SETLK on a regular file. With this PR stacked on top (fork |
What
Fixes #4479. Carries @screamm's fix commit from that issue (
screamm:feat/libkernel-record-locks-windows,b7cc2856), keeping the commit and its authorship, as the issue invites. The branch sits on top of #3649's two commits, which this commit does not need (#3649 is only how the titles reach the call); it cherry-picks onto currentmainwithout changes and builds there.On Windows,
fcntlrecord locks (F_GETLK,F_SETLK,F_SETLKWand the oldF_OGETLK/F_OSETLK/F_OSETLKWforms) throwfcntl: record locks are not implemented on Windows, which #3840 recorded as a limit. With #3649 applied, ProsperoRadio (PPSA99001) stops there onF_SETLK, and ProsperoTV (PPSA99003) does too once #4478's fix is applied. This commit applies FreeBSD 11.4's argument checks in FreeBSD's order (kern_fcntl,lf_advlockasync) and then followslf_blocks, where only locks of different owners conflict: every lock belongs to the one guest process, soF_SETLKandF_SETLKWsucceed without waiting andF_GETLKreportsF_UNLCK. Nothing is held on the host, because Windows byte-range locks are mandatory and would fail the guest's own reads and writes through its other descriptors; the fcntl TechnicalDebt entry records that other host processes neither see nor are seen by these locks. ASEEK_ENDlock on a directory and locks on character devices still throw. Linux is unchanged.guest_kernel_fcntlreplaces the old Windows "throws" expectation with the round trip and addsRecordLocksBelongToTheProcess(overlap merge and split, two descriptors on one range, another thread'sF_SETLK/F_SETLKW/F_GETLK,SEEK_CUR/SEEK_END, negative lengths, the 24-byte old struct,EINVAL,EBADFon read-only and write-only descriptors, relock after close); theEOVERFLOW, error-order,F_GETLKwrite-back and directory cases are Windows-only.Tested
guest_kernel_fcntl,guest_poll,guest_dup,guest_file_vector3/3; 620/620 ofctest -j8 -E "^agc_driver_|^video_out_flip"; ProsperoRadio relinked against the build (with feat(libkernel): poll on file descriptors and Windows pipe read ends #3649) gets past thefcntlstop and reaches libkernel: poll throws for POLLPRI on guest sockets under WSAPoll (PSXS5, SwanStation, ProsperoTV after #3649); fix branch ready, PSXS5 renders #4478's.main33ae282e+ feat(libkernel): poll on file descriptors and Windows pipe read ends #3649 + libkernel: poll throws for POLLPRI on guest sockets under WSAPoll (PSXS5, SwanStation, ProsperoTV after #3649); fix branch ready, PSXS5 renders #4478's fix + this commit): ProsperoRadio and ProsperoTV get pastpolland the record locks and stop next on the graphics side (fix(recompiler): read P10 of a flat input as the provoking vertex #4030).main33ae282.guest_kernel_fcntl,guest_poll,guest_dupandguest_file_vectorpass, and all 306guest_*tests pass. The new sharedRecordLocksBelongToTheProcesscases also pass withmain'sDescriptorControl.cpp, as expected since Linux forwards record locks to the host. I could not run the Windows path myself.guest_kernel_fcntl3/3 and fails withmain'sDescriptorControl.cpp(fcntl: record locks are not implemented on Windows, command=12, fd=3);ctest -E '^agc_'554/554; ProsperoRadio with feat(libkernel): poll on file descriptors and Windows pipe read ends #3649 gets past the database's record lock to its HTTP code.maind1fd937 + fix(libkernel): support POLLPRI on Windows sockets #4525 stops atwhat(): fcntl: record locks are not implemented on Windows, command=12, fd=4(F_SETLKon a regular file); with this PR on top it gets past the lock and renders its menu for the full 60 s with no throw.tools/check_conventions.py --base upstream/mainpasses.Checklist
main; no other open PR implements the same functionsNotImplemented_nid_no_patch); silent stubs are listed in TechnicalDebt.mdfiles or images added to the repository (attach them to this PR)🤖 Generated with Claude Code