') + ')', 'gi'); if (regex.test(text)) { found = true; var frag = document.createDocumentFragment(); var parts = text.split(regex); parts.forEach(function(part, i) { if (i % 2 === 0) { frag.appendChild(document.createTextNode(part)); } else { var span = document.createElement('span'); span.className = 'userscript-highlight'; span.textContent = part; frag.appendChild(span); } }); node.parentNode.replaceChild(frag, node); } }); } else if (node.nodeType === 1 && node.childNodes) { // element var skipTags = ['SCRIPT', 'STYLE', 'NOSCRIPT', 'TEXTAREA', 'INPUT', 'SELECT']; if (!skipTags.includes(node.tagName)) { Array.from(node.childNodes).forEach(highlight); } } } highlight(document.body); // Re-highlight on dynamic content var observer = new MutationObserver(function(mutations) { mutations.forEach(function(m) { m.addedNodes.forEach(function(node) { if (node.nodeType === 1 || node.nodeType === 3) highlight(node); }); }); }); observer.observe(document.body, { childList: true, subtree: true }); })(); } } catch(__e) { console.warn('[Userscript:Highlight Search Terms]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + ', 'i'); if (__m === '*' || __re.test(location.href)) { // Strip utm_, fbclid, gclid, etc. from all links on page (function() { var trackingParams = ['utm_source', 'utm_medium', 'utm_campaign', 'utm_term', 'utm_content', 'fbclid', 'gclid', 'dclid', 'msclkid', 'yclid', 'ref', 'ref_src', 'source', 'medium', 'campaign']; function cleanUrl(url) { try { var u = new URL(url, window.location.origin); var changed = false; trackingParams.forEach(function(p) { if (u.searchParams.has(p)) { u.searchParams.delete(p); changed = true; } }); return changed ? u.toString() : url; } catch (e) { return url; } } function cleanLinks() { document.querySelectorAll('a[href]').forEach(function(a) { var clean = cleanUrl(a.href); if (clean !== a.href) a.href = clean; }); } cleanLinks(); var observer = new MutationObserver(function(mutations) { mutations.forEach(function(m) { m.addedNodes.forEach(function(node) { if (node.nodeType === 1) { if (node.tagName === 'A') cleanLinks(); node.querySelectorAll('a[href]').forEach(function(a) { var clean = cleanUrl(a.href); if (clean !== a.href) a.href = clean; }); } }); }); }); observer.observe(document.body, { childList: true, subtree: true }); })(); } } catch(__e) { console.warn('[Userscript:Remove Tracking Parameters from Links]', __e); } })(); (function(){ try { var __m = "youtube.com"; var __re = new RegExp('^' + "youtube\\.com" + ', 'i'); if (__m === '*' || __re.test(location.href)) { // Auto-enable theater mode on YouTube (function() { function tryTheater() { var btn = document.querySelector('button[aria-label="Theater mode"], ytd-player #player button[title="Theater mode"]'); if (btn && !btn.classList.contains('activated')) { btn.click(); } } // Try immediately tryTheater(); // Try after navigation (SPA) var lastUrl = location.href; setInterval(function() { if (location.href !== lastUrl) { lastUrl = location.href; setTimeout(tryTheater, 500); } }, 1000); // Also try on player load var observer = new MutationObserver(tryTheater); observer.observe(document.body, { childList: true, subtree: true }); })(); } } catch(__e) { console.warn('[Userscript:YouTube Theater Mode Default]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + ', 'i'); if (__m === '*' || __re.test(location.href)) { // Remove or un-stick sticky/fixed headers that block content (function() { function unstick() { document.querySelectorAll('header, nav, [role="banner"], .header, .navbar, .sticky, .fixed-top, [style*="position: fixed"], [style*="position:sticky"]').forEach(function(el) { if (el.style.position === 'fixed' || el.style.position === 'sticky' || getComputedStyle(el).position === 'fixed' || getComputedStyle(el).position === 'sticky') { el.style.position = 'static'; el.style.top = 'auto'; el.style.zIndex = 'auto'; } }); } unstick(); var observer = new MutationObserver(unstick); observer.observe(document.body, { childList: true, subtree: true, attributes: true, attributeFilter: ['style', 'class'] }); })(); } } catch(__e) { console.warn('[Userscript:Kill Sticky Headers]', __e); } })(); })(); fix(locks): cross-platform file locking — fix Windows fcntl crash by KylinMountain · Pull Request #99 · VectifyAI/OpenKB · GitHub
Skip to content

fix(locks): cross-platform file locking — fix Windows fcntl crash - #99

Merged
KylinMountain merged 2 commits into
mainfrom
fix/windows-fcntl-compat
Jun 15, 2026
Merged

fix(locks): cross-platform file locking — fix Windows fcntl crash#99
KylinMountain merged 2 commits into
mainfrom
fix/windows-fcntl-compat

Conversation

@KylinMountain

@KylinMountainKylinMountain commented Jun 15, 2026

Copy link
Copy Markdown
Collaborator

Summary

openkb/locks.py and openkb/config.py hard-imported fcntl and called os.fchmod / directory os.fsync unconditionally — all Unix-only — so OpenKB crashed at import time on Windows:

ModuleNotFoundError: No module named 'fcntl'

This is a Windows-only regression from the KB-locks feature (#86). It surfaced in #93 once the Copilot extra_headers fix (#98) let that (Windows) user get past the header error.

Changes

  • locks.flock / locks.funlock — advisory-lock helpers. POSIX uses fcntl.flock; Windows falls back to msvcrt.locking byte-range locks (exclusive-only — shared requests degrade to exclusive, which is safe — and fcntl.flock's blocking acquire is emulated via a non-blocking retry).
  • Guard os.fchmod with hasattr (absent on Windows).
  • Skip the parent-directory fsync on Windows (can't fsync a directory handle; os.replace is already atomic on NTFS).
  • config.py drops its direct fcntl import and uses locks.flock/funlock.

Testing

  • New tests/test_cross_platform_locks.py simulates the no-fcntl (Windows) path on POSIX — a subprocess reproduces the import crash, and a faked msvcrt exercises the fallback dispatch.
  • Full suite: 750 passed locally (macOS).

⚠️ The Windows path is simulated on macOS/Linux — it has not been run on a real Windows host. The msvcrt logic follows the working patch from the #93 reporter; a confirmation on Windows (or Windows CI) would be ideal before relying on it.

Refs #93

@KylinMountain
fix(locks): make file locking cross-platform (Windows support)
10c63bd
openkb/locks.py and openkb/config.py hard-imported fcntl and called
os.fchmod / directory os.fsync unconditionally — all Unix-only — so OpenKB
crashed at import on Windows (ModuleNotFoundError: No module named 'fcntl'),
surfaced in #93 once the Copilot extra_headers fix (#98) unblocked that user.
- locks.flock/funlock: advisory-lock helpers — fcntl on POSIX, msvcrt
byte-range locks on Windows (exclusive-only; shared degrades to exclusive,
fcntl's blocking acquire emulated via non-blocking retry).
- guard os.fchmod with hasattr; skip directory fsync on Windows
(os.replace is already atomic on NTFS).
- config.py drops its direct fcntl import and uses locks.flock/funlock.
Adds tests/test_cross_platform_locks.py: simulates the no-fcntl (Windows)
path on POSIX via subprocess + a faked msvcrt.
Refs #93
@KylinMountainKylinMountain mentioned this pull request Jun 15, 2026
Closed
github-code-quality[bot]
github-code-qualityBot found potential problems Jun 15, 2026
View reviewed changes
Comment threadtests/test_cross_platform_locks.py Fixed
@KylinMountain
fix(locks): harden Windows lock-acquire loop (code review)
743ccef
Addresses review findings on the msvcrt fallback:
- flock's Windows retry loop no longer spins forever: bound the wait by
_WINDOWS_LOCK_TIMEOUT (default 3600s, OPENKB_LOCK_TIMEOUT override) so a
stuck/never-released lock or a non-contention OSError surfaces as an error
instead of an infinite, silent busy-loop. Add exponential backoff (was a
fixed 100ms spin) and a one-time 'still waiting' warning.
- Document that shared locks degrade to exclusive on Windows (msvcrt has no
shared mode), so concurrent in-process readers serialise there.
- Correct the _fsync_directory comment to not conflate NTFS atomicity with
crash durability.
Tests: cover the retry-until-available and raise-after-timeout paths (the
previously-uncovered msvcrt contention logic), runnable on POSIX via a faked
msvcrt. Full suite 752 passed.
@KylinMountain
KylinMountain merged commit 934d246 into mainJun 15, 2026
1 check passed
@KylinMountainKylinMountain mentioned this pull request Jun 15, 2026
Merged
KylinMountain added a commit that referenced this pull request Jun 15, 2026
@KylinMountain
refactor(locks): delegate cross-platform file locking to portalocker (#…
293783b
…100)
* refactor(locks): delegate file locking to portalocker
Replace the hand-rolled fcntl/msvcrt flock/funlock (merged in #99) with
portalocker, which is fcntl-backed on POSIX and msvcrt/Win32-backed on Windows
with maintained, cross-platform-tested behaviour. Removes the hand-written
Windows retry/timeout loop that could not be exercised on POSIX.
- flock/funlock now delegate to portalocker.lock/unlock.
- Drop the guarded 'import fcntl', the msvcrt fallback, and _WINDOWS_LOCK_TIMEOUT.
- Keep the os.fchmod guard and Windows directory-fsync skip (atomic writes,
which portalocker does not cover).
- Pin portalocker==3.2.0 (BSD-3) to match the exact-pin dependency policy.
Note: true shared (reader) locks on Windows would still need pywin32; without
it portalocker uses msvcrt (exclusive). Not added — in-process concurrent KB
reads are rare. Refs #93.
Tests: swap the msvcrt-internals tests for a cross-process exclusion test that
verifies flock takes a real OS lock, plus the retained atomic-write/fsync-skip
guards. Full suite 749 passed.
* fix(locks): review fixes — accurate docs, fcntl import guard, test hardening
Addresses /code-review findings on the portalocker refactor:
- flock docstring corrected: portalocker uses Win32 LockFileEx (pywin32, pulled
in automatically on Windows) for SHARED locks, so concurrent readers ARE
honoured; EXCLUSIVE uses msvcrt (retries ~10s then raises, not an infinite
block); failures raise portalocker.LockException, not OSError.
- Re-add the issue #93 regression guard: assert no openkb module hard-imports
the Unix-only fcntl at module level (replaces the dropped import-without-fcntl
test without depending on portalocker internals).
- Strengthen the cross-process lock test: assert both BLOCKED (while held) and
ACQUIRED (after release), and check the probe's exit code so an ImportError
surfaces clearly instead of an empty-stdout false failure.
- Drop the now-dead 'import pytest' / 'import portalocker' from the test module.
* test(locks): resolve code-quality bot nits — single openkb import style
Drop the unused 'import openkb' and derive the package dir from
locks.__file__ instead, so the test module uses one import style for openkb
(github-code-quality bot). The unused 'import portalocker' was already removed
in the prior commit.
@KylinMountain
KylinMountain deleted the fix/windows-fcntl-compat branch June 22, 2026 09:53
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

1 more reviewer
@github-code-qualitygithub-code-quality[bot]github-code-quality[bot] left review comments
Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

1 participant

@KylinMountain