Skip to content

fix: point SQLite's Sleep at kernel32 on Windows - #1

Open
mrhoribu wants to merge 5 commits into
mainfrom
fix/windows-sleep-gvl
Open

mrhoribu wants to merge 5 commits into
mainfrom
fix/windows-sleep-gvl

Conversation

@mrhoribu

@mrhoribu mrhoribu commented Nov 5, 2026 •

Copy link
Copy Markdown
Owner

Draft PR on the fork. Issue and PR references are in code so this PR doesn't post cross-references on upstream issues; un-wrap them before opening upstream against sparklemotion/sqlite3-ruby.

Fixes #766.

Problem

On Windows, SQLite's own sleeps release the GVL and can deliver interrupts while SQLite holds a connection's mutex. That contradicts Database#busy_timeout's documentation ("holds the GVL") and can deadlock the whole process:

  • Sharing: another Ruby thread uses the same connection during the sleep. It blocks on SQLite's mutex while holding the GVL, and the sleeping thread can never get the GVL back.
  • Interrupts: a Thread#raise, Thread#kill or Timeout lands in the sleep and unwinds through SQLite's C frames, leaving the mutex locked for the next caller.

We hit the first in production: whole-process freezes at 0% CPU in a multi-process app. A full-memory dump showed the GVL owned by a thread in EnterCriticalSection on a connection mutex, and that mutex owned by a thread in rb_w32_Sleep, inside SQLite's winLock retry, waiting for the GVL. The write-up is in #766 and in elanthia-online/lich-5#2254 (https://github.com/elanthia-online/lich-5/issues/2254#issuecomment-6647684262).

Cause

  • SQLite's Windows VFS sleeps through the "Sleep" entry of its system-call table (osSleep) while holding db->mutex:
    • winLock's PENDING-lock retry (osSleep(1) between 3 attempts), which happens whenever another process holds the lock, even with busy_timeout(0);
    • winRetryIoerr;
    • the default busy handler behind busy_timeout(N) (sqliteDefaultBusyCallback → winSleep);
    • winClose's retry.
  • With SQLite linked into the extension, mkmf links Ruby's import library before kernel32. The Ruby DLL exports a Sleep symbol that forwards to rb_w32_Sleep: win32/mkexports.rb turns #define Sleep(msec) rb_w32_Sleep(msec) in include/ruby/win32.h into an export. So that "Sleep" slot resolves to rb_w32_Sleep.
    • Every precompiled Windows gem I checked imports Sleep from the Ruby DLL: 13 builds from 1.3.13 to 2.10.2.
    • Cross-linking with MinGW shows the same for a vendored source build. A build with SQLite as a separate DLL (--enable-system-libraries) gets kernel32's Sleep.
  • rb_w32_Sleep (thread_win32.c) runs the sleep in BLOCKING_REGION, which releases the GVL and, through unblock_function_set, runs RUBY_VM_CHECK_INTS.

Fix

In Init_sqlite3_native, on Windows, after sqlite3_initialize(), look up kernel32's Sleep with GetProcAddress and install it in SQLite's system-call table through xSetSystemCall.

  • Why GetProcAddress: inside the extension, writing Sleep would give you the ruby/win32.h macro and the same Ruby import, so the address has to be looked up at run time.
  • Which VFSes: os_win.c's VFSes (win32, win32-longpath, win32-none, win32-longpath-none) share one table. Each is tried in case some are compiled out.
  • When SQLite is a separate DLL: this is harmless, because that DLL's Sleep already is kernel32's.
  • Guards: #if defined(_WIN32) && SQLITE_VERSION_NUMBER >= 3007006, since xSetSystemCall needs VFS iVersion 3. Nothing changes on other platforms.

Behaviour change

This applies only to builds with SQLite linked into the extension: the precompiled gems and the default build. Builds with --enable-system-libraries (which SQLCipher builds imply) already used kernel32's Sleep. That includes CI's mswin job, but a vendored mswin build would be affected like the default build, and the fix runs there too. On those Windows builds, SQLite's waits now hold the GVL, as documented and as on Linux and macOS. Upgraders will notice:

  • Same-process lock holders can't commit. Other Ruby threads don't run during a busy_timeout wait. If another connection in the same process holds the lock (a connection pool, say), its thread can't run to release it, so the wait ends in BusyException after the full timeout.
  • Interrupts wait. Timeout.timeout, Thread#raise, Thread#kill and Ctrl-C take effect only once the wait ends. A Timeout shorter than the busy timeout ends in BusyException rather than Timeout::Error.
  • VFS retries hold the GVL too. The Windows VFS's own retries on sharing or lock violations (antivirus, indexers) now hold the GVL whatever busy handler is set: up to about 1.4 s per failing I/O call (winRetryIoerr), and up to 200 ms when closing (winClose).
  • busy_handler_timeout= doesn't help shared connections. It lets other threads run during the wait only if they don't use the waiting connection. SQLite runs the Ruby handler while holding the connection's mutex, so a thread that calls into the connection meanwhile hangs the process, on every platform. If threads share a connection, keep busy_timeout, or give each thread its own connection.

These are in the CHANGELOG, in Database#busy_timeout's rdoc (GVL and interrupts), and in Database#busy_handler_timeout='s rdoc (the shared-connection caveat).

Alternatives considered

  1. Runtime override (this PR). At load time, look up kernel32's Sleep and install it with xSetSystemCall. It doesn't depend on link order or toolchain.
    • Cost: xSetSystemCall is documented as a testing aid, and the set of overridable calls may change between SQLite releases. Mitigations: the return codes are checked, and the extension warns at load time if no VFS accepted the override. test/test_sqlite_sleep.rb also fails on the Windows CI jobs if the override stops working, which the tests-only run demonstrated.
  2. Fix the link order. Make sure kernel32 is searched before Ruby's import library for SQLite's objects, for example by appending -lkernel32 right after the vendored libsqlite3.a in extconf.rb. A MinGW cross-link test showed this binds kernel32's Sleep.
    • Cost: it depends on mkmf's link-line details and on each toolchain (rake-compiler-dock, MinGW, mswin). It would also silently regress if the vendored library moves in the link line.
  3. Build SQLite as a separate DLL (as --enable-system-libraries does). Its Sleep comes from kernel32.
    • Cost: it changes how the precompiled gems are packaged (another DLL to ship and locate) for the sake of one import.

Option 1 can also be combined with 2 later, if the maintainers prefer to fix the link and keep the override as a guard.

Tests (test/test_sqlite_sleep.rb)

  1. A busy_timeout wait doesn't let another Ruby thread run (a ticker thread counts at most a tick or two over a 300 ms wait).
  2. A thread using a connection while another thread waits on it doesn't deadlock.
  3. A Thread#raise aimed at a thread in a busy_timeout wait leaves the connection usable from another thread.

Tests 2 and 3 run in a child process with a 30 s timeout, because a deadlock stops every thread of the process. They use spawn rather than fork (unlike assert_connection_usable_from_another_thread), so they run on Windows too.

Checking that the tests catch the bug. The bug can't be reproduced natively on Linux, so I used a small test-only LD_PRELOAD shim. It routes nanosleep calls made from sqlite3_native.so through rb_thread_call_without_gvl, which releases the GVL and checks interrupts, as rb_w32_Sleep does. With it, all three tests fail as they would on Windows:

  • test 1: "another thread ran 59 times during the busy wait";
  • tests 2 and 3: the child deadlocked and was killed after 30 s.

Without the shim, all three pass.

Test 1 is skipped on non-CRuby. TruffleRuby has no GVL, so other threads legitimately run during the wait. The two subprocess tests run everywhere.

Results:

  • Linux (Ruby 4.0.9, vendored SQLite 3.54.1): full suite 431 runs, 0 failures, 1 skip (baseline 428 runs, 0 failures, 1 skip). The new tests pass 5 out of 5. RuboCop clean.
  • Windows, on this repo's own CI workflow, run on my fork. One run had only the test commit (tests only), the other the fix commit on top (tests + fix):
Windows job Tests only Tests + fix
test (windows, 3.2–4.0, disable) (vendored SQLite) 3 failures: the new tests pass
test source (windows, 3.2–4.0, disable) 3 failures pass
x64-mingw-ucrt 3.2–4.0 (native gem tests) 3 failures pass
test (windows, *, enable), mswin (both --enable-system-libraries: SQLite as a separate DLL) pass pass

On the Ruby 4.0 Windows job without the fix, the failures were:

  • test 1: "another thread ran 49 times during the busy wait";
  • tests 2 and 3: the child process deadlocked.

Skip counts are the same in both runs, so the tests ran rather than being skipped. Jobs that link SQLite as a separate DLL pass either way, because that DLL's Sleep comes from kernel32.

This PR's head (5bd933d, CI on this PR): 133 jobs pass, 1 fails. The failure is sqlcipher (windows, mingw), whose MSYS2 package install also fails on main. All Windows, macOS, Linux, TruffleRuby and native-gem jobs pass.

One earlier run (before the TruffleRuby skip) also had a single failure of the existing test_busy_timeout wall-clock check (0.996 >= 1.0) on x86_64-linux-gnu 4.0. It passed on this head.

Notes

  • The Ruby DLL also exports a forwarding getpid (rb_w32_getpid), which the extension imports too. It's harmless and left alone.
  • The same link-order effect makes MinGW's DLL startup code bind its Sleep to the Ruby export as well. That's outside SQLite and not touched here.
  • Before opening upstream: both CHANGELOG entries cite only the issue. House style also cites the PR (#766, #NNN), so add the upstream PR number once it exists.

Database#busy_timeout documents that its wait holds the GVL. SQLite sleeps
while holding the connection's mutex, so a sleep that released the GVL
would let another thread enter the connection and block on that mutex
with the GVL held, and a sleep that checked for interrupts would let a
Thread#raise unwind through SQLite and leave the mutex locked.

- A busy_timeout wait doesn't let another thread run.
- A thread using a connection while another thread waits on it doesn't
  deadlock (in a child process, since a deadlock stops every thread).
- A Thread#raise aimed at a waiting thread leaves the connection usable
  from another thread (also in a child process).

The child processes use spawn, not fork, so these also run on Windows,
where SQLite's Sleep resolves to the Ruby DLL's rb_w32_Sleep (upstream issue 766).

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
SQLite's Windows VFS sleeps through the "Sleep" entry of its system-call
table (winLock's lock retries, winRetryIoerr, and the default busy
handler behind Database#busy_timeout), while holding the connection's
mutex. Linked into this extension, that "Sleep" resolves against the
Ruby DLL, which exports a "Sleep" forwarding to rb_w32_Sleep
(win32/mkexports.rb turns the "#define Sleep" in include/ruby/win32.h
into an export). rb_w32_Sleep releases the GVL and checks for
interrupts, so another Ruby thread could enter the connection and block
on its mutex with the GVL held, or a Thread#raise could unwind through
SQLite and leave the mutex locked. Either deadlocks the process.

At load time, look up kernel32's Sleep with GetProcAddress (naming Sleep
in the source would get the ruby/win32.h macro and the same import) and
install it with each Windows VFS's xSetSystemCall. Harmless when SQLite
is a separate DLL, whose Sleep already comes from kernel32.

Fixes upstream issue 766

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

@MahtraDR MahtraDR left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

PR Review: fix: point SQLite's Sleep at kernel32 on Windows

Reviewer: Claude · 5bd933d · 2026-11-06

The fix itself holds up, and I couldn't break it. The shipped 2.10.2 x64-mingw-ucrt gem really does import Sleep from x64-ucrt-ruby*.dll for all four Ruby versions. Every sleep in SQLite 3.54.1's Windows VFS goes through the one aSyscall "Sleep" slot this replaces: winLock, winRetryIoerr, winClose, and winSleep (which the busy handler and WAL retries call). Your tests-only and with-fix CI runs show exactly what the description says.

I also checked the tests a second way. On macOS I interposed SQLite's nanosleep with a shim that releases the GVL and checks interrupts, the way rb_w32_Sleep does. With the shim, all three tests fail every time (20 out of 20). Test 2 catches the GVL release and test 3 catches the interrupt. Without it they pass every time, and test 1 counted 0 ticks even under heavy CPU load and GC stress.

Everything left to fix is in the text that ships with the PR.

1. The CHANGELOG points shared-connection users at a method that deadlocks for them (CHANGELOG.md:15). The new "Changed" line says to use busy_handler_timeout= "for lock waits that let other threads run". That method installs a Ruby busy handler, and SQLite runs it while holding the connection's mutex. If another thread uses the same connection during the wait, it blocks on that mutex while holding the GVL, and the process hangs. That is the deadlock this PR fixes, on every platform.

I reproduced it 3 out of 3 times on macOS, against a busy_timeout control that passes every time. A handler with no sleep at all hangs too, so removing the sleep(0.001) wouldn't help. README says sharing a Database between threads is safe, and lich-5 does exactly that. Please qualify the sentence. For example: other threads can run during the wait only if they don't use the waiting connection. If threads share a connection, keep busy_timeout, or give each thread its own connection.

2. The "Changed" entry understates what Windows users will notice (CHANGELOG.md:15). It says other threads don't run during the wait. That's accurate, but it leaves out three consequences people will hit after upgrading:

  • With two connections in one process (Sequel's pool, which calls busy_timeout(5000) by default, or lich-5), the thread holding the lock can't run to commit. So the wait now always ends in BusyException after the full timeout. On macOS: 2.16 s BusyException with busy_timeout, against success in 0.21 s with busy_handler_timeout=.
  • Timeout.timeout, Thread#raise/#kill and Ctrl-C now take effect only when the wait ends. A Timeout shorter than the busy timeout returns BusyException instead of Timeout::Error. On macOS: 2.10 s against 0.20 s.
  • The Windows VFS's own retries now hold the GVL too, and busy_handler_timeout= doesn't change them. On sharing or lock violations (antivirus, indexers), winRetryIoerr sleeps up to about 1.4 s per failing call, and winClose up to 200 ms.

The change itself is intended and matches Linux and macOS. I'm only asking for a sentence or two so upgraders know what to expect, or a note from you if you'd rather not.

3. Scope (CHANGELOG.md:7 and :15). Both entries say "On Windows". Your own comment in sqlite3.c and the CI table show that only builds with SQLite linked into the extension change, which means the precompiled gems and the default build. --enable-system-libraries, mswin and SQLCipher builds already used kernel32's Sleep. Something like "On Windows, when SQLite is linked into the extension (the precompiled gems and the default build)" would cover it.

4. "As documented" (test/test_sqlite_sleep.rb:9). The test header says Database#busy_timeout documents that its sleeps don't release the GVL or check for interrupts. Its rdoc (ext/sqlite3/database.c:779) says neither. Only the busy_handler_timeout= rdoc mentions the GVL, and no doc mentions interrupts. Adding one sentence to the busy_timeout rdoc about the GVL and interrupts would make both "as documented" claims true, and would give item 2 a lasting home.

5. C comment (ext/sqlite3/sqlite3.c:219). The reason given for GetProcAddress ("Sleep is the macro from ruby/win32.h") doesn't hold. The macro is function-like, so a bare Sleep isn't expanded. The real reason is the second half of the sentence: a direct reference would bind to the same Ruby-DLL import. The list of sleep sites at :209 also leaves out winClose's retry and the WAL read retry. Both are covered by the fix.

6. The test-only hook (ext/sqlite3/sqlite3.c:243). sqlite3.h describes xSetSystemCall as a testing aid whose overridable calls can change from one release to the next, and the return code is ignored here. Your Windows CI test would catch a renamed slot, so the risk is small. But a user-built library outside CI would lose the fix silently, and maintainers may ask why not fix the link order instead. I'd suggest three things:

  • make "no VFS accepted it" visible, at least in tests or debug builds;
  • say in the comment that the hook is test-only and which CI test guards it;
  • copy the alternatives discussion from sparklemotion#766 into the PR description.

Before you open it upstream: both CHANGELOG entries cite only issue sparklemotion#766. The house style cites the PR too ("sparklemotion#766, #NNN").

A related note on code this PR doesn't touch, for whoever looks at item 1 upstream: any Ruby callback that runs while SQLite holds the connection's mutex hangs a second thread that uses the connection, on every platform. That includes busy_handler (with or without sleep) and a create_function block that sleeps or loops. That is in some tension with README's thread-safety paragraph, but it isn't this PR's to fix.

The one red CI job, sqlcipher (windows, mingw), fails the same way on upstream main (pacman can't find the package).

…n if it can't be installed

Review follow-up:

- CHANGELOG: limit both entries to builds with SQLite linked into the
  extension (the precompiled gems and the default build); list what
  upgraders will notice (same-process lock holders can't run, so the
  wait ends in BusyException; interrupts and Timeout take effect only
  when the wait ends; the VFS's own sharing-violation retries hold the
  GVL too); and qualify the busy_handler_timeout= advice: it lets other
  threads run only if they don't use the waiting connection.
- Database#busy_timeout rdoc: state that the wait holds the GVL and
  defers interrupts. Database#busy_handler_timeout= rdoc: add the same
  shared-connection caveat.
- sqlite3.c: give the real reason for GetProcAddress (a direct reference
  would bind to the Ruby-DLL import; the function-like macro isn't what
  matters), list winClose's retry and the WAL read retry, note that
  xSetSystemCall is a testing aid and which test guards the fix, and
  warn at load time if no VFS accepts the override instead of failing
  silently.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@mrhoribu

mrhoribu commented Nov 6, 2026

Copy link
Copy Markdown
Owner Author

@MahtraDR thanks. All six items are addressed in 15e4a37.

1. busy_handler_timeout= advice. The CHANGELOG now says it lets other threads run only if they don't use the waiting connection, and that threads sharing a connection should keep busy_timeout or use one connection per thread. The same caveat is in busy_handler_timeout='s rdoc, so it outlives the CHANGELOG.

2. Upgrade effects. The "Changed" entry now lists three effects:

  • a same-process lock holder can't run, so the wait ends in BusyException after the full timeout;
  • Timeout, Thread#raise/#kill and Ctrl-C take effect only when the wait ends, and a shorter Timeout gives BusyException instead of Timeout::Error;
  • the VFS's own sharing-violation retries hold the GVL too, whatever busy handler is set (about 1.4 s per failing I/O call, 200 ms on close).

The PR description has the same list.

3. Scope. Both entries now say "when SQLite is linked into the extension (the precompiled gems and the default build)". The fix entry adds that --enable-system-libraries, mswin and SQLCipher builds already used kernel32's Sleep.

4. "As documented". Database#busy_timeout's rdoc now says the wait holds the GVL, that interrupts take effect only once it ends, and that a same-process lock holder can't release the lock in the meantime. That makes the test header's claim true.

  • Side note: an ad-hoc rdoc run renders busy_timeout= without any description, including its existing text, so the call-seq style there may need a look. That's pre-existing.

5. C comment. Fixed:

  • The reason for GetProcAddress is now the real one: a direct reference would bind to the same Ruby-DLL import.
  • The list of sleep sites now includes winClose's retry and the WAL read retry (both through winSleep/osSleep).

6. Testing hook.

  • The xSetSystemCall return codes are now checked. If no VFS accepts the override, the extension warns once at load time instead of failing silently.
  • The comment says xSetSystemCall is a testing aid whose overridable calls may change, and that test/test_sqlite_sleep.rb guards the fix on the Windows CI jobs.
  • The PR description now has an "Alternatives considered" section, covering the runtime override, link-order fixes and SQLite as a separate DLL, with the trade-offs.

House style. Noted in the description: add the upstream PR number to both CHANGELOG entries once it exists.

Verification at 15e4a37:

  • Linux: full suite 431 runs, 0 failures. RuboCop clean.
  • MinGW cross-compile of sqlite3.c and database.c against RubyInstaller 4.0.9's headers: no new warnings (3 before, 3 after, all pre-existing).
  • This push triggers the PR's CI; I'll post the result.

Housekeeping: GitHub resolves a plain #766 written on this fork to the upstream issue, so references here are in code formatting until the PR goes upstream.

@MahtraDR MahtraDR left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

PR Review: fix: point SQLite's Sleep at kernel32 on Windows

Reviewer: Claude · 15e4a37 · 2026-11-06

Thanks. I re-checked all six items at 15e4a37, and each one is fixed in a way that answers the point.

  1. The CHANGELOG and the busy_handler_timeout= rdoc now both say it's only safe when no other thread uses the waiting connection. The rdoc text shows up in the generated docs.
  2. The three upgrade consequences are in the "Changed" entry and the PR description. The figures match what I measured in round 1.
  3. Both entries now say the change applies when SQLite is linked into the extension.
  4. The busy_timeout rdoc now states the GVL hold, the deferred interrupts and the starved same-process lock holder, so the test header's "as documented" is true.
    • Your side note is right, and it predates this PR. With the repo's rdoc, busy_timeout= renders with no description at all, on main too, so the new text is in the source but not in the rendered docs. Switching the call-seq to the multi-line form didn't fix it. That's worth raising upstream separately.
  5. The C comment now gives the real reason for the runtime lookup, and the list of sleep sites includes winClose and the WAL retry.
  6. The return codes are checked, and the warning works.
    • I compiled the new function body verbatim against stubs. All eight cases behave as expected: no warning when any VFS accepts the override, and exactly one warning when none does, when kernel32 or Sleep can't be found, or when the VFS is too old. The check fails if I remove the == SQLITE_OK test.
    • The only case where it would warn without cause is a new header used with a pre-3.7.6 runtime DLL, which isn't a realistic setup.
    • The SQLCipher Windows job, which runs this code against a separate DLL, printed no warning.

One correction to my own round-1 wording, which you followed: I listed "mswin" among the builds that use SQLite as a separate DLL. That's true for mswin with --enable-system-libraries, which is the only mswin setup CI tests. extconf doesn't force it, though, so a vendored mswin build (if that works at all) would be affected like the default build. The fix runs there too, so nothing needs to change.

CI at 15e4a37 has 131 passing jobs and 3 failures, none caused by this PR:

  • sqlcipher (windows, 3.3) failed test_replacing_a_collation_releases_the_previous_comparator, an ObjectSpace count after GC.start. It fails the same way on upstream main at this PR's base commit (run 41797589777) and on the v2.10.2 tag.
  • sqlcipher (windows, mingw) is the same MSYS2 package failure as before.
  • arm64-darwin 3.3 timed out downloading the gem artifact before any test ran. Nothing in this commit compiles differently on macOS, and the job passed at 5bd933d. A re-run should turn it green.

The issue-only CHANGELOG citation is noted in your description for when it goes upstream.

@mrhoribu

mrhoribu commented Nov 6, 2026

Copy link
Copy Markdown
Owner Author

CI at 15e4a37: 131 jobs pass, 3 fail, none from this change.

  • sqlcipher (windows, mingw): the MSYS2 package install fails. It fails the same way on main.
  • arm64-darwin 3.3: the artifact download timed out (ETIMEDOUT).
  • sqlcipher (windows, 3.3): TestCollation#test_replacing_a_collation_releases_the_previous_comparator failed. The same test fails on main's own CI (test source (windows, 3.3, disable)). SQLCipher builds don't use the path this PR changes, and the test passed on this PR's previous head.

All Windows jobs with SQLite linked into the extension pass, including the native-gem tests. On the Ruby 4.0 job (test (windows, 4.0, disable)): 431 runs, 0 failures, and no "could not point SQLite's Sleep" warning in the log, so the override installed.

mrhoribu and others added 2 commits November 5, 2026 20:59
Only --enable-system-libraries builds (which SQLCipher builds imply) use
SQLite as a separate DLL and already had kernel32's Sleep. CI's mswin job
uses that option, but a vendored mswin build would be affected like the
default build; the fix runs there too.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@mrhoribu

mrhoribu commented Nov 6, 2026

Copy link
Copy Markdown
Owner Author

Opened upstream as sparklemotion#771, from this same branch. The rdoc rendering problem is filed separately as sparklemotion#772.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants