fix(sqlite): write the Windows lock byte in binary; drop source-text asserts
On native Windows the text-mode ``handle.write("\n")`` that seeds an empty
``--lock-file`` is translated to ``\r\n``, so a fresh lock file is two bytes
and the msvcrt regression test fails on the platform it protects. Seed the
file with ``os.write(fd, b"\0")`` on the descriptor instead: exactly one byte
whatever the text-mode newline translation. The Windows-branch tests now run
under a ``\n`` -> ``\r\n`` translating ``open`` so this failure reproduces
on any host.
Remove the two ``... not in src`` assertions added for the previous review
from test_issue2628: the behaviours they hinted at (every missing-index
projection uses the pre-aggregated ordering; listing never creates an
index) are already asserted through query plans, result equivalence and
``PRAGMA index_list`` in test_pr7445_readonly_review_followups.py,
test_state_db_readonly_readers.py and test_issue3887_index_prime.py.
This commit is contained in:
1 parent
70146b74d9
commit
f04e3ca8b2
3 files changed
+24
-10
No files matched your search
@@ -67,9 +67,11 @@ def _exclusive_lock(lock_file):
|
||||
# Lock one byte at offset 0 without blocking; raises OSError when held.
|
||||
# ``a+`` may have just created an empty file, and Windows cannot lock
|
||||
# a range beyond EOF, so make sure byte 0 exists first. An existing
|
||||
# (deployment-owned) lock file is left untouched.
|
||||
# (deployment-owned) lock file is left untouched. Write the byte on
|
||||
# the descriptor: a text-mode ``handle.write("\n")`` becomes two
|
||||
# bytes (``\r\n``) on native Windows.
|
||||
if os.fstat(handle.fileno()).st_size == 0:
|
||||
handle.write("\n")
|
||||
os.write(handle.fileno(), b"\0")
|
||||
handle.flush()
|
||||
handle.seek(0)
|
||||
msvcrt.locking(handle.fileno(), msvcrt.LK_NBLCK, 1)
|
||||
|
||||
@@ -209,12 +209,8 @@ def test_importable_agent_rows_push_sidebar_limit_into_sql(tmp_path):
|
||||
assert "JOIN candidates c ON c.id = s.id" in src
|
||||
assert "latest_messages AS" in src
|
||||
assert "LEFT JOIN latest_messages lm ON lm.session_id = s.id" in src
|
||||
# Pre-aggregation covers every missing-index projection, not only cron (#7445 review).
|
||||
assert 'included == ("cron",)' not in src
|
||||
assert "not messages_index_present" in src
|
||||
assert "PRAGMA index_list(messages)" in src
|
||||
# Listing is a pure read: index creation lives in the drained maintenance tool.
|
||||
assert "CREATE INDEX IF NOT EXISTS idx_messages_session" not in src
|
||||
assert "_CRON_PREAGGREGATE_CANDIDATE_ORDER_MIN_MESSAGES" not in src
|
||||
assert "MAX(mx.timestamp) FROM messages mx WHERE mx.session_id = s.id" in src
|
||||
assert "candidate_limit = max(result_limit * 8, result_limit)" in src
|
||||
|
||||
@@ -18,7 +18,9 @@ Contract under test:
|
||||
there with a clear error instead of an ``ImportError`` at import time;
|
||||
* on the ``msvcrt`` branch the tool locks byte 0 of the lock file only after
|
||||
making sure that byte exists, so a fresh (empty) ``--lock-file`` is acquired
|
||||
without relying on the CRT's beyond-EOF locking behaviour.
|
||||
without relying on the CRT's beyond-EOF locking behaviour; that byte is
|
||||
written as exactly one byte even under the ``\\n`` → ``\\r\\n`` translation
|
||||
native Windows applies to text-mode handles.
|
||||
"""
|
||||
import importlib
|
||||
import os
|
||||
@@ -291,7 +293,20 @@ class _StrictMsvcrt:
|
||||
raise OSError(36, "Resource deadlock avoided: lock region beyond end of file")
|
||||
|
||||
|
||||
def test_windows_lock_branch_acquires_a_fresh_empty_lock_file(tmp_path, maintenance_without_fcntl, monkeypatch):
|
||||
@pytest.fixture
|
||||
def windows_text_mode(maintenance_without_fcntl, monkeypatch):
|
||||
"""Make the tool's text-mode ``open`` translate ``\\n`` to ``\\r\\n`` as on native Windows."""
|
||||
real_open = open
|
||||
|
||||
def crlf_text_open(file, mode="r", *args, **kwargs):
|
||||
if "b" not in mode:
|
||||
kwargs.setdefault("newline", "\r\n")
|
||||
return real_open(file, mode, *args, **kwargs)
|
||||
|
||||
monkeypatch.setattr(maintenance_without_fcntl, "open", crlf_text_open, raising=False)
|
||||
|
||||
|
||||
def test_windows_lock_branch_acquires_a_fresh_empty_lock_file(tmp_path, maintenance_without_fcntl, windows_text_mode, monkeypatch):
|
||||
module = maintenance_without_fcntl
|
||||
assert module.fcntl is None # the fixture already routes the tool to the msvcrt branch
|
||||
fake = _StrictMsvcrt()
|
||||
@@ -305,7 +320,8 @@ def test_windows_lock_branch_acquires_a_fresh_empty_lock_file(tmp_path, maintena
|
||||
result = module.ensure_read_indexes(path, confirmed_drained=True, lock_file=lock)
|
||||
|
||||
assert set(result.values()) == {"created"}
|
||||
# Exactly one non-blocking lock on byte 0, taken while that byte existed.
|
||||
# Exactly one non-blocking lock on byte 0, taken while that byte existed
|
||||
# and was the only one: no newline translation may inflate the file.
|
||||
assert fake.calls == [(fake.LK_NBLCK, 0, 1, 1)]
|
||||
assert lock.stat().st_size == 1
|
||||
with closing(sqlite3.connect(str(path))) as conn:
|
||||
@@ -329,7 +345,7 @@ def test_windows_lock_branch_keeps_an_existing_lock_file_intact(tmp_path, mainte
|
||||
assert lock.read_bytes() == b"deployment-owned\n"
|
||||
|
||||
|
||||
def test_windows_lock_branch_reports_a_held_lock_without_touching_the_db(tmp_path, maintenance_without_fcntl, monkeypatch):
|
||||
def test_windows_lock_branch_reports_a_held_lock_without_touching_the_db(tmp_path, maintenance_without_fcntl, windows_text_mode, monkeypatch):
|
||||
module = maintenance_without_fcntl
|
||||
|
||||
class _HeldMsvcrt(_StrictMsvcrt):
|
||||
|
||||
Reference in new issue
Block a user