Skip to content

fix(libkernel): return EWOULDBLOCK from a contended LOCK_NB flock - #1764

Merged
oneandonlydean merged 2 commits into
boykopovar:mainfrom
Zurisen:fix/flock-nonblocking-contention
Oct 10, 2026
Merged

oneandonlydean merged 2 commits into
boykopovar:mainfrom
Zurisen:fix/flock-nonblocking-contention

Conversation

@Zurisen

@Zurisen Zurisen commented Oct 8, 2026 •

Copy link
Copy Markdown
Contributor

What

Fixes #1763. flock_nid_postfix returns -1 with guest errno 35 (EWOULDBLOCK) when LOCK_NB is set and the host reports the lock as held: ERROR_LOCK_VIOLATION on Windows, EWOULDBLOCK on Linux. This follows FreeBSD flock(2). Other failures still throw. As on FreeBSD, a failed LOCK_NB conversion leaves the descriptor without a lock.

Builds on #1755 (merged): on Windows its per-descriptor lock state is what makes a contended request fail with ERROR_LOCK_VIOLATION and not with a stacked-lock error.

Tested

Windows 11 (10.0.26200), MinGW-w64 GCC 15.2.0 (WinLibs r7), Release, RTX 4070 Laptop:

  • guest_filesystem gains contention cases on two descriptors of one file: LOCK_EX | LOCK_NB and LOCK_SH | LOCK_NB against an exclusive lock, LOCK_EX | LOCK_NB against a shared lock (all -1 with errno 35), shared locks on both descriptors, an exclusive lock once the other descriptor unlocks, and LOCK_SH | LOCK_EX | LOCK_NB | 0x10 taking an exclusive lock (decoded as FreeBSD does in fix(libkernel): give flock BSD semantics on Windows #1755), so a shared LOCK_NB request from the other descriptor fails with errno 35.
  • Without the change the first contended request aborts (error=33); with it the cases pass.
  • Full ctest with ANYPS5_REQUIRE_VULKAN=1 --repeat until-fail:3: 610/611 passed on 6635c7ab. libs target builds. The failure, pr_overlap_encoding, also fails on main locally: it patches subprocess._text_encoding, which Python 3.10 does not have.
  • Linux: nauroth ran guest_filesystem on Ubuntu 24.04 (comment below) before the combined-operation case was added.

Checklist

  • Based on current main; no other open PR implements the same functions
  • One topic per PR; follow-ups go in a new PR
  • No comments in code except technical debt
  • Unimplemented paths throw (NotImplemented_nid_no_patch); silent stubs are listed in TechnicalDebt
  • No notes, investigation .md files or images added to the repository (attach them to this PR)
  • New third-party code is a submodule built from source
  • Generic behaviour, not specific to one title
  • Depends on: none (fix(libkernel): give flock BSD semantics on Windows #1755 is merged)
  • AI-assisted: yes

@MarcZX10

MarcZX10 commented Oct 8, 2026 •

Copy link
Copy Markdown
Contributor

Current-main check: this branch is 21 commits behind main 0447793. CONTRIBUTING asks PR branches to stay based on current main; please rebase the dependent change while preserving Depends on #1755. No checks are currently reported on this head, so the rebase should also give us a check result for this PR.

@MarcZX10

MarcZX10 commented Oct 8, 2026

Copy link
Copy Markdown
Contributor

Update: main has since advanced from 0447793 through 364a56b to aa258760 (#1826). This dependent branch remains based on an older main; when rebasing, please keep Depends on #1755. The 364a56b baseline Build was cancelled before Linux CTest, and a new baseline run is in progress on aa258760.

@MarcZX10

MarcZX10 commented Oct 8, 2026

Copy link
Copy Markdown
Contributor

Current-base update: this dependent branch is 30 commits behind main 94b1eecb (merge base 4ddbf049). The latest-main Build is green, including the buffer-format test; please rebase while preserving Depends on #1755 and rerun checks on the final head.

@github-actions github-actions Bot mentioned this pull request Oct 8, 2026
9 tasks done
@Zurisen
Zurisen force-pushed the fix/flock-nonblocking-contention branch from 7954f31 to 28f74c1 Compare October 9, 2026 06:38
@nauroth

nauroth commented Oct 9, 2026

Copy link
Copy Markdown

Linux run, since the description says it wasn't run there: Ubuntu 24.04, GCC 13.3.0, glibc 2.39, Release. The two commits apply as one clean diff onto current main (d70b899), so the rebase should be painless, and guest_filesystem passes 3/3 with --repeat until-fail:3, including the new contention block (EWOULDBLOCK = 35 on both the exclusive and shared non-blocking requests while the other descriptor holds the lock).

I also compared the Windows tracking with FreeBSD's sys_flock: LOCK_UN with nothing held succeeding, and an upgrade or downgrade being a release followed by a new acquire (with the documented window in between), both match. One difference that doesn't matter in practice: for an operation with none of LOCK_SH|LOCK_EX|LOCK_UN, FreeBSD returns EBADF and Linux EINVAL; File::Flock picks EINVAL, but flock_nid_postfix throws on anything except the LOCK_NB contention anyway, so the two hosts stay consistent.

@Zurisen

Zurisen commented Oct 9, 2026

Copy link
Copy Markdown
Contributor Author

@nauroth Thanks for the Linux run and the sys_flock comparison. The EBADF/EINVAL difference is settled in #1755 (e9f2825): flock_nid_postfix now decodes the operation as sys_flock does, so an operation without a lock type fails with EBADF on both hosts. This branch adds a contention case for the decoding: LOCK_SH | LOCK_EX | LOCK_NB | 0x10 takes an exclusive lock, and a shared LOCK_NB request from the other descriptor fails with 35.

@MarcZX100 Rebased onto d70b8998 on top of the updated #1755, keeping Depends on #1755; 506/507 pass locally on Windows (pr_overlap_encoding also fails on main with Python 3.10).

@Rohan03x

Copy link
Copy Markdown
Contributor

Tested on Windows with Palworld (PPSA23126, v01.102.999). Windows 11 22621, GCC 15.2.0 UCRT Release, this PR (a61c879) merged onto main 1f0c9fe plus local unimplemented-export declarations the title needs to start.

Before (main): start-up aborts in sentry-native's lock handling: a second open of /temp0/.sentry-native/<uuid>.run.lock, then flock_nid_postfix: flock failed, fd=5, error=33.

After: no flock error; start-up continues (engine threads, GPU init, settings file written) and stops later on unrelated problems. ctest: 0 failed of 559, 3 skipped, same as main.

Limits: one title; that the call uses LOCK_NB is inferred from error 33, not observed. AI-assisted: yes.

@github-actions github-actions Bot added the needs rebase Conflicts with main, rebase needed label Oct 10, 2026
@Zurisen
Zurisen force-pushed the fix/flock-nonblocking-contention branch from a61c879 to dc84f28 Compare October 10, 2026 04:58
@Zurisen

Zurisen commented Oct 10, 2026

Copy link
Copy Markdown
Contributor Author

Rebased onto bc40a6b5 on top of the updated #1755, keeping Depends on #1755. Full suite locally on Windows: 605/606; the failure is pr_overlap_encoding, which also fails on main with Python 3.10.

@github-actions github-actions Bot removed the needs rebase Conflicts with main, rebase needed label Oct 10, 2026
flock_nid_postfix threw on every failure, so a LOCK_NB request on a lock
held through another descriptor ended the process. FreeBSD flock(2)
returns -1 with EWOULDBLOCK there. Return guest errno 35 when LOCK_NB is
set and the host reports the lock as held: ERROR_LOCK_VIOLATION on
Windows, EWOULDBLOCK on Linux. Other failures still throw.
@Zurisen
Zurisen force-pushed the fix/flock-nonblocking-contention branch from dc84f28 to 6d7d4b0 Compare October 10, 2026 06:00
@Zurisen

Zurisen commented Oct 10, 2026

Copy link
Copy Markdown
Contributor Author

#1755 is merged, so this is rebased onto 6635c7ab and now holds only its own two commits. Full suite locally on Windows: 610/611; the failure is pr_overlap_encoding, which also fails on main with Python 3.10.

@oneandonlydean
oneandonlydean merged commit 01dc5c1 into boykopovar:main Oct 10, 2026
12 of 13 checks passed
SleglJan pushed a commit to SleglJan/AnyPS5 that referenced this pull request Oct 10, 2026
…-contention

fix(libkernel): return EWOULDBLOCK from a contended LOCK_NB flock
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

flock: a contended LOCK_NB request aborts the process instead of failing with EWOULDBLOCK

5 participants