') + ')', '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); } })(); })(); refactor(locks): delegate cross-platform file locking to portalocker by KylinMountain · Pull Request #100 · VectifyAI/OpenKB · GitHub
Skip to content

refactor(locks): delegate cross-platform file locking to portalocker - #100

Merged
KylinMountain merged 3 commits into
mainfrom
fix/locks-portalocker
Jun 15, 2026
Merged

refactor(locks): delegate cross-platform file locking to portalocker#100
KylinMountain merged 3 commits into
mainfrom
fix/locks-portalocker

Conversation

@KylinMountain

@KylinMountainKylinMountain commented Jun 15, 2026•
edited
Loading

Copy link
Copy Markdown
Collaborator

Summary

Replace the hand-rolled fcntl/msvcrtflock/funlock added in #99 with portalocker — fcntl-backed on POSIX, msvcrt/Win32-backed on Windows, with maintained, cross-platform-tested behaviour.

Motivation: the #99 Windows fallback was hand-written msvcrt logic (retry loop, timeout, byte-range locking) that could not be exercised on POSIX, so its Windows behaviour was unverified. Delegating to a vetted library moves the untestable platform path to code its maintainers test on Windows.

Changes

  • flock/funlock now call portalocker.lock / portalocker.unlock.
  • Drop the guarded import fcntl, the msvcrt fallback, and _WINDOWS_LOCK_TIMEOUT / retry loop.
  • Keep the os.fchmod guard and the Windows directory-fsync skip (atomic writes — not covered by portalocker).
  • Pin portalocker==3.2.0 (BSD-3) to match the exact-pin dependency policy. portalocker declares pywin32 under sys_platform == 'win32', so it is installed automatically on Windows (no transitive dep on POSIX).

Windows behaviour (verified against portalocker 3.2.0 source)

  • Shared (reader) locks ARE honoured on Windows — portalocker uses the Win32 LockFileEx API (via the auto-installed pywin32) for shared locks, so concurrent readers work. (Correcting an earlier draft of this PR that wrongly said shared locks need a manual pywin32 add.)
  • Exclusive locks on Windows use msvcrt.locking, which retries ~10s then raises rather than blocking indefinitely. A concurrent exclusive waiter contending with a long-held lock fails fast (LockException) instead of queueing — acceptable for OpenKB's usage; documented in the flock docstring.
  • Lock-acquire failures raise portalocker.LockException (notOSError).

Testing

Supersedes the hand-rolled approach in #99. Refs #93.

@KylinMountain
refactor(locks): delegate file locking to portalocker
8cba8c1
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.
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): review fixes — accurate docs, fcntl import guard, test ha…
20420b3
…rdening
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.
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
test(locks): resolve code-quality bot nits — single openkb import style
bb27f97
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 merged commit 293783b into mainJun 15, 2026
1 check passed
@KylinMountain
KylinMountain deleted the fix/locks-portalocker 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