[el-marvin]

test_diff_config and test_diff_config_extended in tests/fast/test_many_con_same_file.py fail under pytest-xdist with IOException: Could not set lock on file ".../test.db": Conflicting lock is held. Every file database test in that module opens test.db in the working directory, and a database file is locked by the process that opens it, so when xdist runs two of these tests on different workers at the same time the second cannot open it. The file alone at -n 8 fails in each of three runs, test_multiple_writes included; serially it passes.

Each of the three tests now opens its own database under tmp_path, and the unlinking of test.db around test_multiple_writes goes away. They still check the same things: two connections to one file see each other's tables, and a second connection to a file with a different configuration is refused. The file passes ten runs in a row at -n 8. test_windows_abs_path opened the same test.db in the working directory and now changes into tmp_path and opens it there. The change of directory is needed because the test strips the drive letter from the path, and a path without its drive resolves on the current drive; with the checkout on D: and the temp directory on C:, as on the Windows runners, the stripped path would otherwise name a directory that does not exist. The test runs on Windows only.

test_arrow_str_scan_runs_without_gil holds the GIL for 0.2 seconds in a UDF at the first row and measures how much the profiler's summed TABLE_SCAN time grows: for an object column, which takes the GIL for every vector and so must stall by at least one hold (the calibration), and for the str column, which must grow by less than half a hold. It took one held run minus one free run per round and compared the medians of three rounds. Load on the machine adds time to any run, so a slow free run shrinks a round's difference and a slow held run grows it. On a shared macOS x86_64 runner the calibration came out at 3.41, 0.99 and 0.99 holds and failed. Under a one core CPU quota with busy loops beside it here, the str column's median came out at 0.71 and 0.88 holds in two of twelve runs, although the scan does not take the GIL.

The test now runs five rounds and judges both columns with one statistic, the fastest held run minus the fastest free run. The str column is judged first: a growth of two holds or more fails whatever the calibration says, since a stall that large needs no calibration to be trusted. A scan that takes the GIL reads about three holds, as three of its four threads wait out the hold, and read 2.12 at its weakest under load with the mutation below; load alone has read up to 0.96 holds on a scan without the GIL, so a line at one hold would be within 0.04 of failing a correct build. Below that, the object column must have reached one hold; load can take stall away from a held run of a scan that takes the GIL, when fewer of its threads wait out the hold, so when the calibration misses, the measurement could not see a stall under that load and the test skips as inconclusive, printing both values. With a met calibration the str column must stay below half a hold, as before. test_str_scan_judgement pins the judgement over recorded values, among them a recorded window under a GIL taking mutation whose calibration read 0.61 holds while the str column read 3.15, which fails rather than skips, and the loaded window where the str column read 0.96 against a calibration of -0.02, which skips. With ScanArrowColumn made to take the GIL for every vector, every window of five recorded rounds fails at each of three loads, and every test run fails each parameter it does not skip, at 2.0 to 4.0 holds.

TestGil in tests/fast/arrow/scan/test_arrow_scan_row_ranges.py made the same measurement with its own copy of the code and judged each source that reads without the GIL by one held minus free round; on one core beside sixteen busy loops it failed 4 of 24 runs that way. The measurement and the statistics now live in one module, tests/_gil_stall.py, that both tests import, and TestGil runs five rounds for every source. The direction of a bound picks its statistic. The three sources without the GIL are bounded from above by the fastest held run minus the fastest free run, as the str column is. The object_frame source is bounded from below, and there the fastest held run is the wrong pick: one held run that lost its stall under load decides it, and on one core beside sixteen busy loops it read 0.52 and 0.68 holds in 2 of 9 runs, while in 10 of 46 recorded windows under that load it fell under one hold. object_frame is therefore judged by its best round, the largest held minus free difference of the five, which is the rule it had before over three rounds. The figures for it depend on how the load is applied. Under a one CPU quota with 16 busy loops in the same scope, the load the figures in this description come from, the best of five rounds passes 8 of 8 runs and reads at least 1.91 holds over every recorded window of five rounds on a correct build. Pinned to one core beside 16 busy loops with taskset, where the four scan threads share the core with the loops and a thread that waits for the GIL would have waited for the CPU anyway, the bound still fails about one run in eight: 1 of 8 at 0.48 holds, the same rate as the merge base's best of three, which failed 1 of 8 at 0.92 there. It still asserts the stall hard, so a measurement that stopped seeing any stall fails the suite there. The module sits in tests/ because that is the one directory both test files import from already, through the root conftest.py; a second conftest.py is not an option, since pytest replaces the root one in sys.modules when a conftest in a directory that is not a package loads, which breaks the imports of _pushdown_helpers.

Both timing tests now run on Linux only, at the maintainer's decision: the GIL release lives in the binding and does not vary by platform, while the measurement is what varies, and the macOS runners supplied the noise. The one decorator per test replaces the Windows skip it subsumes, and the restriction is temporary: it holds until a test-only counter of GIL acquisitions replaces the timing measurement.

The fastest held run is less sensitive to a GIL taking scan than the old median under the heaviest load: its weakest window against that mutation was 0.52 holds, just above the half hold bound, where the median's was 2.63; that window had a met calibration and so failed on the bound, and every window whose calibration missed read 3.10 holds or more, which is what the two hold line rests on. The tests also cost more: about 2.8 seconds per parameter of the str column test normally instead of 1.7, and TestGil runs ten queries per source, where it ran two for the sources without the GIL and six for object_frame. Under a one core quota with 16 busy loops a parameter took up to about 50 seconds against the 120 second timeout, and the calibration misses in some windows there, so the str column test skips one of its parameters in some runs: in 14 runs of both tests under that load here it skipped 4 of 42 parameters, and in one of those the str column itself read 0.96 holds, the reading that sets the fail line at two. In those runs TestGil failed once on object_frame with the fastest held run as its statistic, which is what moved that source to its best round. With the best round, TestGil passed all four sources in eight of eight runs under that quota load.

test_a_python_error_in_a_pull_on_an_engine_thread_fails_the_query in tests/fast/arrow/scan/test_arrow_scan_streams.py failed once with DID NOT RAISE in this pull request's own CI run. Its stream raises only when an engine worker thread pulls, and yielded 2000 one-row batches. execute drives the query on the submitting thread until its first result, every pull takes the one pull lock, and a mutex is not fair, so when the worker threads get no CPU for a few tens of milliseconds the submitting thread pulls all 2000 batches itself and the query returns the sum. That is a race in the test: the product never lost an error raised on an engine thread in about 12,000 probe runs. With the engine workers confined to one CPU beside 32 busy loops and the submitting thread on another, the old shape returned the sum in 99 of 100 runs with the pull outside the GIL and 85 of 100 with it inside. The stream is now endless, so the query can only end through an engine thread's error; the test carries the file's 60 second timeout, so a run in which no worker ever pulls ends as a timeout; and its precondition, that an engine thread raised, is asserted before the error message, so a precondition failure names itself. Under the same confinement the new shape raised in 100 of 100 runs of each kind, after at most 25,850 pulls on the submitting thread and within 535 milliseconds.

The appender test test_a_flush_refused_while_another_thread_runs_a_statement_keeps_the_rows is not part of this change. Its flake comes from a connection claim defect: execute drops the claim between submitting a statement and settling it, so a call from another thread in that gap runs instead of being refused, or is refused with the engine's open result message. That defect is fixed on the branch fix-execute-claim on v2.0-cyanoptera, which holds one claim from submission to settling in execute, executemany and the DB-API Arrow fetch; the test stays as it is until that lands, so the flake stays visible.

[el-marvin]

Changes requested.

What I reviewed. This is round 1, on 65403ac, the single commit on top of the merge base c2556ed. The change touches tests only, so one build served both sides: the head's release build ran the head tests, and also the merge base tests from a clone at the merge base. For the mutations I built a copy of the head with two switches, each off unless an environment variable turns it on:

• a sleep in DuckDBPyConnection::Execute between its two connection claims;
• the GIL taken for every vector in ScanArrowColumn.

The premise holds for all three tests. On the merge base:

• test_many_con_same_file.py fails 3 of 3 runs at -n 8 on the lock of test.db.
• The appender test fails 50 of 50 runs, with the message from CI, once the gap in execute is widened by 50 ms.
• The GIL test's calibration median (the old statistic) falls under one hold in 4 of 10 windows of three rounds, recorded on one core shared with 16 busy loops.

Layer. The file fix is right and sits at the right layer. The GIL change is a measured improvement, with one gap in the order of its checks. The appender change sits at the wrong layer. The race it avoids is a defect in execute, and that defect can also refuse execute itself and hang the process. Pinning the test to one thread removes the suite's only trigger for it.

Findings

1. SET threads = 1 hides a hang in DuckDBPyConnection::Execute, which releases the connection claim between submitting a statement and settling it (tests/fast/api/test_appender.py:1440).
Severity: blocking. Category: design. Evidence: ran. Fix: root-cause. Origin: repository. Base: widened.
Hold one claim from submission to settling in execute, and also in executemany and the DB-API Arrow fetch, which take the same two claims. Close a refused pending result with the GIL released. Keep this test at the default thread count. The alternative is to take this hunk out of the pull request until that fix lands. The evidence is in the inline comment.

2. The inconclusive skip runs before the str column is judged, so a str scan that stalls by three holds is reported as a skip whenever the calibration misses (tests/fast/pandas/test_pandas_string.py:965).
Severity: should fix. Category: missing coverage. Evidence: ran. Fix: root-cause. Origin: repository. Base: introduced.
Judge the str column first and fail when its statistic reaches one hold, whatever the calibration says. Skip only when the calibration misses and the str column stays below that.

3. TestGil.test_scanning_threads_keep_reading_while_another_holds_the_gil makes the same measurement and keeps the single round judgement that this pull request replaces, and it fails under the same load (tests/fast/arrow/scan/test_arrow_scan_row_ranges.py:879 and 888). This finding does not block the pull request.
Severity: minor. Category: design. Evidence: ran. Fix: root-cause. Origin: repository. Base: pre-existing.
Move the measurement and the statistic into one helper that both tests use, so that one measurement is judged one way.

This test runs the same query with a 0.2 s hold, 1,000,000 rows and 4 threads. It judges each source that runs without the GIL by one held minus free round:

        rounds = 3 if takes_gil else 1
        ...
            assert holds < 0.5, holds

On one core with 16 busy loops, its three parameters without the GIL failed 4 of 24 runs (4 failed, 20 passed, 8 deselected), two of them at 0.55 and 0.70 holds. Under the same load, the test this pull request changes passed every window of five rounds.

The GIL test's statistic

The description says this question is with Evert. I give the measurements here and do not settle it.

What it measures. The profiler's TABLE_SCAN timing, summed over the four scan threads of the connection. A thread that waits for the GIL inside the scan adds its wait to that sum. The hold at the first row stalls every other thread that is scanning a column that takes the GIL. That is why the object column reads about three holds unloaded: three of the four threads wait.

How many samples. Five rounds, each with one held run and one free run per column: 20 queries per parameter. The statistic takes the fastest of five held runs and the fastest of five free runs per column, and the two may come from different rounds.

On a loaded box. I recorded rounds of string_pyarrow with the test's own query and judged every window of consecutive rounds: windows of three for the old statistic, windows of five for the new. "With the GIL taken" means the copy of the head whose ScanArrowColumn takes the GIL for every vector.

• No load, 30 rounds. Old statistic: 0 of 28 windows fail. New statistic: 26 pass, 0 skip, 0 fail; str column at most 0.00 holds. With the GIL taken: 0 pass, 0 skip, 26 fail; str column at least 2.94 holds.
• 2 cores, 8 busy loops, 12 rounds. Old: 0 of 10 fail. New: 8 pass, 0 skip, 0 fail; str at most 0.03 holds. Not run with the GIL taken.
• 1 core, 8 busy loops, 15 rounds. Old: 0 of 13 fail. New: 11 pass, 0 skip, 0 fail; str at most 0.00 holds. With the GIL taken: 0 pass, 0 skip, 11 fail; str at least 2.12 holds.
• 1 core, 16 busy loops, 12 rounds. Old: 4 of 10 fail, all on the calibration. New: 8 pass, 0 skip, 0 fail; str at most 0.45 holds. With the GIL taken: 0 pass, 1 skip, 7 fail; str at least 3.10 holds.

What this shows:

• Run times on a loaded single core take two values. A run that escapes descheduling reads about 0.007 s. One that does not reads 0.1 to 0.18 s for the str column.
• Why the statistic resists load. The fastest of five keeps the undisturbed value whenever one run escaped. When all five held runs were disturbed and one free run was not, the str column reads 0.45 holds against the 0.5 bound. So at the heaviest load I ran, the bound has a margin of 0.05 holds.
• Against the mutation, the test held. Every window that was not skipped failed, the weakest at 2.12 holds. The old statistic let one window of ten pass at the heaviest load.
• The description's weakest window did not reproduce. It reports 0.52 holds; mine was 2.12. My load pins the process and the busy loops to cores with taskset rather than a CPU quota, which may explain the difference.
• The skip cannot become a permanent hole by itself. If the measurement stopped seeing the object column stall at all, the object_frame parameter of TestGil asserts that stall hard (tests/fast/arrow/scan/test_arrow_scan_row_ranges.py:886) and would fail.

Smaller things

• _held_and_free_seconds returns pairs that _growth_in_holds transposes straight back with zip(*rounds, strict=True). Two lists per column would say the same with less code.
• test_windows_abs_path (tests/fast/test_windows_abs_path.py:43) still opens test.db in the working directory. No other test uses that name now, so it cannot collide, but tmp_path would keep the working directory clean.
• The comment at tests/fast/api/test_appender.py:1438 describes the gap in execute as a property of the test. It goes with finding 1.

Questions

• How often does the macOS x86_64 runner skip? The description's failing run there read 3.41, 0.99 and 0.99 holds per round. A round at 0.99 is a held run in which only one thread waited out the hold. When the fastest held run decides, one such run among five would put the calibration under one hold, so that runner may skip most of the time. I could not run it.
  • What would settle it: the skip reason prints the calibration value, so about 20 runs of the test on macos-15-intel with -rs give the skip rate.
  • If it skips mostly, a calibration bound under one hold would help. A single waiting thread reads just under one, and a lower bound still keeps a margin over the 0.5 bound.

Outside findings not taken

• Codex: the fastest held run lets a run without contention decide, so a GIL taking str scan could pass. Its example was held times of 0.23, 0.23, 0.03, 0.23 and 0.23 s. Not reproduced: under the mutation, no held run of the str column escaped the stall at any of three loads, and every window not skipped failed at 2.12 holds or more.
• code-review: the same point, with a simulated false pass rate of up to 9 percent. Rejected for the same reason.
• Codex: the skip can permanently disable the assertion when the measurement breaks. The object_frame parameter of TestGil asserts the stall hard (test_arrow_scan_row_ranges.py:886), so such a breakage fails the suite there.
• Codex: the calibration call counts are discarded (test_pandas_string.py:956). The str frame runs the same query text, and its counts are asserted. A calibration whose UDF did not run reads under one hold and skips, so it cannot let a regression pass.
• code-review: a calibration bound of one hold makes the macOS x86_64 runner skip in about 90 percent of windows, by its model. Moved to Questions: I could not run that runner, and the model rests on three reported values.

Prerequisites in the engine

None. The engine's Submit runs a query on worker threads only when there are more threads than external threads (external/duckdb/src/include/duckdb/main/client_context.hpp:190). That is its documented contract; the gap is in the client.

What I verified

Head and range. Head 65403ac, range c2556ed..65403ac. One commit, no merge commit, three test files, no source file.

Builds.

• The head's release build in the tree (BUILD OK).
• A release copy of the head with the two switches: REVIEW_EXECUTE_SLEEP_MS sleeps between Execute's claims, and REVIEW_ARROW_GIL takes PyUtil::QueryGil at the top of ScanArrowColumn.
• The merge base tests ran from a clone against the head build, since the sources are identical.

Shared file.

• The base file at -n 8: 3 runs, each 2 failed, 3 passed, with IOException: Could not set lock on file ".../test.db": Conflicting lock is held.
• The base file run serially: 5 passed.
• The head file at -n 8: 10 of 10 runs 5 passed.

Appender.

• The base test with the 50 ms sleep: 50 failed, each with assert 'is busy with another call' in 'Resource In Use Error: connection has an open result; drain or destroy it before starting a new query'.
• The head test with the sleep: 50 passed.
• Without the sleep: 400 runs each of the base and head tests at -n 8, all passed.

Claim gap.

• The 200 ms gap probe, and the hang reproduced twice, are described in the inline comment on finding 1.
• Without a widening sleep and without a busy Python thread, 1,500 flushes and 1,000 relation creations from a second thread were all refused as busy.

GIL test.

• Recorded rounds as in the table above, with 4 threads and memory_limit 8GB.
• The touched files and their siblings at head (test_many_con_same_file.py, test_appender.py, test_pandas_string.py, test_arrow_scan_row_ranges.py, test_execute_contract.py) at -n 8: 669 passed, 31 skipped, 4 xfailed in 21.19s.
• The same files from the merge base: 669 passed, 31 skipped, 4 xfailed in 20.85s.
• TestGil on one core with 16 busy loops, 8 repeats: 4 failed, 20 passed, 8 deselected.

Outside reviews.

• The code-review skill ran at max effort, not the high I asked for, because it reads the level only when it comes first.
• Codex ran read only at high reasoning effort.

CI at the time of review: no check had failed. Lint, the full API build with assertions, the sdist, the seed wheels and several abi3 test jobs had passed; most abi3 test jobs were still pending.

What held

• One file per test under tmp_path. Two connections to one file still see each other's tables, and a second connection with another configuration is still refused, without a file shared between workers.
• The new GIL statistic. At no load I ran did it falsely fail or skip a column that runs without the GIL, while the old median failed 4 of 10 windows at the heaviest load. It caught the mutation in every window it did not skip.
• The appender test at one thread. It still tells the two refusals apart (the base form fails with the open result message once the gap opens), and it still pins that the refused row stays and that a later flush writes it.
• The description's figures that I could rerun. These hold: the 3 of 3 failures and 10 clean runs of the shared file, the 50 of 50 failures with the 50 ms sleep, and the 2.8 seconds per parameter (0.57 s per round here).

[el-marvin]

SET threads = 1 hides a hang in DuckDBPyConnection::Execute, which releases the connection claim between submitting a statement and settling it.

Severity: blocking. Category: design. Evidence: ran. Fix: root-cause. Origin: repository. Base: widened.

The race the description names is real, and it is more than a different message. execute() submits through PrepareAndSubmitInternal, whose session.Submit takes a claim and drops it on return (src/engine/connection.cpp:345, auto claim = Claim();). It then claims again in res->Settle(CheckSignals) (src/pyconnection.cpp:907, through ClaimWhileRunning at src/engine/result.cpp:286). With worker threads the engine starts the statement as soon as it is submitted (external/duckdb/src/include/duckdb/main/client_context.hpp:190), so another thread can take the claim in between. I reproduced three consequences:

• The other thread's call goes through instead of being refused. I built a copy of the head with the gap widened to 200 ms and ran it with 8 threads. From the second thread, con.appender("t"), con.query_appender(...), con.sql("SELECT 1") and con.table("t") all ran while the statement was half run. Without the sleep, or at one thread, each of them is refused as busy. A second con.execute("SELECT 1") passed the busy check and waited 10.2 s for the first call's lock instead of refusing. The first thread's fetchall() then returned [(1,)], the second thread's row, instead of [(0,)].
• execute itself is refused as busy, and the process hangs. This happens when the other thread still holds the claim at the moment Settle claims. The pending result is then destroyed after the GIL has been taken back. Closing it waits for the statement's tasks (Executor::CancelTasks), and a task running a Python UDF waits for the GIL, so no timeout ends the hang.
  • On the unmodified head build I ran this test's shape without the SET: a flush from a second thread, 4 threads, a busy Python thread and sys.setswitchinterval(0.0001). It hung at iteration 271 of 600. faulthandler shows the main thread in con.execute, the flushing thread in app.flush() and the UDF in release.wait.
  • With the gap widened, a second thread that keeps calling con.table("t") hangs execute on the first attempt. gdb shows the main thread in duckdb::Executor::CancelTasks under duckdb::engine::Result::~Result.
• The flush gets the wrong refusal. It is refused with the engine's "connection has an open result; drain or destroy it" for a result the caller never held. That is the failure CI saw.

The repository already has the rule this code breaks. Two places hold one claim from submission to the end through SubmitClaimed and RetainClaimed or SettleClaimed:

• src/engine/connection.cpp:517: "One claim from submission to the end of the run, so no other call finds the statement pending".
• src/pyconnection/seam.cpp:784.

Three places take two claims instead: execute (src/pyconnection.cpp:835 and 907), executemany (805 and 816) and the DB-API Arrow fetch (1560 to 1565). Execute's own parameter refusal already closes a pending result with the GIL released (src/pyconnection.cpp:895). The busy refusal at Settle does not.

The sibling test_another_thread_is_refused_while_a_statement_runs in tests/fast/api/test_execute_contract.py uses that file's one thread fixture, so it hides the same gap. Accepting either refusal at the default thread count would not help either, because the hang does not depend on what the test asserts.

Ask: either fix it here or take the hunk out. Fixing it here means three things: hold one claim from submission to settling in those three places, close a pending result that the refusal leaves behind with the GIL released, and keep this test at the default thread count, where it then always sees the busy refusal. The other option is to take this hunk out of the pull request, so the flake stays visible until that fix lands.

[el-marvin]

The inconclusive skip runs before the str column is judged, so a str scan that stalls by three holds is reported as a skip whenever the calibration misses.

Severity: should fix. Category: missing coverage. Evidence: ran. Fix: root-cause. Origin: repository. Base: introduced.

    if calibration_growth < 1:
        pytest.skip(f"inconclusive: the hold stalled the object column scan by only {calibration_growth:.2f} holds")
    assert _growth_in_holds(rounds) < 0.5

I built a copy of the head in which ScanArrowColumn takes the GIL for every vector (behind a switch), and recorded rounds of string_pyarrow on one core with 16 busy loops. Of 8 windows of five rounds, 7 fail. Window 1 skips: its calibration reads 0.61 holds while the str column reads 3.15 holds.

The calibration says whether a low str reading can be trusted. A high reading does not need it. At the same load the str column without the mutation never read above 0.45 holds, and the worst median the description reports under the old statistic was 0.88.

Ask: compute the str statistic first and fail when it reaches one hold, whatever the calibration says. Skip only when the calibration misses and the str column stays below one hold.

[el-marvin]

This is ready to merge. I reviewed 73379f0, one commit on top of round 1's 65403ac, against the merge base c2556ed. The commit takes the appender hunk out, moves the GIL measurement of both timing tests into tests/_gil_stall.py, judges the str column before the calibration, restricts both timing tests to Linux, and adds a fourth change: the pull error test in test_arrow_scan_streams.py now reads an endless stream. The change is tests only, so the head's release build served the head's tests and, from a clone at the merge base, the base's tests; my round 1 copy of the head carries switches for the mutations, and I added two: one that drops a pull error and one that holds the GIL in the Arrow scan.

The premise holds for all four tests. The shared file fails 3 of 3 runs at eight workers on the merge base. The str test's old median and TestGil's single round failed under load in round 1. The pull error test's old form returned the sum instead of raising in 80 of 200 runs here with the engine's worker threads starved of CPU, which is the DID NOT RAISE the x86_64 abi3 leg showed on 65403ac. Each fix sits at the right layer: each removes a race or a load sensitivity in the test, and the one product defect round 1 found, the connection claim in execute, is left visible instead of hidden.

Nothing here blocks merging. The items below are one minor finding and three smaller things the author may take or leave. Outside the code, the maintainer still has to see the CI run on 73379f0 finish: when I looked, only the lint and submodule checks had passed, and the abi3 test legs had not started, among them the one that failed on 65403ac.

Since the last review

• Finding 1, blocking: SET threads = 1 hid a hang in DuckDBPyConnection::Execute. Addressed: the hunk is gone. The diff of tests/fast/api/test_appender.py from c2556ed to 73379f0 is empty, so the test is back at the default thread count and its flake stays visible until the connection claim is held from submission to settling. The description now says so.
• Finding 2, should fix: the inconclusive skip ran before the str column was judged. Addressed, with a fail line at two holds instead of the one hold I asked for, and the reason holds. _judge_str_scan (tests/fast/pandas/test_pandas_string.py:886) now asserts growth < 2 before it looks at the calibration. My round 1 window (calibration 0.61, str 3.15) is a case of test_str_scan_judgement and fails there, and the line is pinned on both sides by 0.61 with 2.00 (fails) and 0.61 with 1.99 (skips). I reran the round 1 recording on the build whose ScanArrowColumn takes the GIL, one core with 16 busy loops: all 8 windows fail on the two hold line, two of them with the calibration at 0.61 holds, which the round 1 code would have skipped; the str column read 2.60 to 3.09 holds. Every window of the round 1 recordings, judged by the new rule, also fails on that line for the mutation (at least 2.12 holds) and passes for the correct build. A line at one hold would sit 0.04 holds above the one reading of 0.96 the author reports for a correct build with a missed calibration, while the mutation has never read under 2.12; two holds is the better line, and better than what I suggested.
• Finding 3, minor: TestGil kept its single round judgement. Addressed. The measurement lives in tests/_gil_stall.py, both tests import it, and TestGil runs five rounds per source with the fastest pair for its upper bounds and the best round for object_frame. The object_frame bound is the subject of the one finding below.
• Question: how often does the macOS x86_64 runner skip? Moot: both timing tests now run on Linux only, the maintainer's decision.
• Smaller things. _held_and_free_seconds and its transposition are gone; ScanTimings keeps two lists. test_windows_abs_path now takes tmp_path and changes into it, which is needed because the test strips the drive letter (read, not run: it runs on Windows only). The comment at test_appender.py:1438 left with the hunk.

Findings

1. TestGil's object_frame lower bound still fails under a pinned one core load, so the description compares round 1's "4 of 24" with eight clean runs made under a different load (tests/fast/arrow/scan/test_arrow_scan_row_ranges.py:851).
Severity: minor. Category: docs. Evidence: ran. Fix: containment. Origin: repository. Base: pre-existing.
Say in the description that the eight clean runs were under a CPU quota and that, pinned to one core, this bound still fails about one run in eight as the base does; or, since one round that shows the stall is enough, stop the object_frame loop at the first round that reaches one hold and allow more rounds before failing. The evidence is in the inline comment.

The Linux restriction

The three conditions attached to the maintainer's decision hold:

• Both tests carry @pytest.mark.skipif(sys.platform != "linux", reason="the GIL release does not vary by platform; its timing does") (tests/fast/pandas/test_pandas_string.py:945, tests/fast/arrow/scan/test_arrow_scan_row_ranges.py:832).
• Both still run where they must. The pull request gate (on_pr.yml) runs the packaging workflow with the whole suite, whose abi3 test matrix has the ubuntu-24.04 x86_64 and ubuntu-24.04-arm aarch64 legs; packaging_wheels.yml:255 copies all of tests/ and runs ./tests/fast or ./tests with no marker or keyword filter. assert_test.yml:100 runs ./tests/fast on ubuntu-24.04. The pandas 2 leg runs test_pandas_string.py itself on Linux (packaging_wheels.yml:297). Here, on Linux, both tests ran and were not skipped.
• The comment above each decorator says "Linux only until a test-only counter of GIL acquisitions replaces the timing.", and the description says the restriction is temporary until such a counter replaces the measurement.

The shared module's import path holds in each of those runs. tests/ has no __init__.py, so pytest's default import mode puts it on sys.path for the root conftest.py; that is the route every from conftest import ... in the suite already takes, and the wheel and pandas 2 legs copy the whole directory. _gil_stall.py matches no test file pattern, so it is not collected. The description's reason for not adding a second conftest.py is right: pytest 9.2 deletes sys.modules["conftest"] before it imports a conftest outside a package (_pytest/config/__init__.py, _importconftest).

Smaller things

• The description gives two weakest readings for the GIL taking mutation: "read 2.12 at its weakest under load with the mutation below" where it sets the two hold line, and "its weakest window against that mutation was 0.52 holds" further down. Both can be true, since the reply says the 0.52 window had a met calibration and failed on the half hold bound, but as written the second undoes the margin the first claims. Saying that every window whose calibration missed read 3.10 holds or more (2.60 or more in mine) would state what the two hold line rests on.
• "TestGil runs ten queries per source instead of two": object_frame ran six before (three rounds), the other sources two.
• The appender paragraph says the claim defect "is fixed separately". A link to the issue or pull request that tracks it would let a reader find it.

Outside findings not taken

Codex (read only, extra high reasoning effort) reported no findings. The code-review skill (high effort) reported ten; none is taken:

• The str test judges its calibration, a lower bound, with the fastest pair, which the shared module's own docstring calls the wrong pick for a lower bound. The statistic and the skip on a missed calibration are the maintainer's decision for this pull request. The cost is a skip, not a false pass: pinned to one core with 16 busy loops, 4 of 8 recorded windows skip, and in each of them the str column read 0.00 holds.
• A calibration that misses now skips instead of failing, so a broken measurement goes unnoticed. The same decision. TestGil's object_frame parameter still asserts the stall hard, so a measurement that stops seeing any stall fails the suite there.
• With an endless stream, a change that keeps every pull on the submitting thread makes the test spin until its timeout. It fails, and names why. I ran the new form with threads = 1 and a 5 second timeout: pytest-timeout's handler raises inside the generator on the submitting thread, the query fails with reading the stream registered as 't' failed: Unknown error: Timeout (>5.0s) from pytest-timeout, and the test then fails on no engine thread pulled, so the error was raised on the submitting thread.
• assert raised_on can no longer fail usefully and its message misleads. The same run: it is the line that fails when no worker pulls, and its message is then accurate.
• The half hold bound on the str column can still flake under load. Not reproduced: in every window whose calibration was met, the str column read at most 0.45 holds in round 1 and at most 0.00 here, at up to 32 busy loops on one core.
• macOS loses both GIL tests. The maintainer's decision, and the three conditions attached to it hold (see below).
• The thread count is a literal 4 in the str test. The base has the same literal; style.
• test_windows_path_accent still writes tést/test.db under the working directory. Untouched by the change, and the Windows legs run pytest without xdist, so nothing races on it.
• The comment "Linux only until a test-only counter of GIL acquisitions replaces the timing." is a task note. It says why the skip exists and when it goes away, which the maintainer asked to be stated.
• scan_seconds builds ctypes.PyDLL(None) on every call. The base did the same, and looking up an already loaded library 20 times per parameter costs nothing measurable.

Prerequisites in the engine

None.

What I verified

Head and range. Head 73379f0; range c2556ed..73379f0, two commits, no merge commit, six test files and no source file. The delta since round 1 touches no file outside tests/.

Builds.

• The head's release build in the tree (BUILD OK; the first import rebuilt nothing).
• My round 1 release copy of the head, with the head's tests brought over and four switches, each off unless its environment variable is set: a sleep in Execute between its two claims (round 1), the GIL taken in ScanArrowColumn (round 1), a pull error dropped in PythonArrowScanExec (it marks the stream exhausted and returns instead of rethrowing), and the GIL held for each PythonArrowScanExec call.
• The merge base's tests ran from a clone at c2556ed against the head build; the sources are the same.

Earlier reproducers.

• Shared file at eight workers: head 10 of 10 runs 5 passed; base 3 of 3 runs failed on Could not set lock on file ".../test.db": Conflicting lock is held (2, 2 and 1 failures).
• Appender: the test file is identical to the merge base.
• GIL recordings, string_pyarrow, twelve rounds each, judged with the head's rules: with the GIL taken, one core with 16 busy loops, 8 of 8 windows fail on the two hold line; the correct build at 16 and at 32 busy loops, 4 pass and 4 skip in each, the str column at 0.00 holds or less in every window. The round 1 recordings judged with the head's rules: every correct window passes, every window with the GIL taken fails on the two hold line.

The GIL tests.

• With ScanArrowColumn taking the GIL: test_arrow_str_scan_runs_without_gil fails all 3 parameters at 2.99 to 3.04 holds on the two hold line.
• With the Arrow scan holding the GIL: TestGil fails its three sources without the GIL at 2.21 to 3.13 holds on the head, and at 3.00 to 3.06 on the base; object_frame passes.
• Pinned to one core with 16 busy loops, 4 repeats: TestGil's three sources without the GIL, 12 passed, where round 1 saw 4 failures in 24 under the same load. object_frame as in the finding.

The pull error test.

• CI job "Test abi3: py3.12-manylinux_x86_64" of run 42815296139 on 65403ac: 1 failed, 17585 passed, 133 skipped, 164 xfailed, the failure test_a_python_error_in_a_pull_on_an_engine_thread_fails_the_query[True] - Failed: DID NOT RAISE InvalidInputException; pytest ran without xdist there.
• The race, reproduced with the test's shape outside pytest, four engine threads created on one CPU with busy loops pinned to it and the submitting thread moved to another, 100 runs per form and mode. 32 loops: the old form returned the sum in 1 of 100 runs with the pull outside the GIL and 1 of 100 inside; the new form raised on a worker in 100 of 100 for each, after at most 9,707 pulls on the submitting thread and within 327 ms. 128 loops: the old form returned the sum in 40 of 100 for each mode; the new form raised in 100 of 100 for each, after at most 18,516 pulls and within 655 ms. The description's 99 and 85 of 100 at 32 loops come from a different setup; the direction is the same.
• In no run did a worker raise and the query still return, so the product kept every engine thread error.
• With the pull error dropped: both parameters of the new form fail with DID NOT RAISE in 0.78 s, no hang. With no worker at all (threads = 1, 5 s timeout): the timeout's exception is raised inside the generator on the submitting thread, the query fails with it, and the test fails on no engine thread pulled, so the error was raised on the submitting thread.

Suites.

• The touched files and their siblings at eight workers (test_many_con_same_file.py, test_appender.py, test_pandas_string.py, test_arrow_scan_row_ranges.py, test_execute_contract.py, test_arrow_scan_streams.py, test_windows_abs_path.py): 799 passed, 33 skipped, 8 xfailed in 44.36s.
• The whole of tests/fast at eight workers with a 600 second timeout per test: 1 failed, 17611 passed, 113 skipped, 164 xfailed in 856.68s. The totals match the claimed 17612 passed, 113 skipped, 164 xfailed. The failure is tests/fast/api/test_query_progress.py::TestQueryProgress::test_query_progress, "query finished too quick", a file this change does not touch; it passed 3 of 3 runs alone.
• ruff check without fixing: clean on all six files. ruff format in check mode flags only test_arrow_scan_row_ranges.py:90, which the change does not touch and which the merge base shares; the venv's ruff is 0.17.4, where pre-commit pins 0.13.3.

Outside reviews. Codex, read only, extra high reasoning effort: no findings. The code-review skill, high effort, on this range in this tree: ten items, triaged above.

CI at the time of review, on 73379f0: lint and submodule sanity passed; the sdist, seed wheel, sanity and full API assertion jobs were pending, and the abi3 test legs had not started. No check had failed.

What held

• The str column judged first. A GIL taking str scan fails on the two hold line whatever the calibration says, in the unit test's recorded cases, in every recorded window and in the test itself; a correct build never came near the line.
• The shared measurement. One module, both tests, the same query for the held and the free run, each bound with the statistic that suits its direction; the upper bounds of TestGil no longer fail under the load that broke them in round 1, and they still catch a GIL taking Arrow scan.
• The endless stream. It removes the race without weakening the test: an error on an engine thread must still fail the query, a lost error still fails at once with DID NOT RAISE, and a run in which no worker pulls fails on its own precondition after the timeout.
• One file per test. The shared file no longer collides under xdist, and its tests still check that two connections to one file see each other's tables and that a second configuration is refused. test_windows_abs_path keeps the file and the current drive together.

[el-marvin]

TestGil's object_frame lower bound still fails under a pinned one core load, so the description compares round 1's "4 of 24" with eight clean runs made under a different load.

Severity: minor. Category: docs. Evidence: ran. Fix: containment. Origin: repository. Base: pre-existing.

        if takes_gil:
            assert timings.best_round_in_holds() >= 1, timings.best_round_in_holds()

The description sets the old judgement's "4 of 24 runs" on one core beside sixteen busy loops (my round 1 figure, made by pinning the test and the loops to one core with taskset) against "eight of eight runs under that load" for the new one, which were made under a one CPU quota. The two loads behave differently for this bound. Eight repeats of this parameter each:

• Under a one CPU quota with 16 busy loops in the same scope: 8 passed, as the description says.
• Pinned to one core with 16 busy loops: 1 failed, assert 0.4780491800000003 >= 1, 7 passed.
• The merge base, pinned the same way: 1 failed, assert 0.9154351050000004 >= 1, 7 passed.

In twelve recorded rounds of the same object column and query, pinned, the best of five rounds fell under one hold in 2 of 8 windows at 16 loops (lowest 0.99) and in 3 of 8 at 32 loops (lowest 0.73), where the description says it "reads at least 1.91 holds" over every recorded window. That figure holds for the quota recordings, not for these. The base's best of three fell under one hold in 4 of 10 and 5 of 10 windows of the same recordings, so the change makes this bound more robust, not less.

The cause is the measurement under saturation. When the four scan threads share one core with the loops, a thread that waits for the GIL would have waited for the CPU anyway, so the hold adds little summed scan time to the held run. The str test reads that state as inconclusive and skips (4 of 8 windows of the same recording); this bound fails.

Ask: say in the description which load the eight clean runs and the 1.91 figure come from, and that pinned to one core this bound still fails about one run in eight as the base does. Or, since one round that shows the stall is enough, stop the object_frame loop at the first round that reaches one hold and allow more rounds before failing, which is also faster in the ordinary case.