Skip to content

libsql: read a cursor response to its end so the connection is reused - #2301

Open
zkochan wants to merge 1 commit into
tursodatabase:mainfrom
zkochan:cursor-read-to-end
Open

zkochan wants to merge 1 commit into
tursodatabase:mainfrom
zkochan:cursor-read-to-end

Conversation

@zkochan

@zkochan zkochan commented Oct 23, 2026

Copy link
Copy Markdown

Fixes #2300.

OwnedCursorStep dropped its Cursor, and the /v3/cursor response body with it, as soon as it read the step_end entry. hyper had not read the final chunk yet, so it closed the connection instead of pooling it. Every query on a remote connection then opened a new TCP connection, and against Turso a new TLS handshake.

After step_end, the step now keeps reading the cursor until the stream ends, then reports no more rows. Entries after step_end are skipped. A read error at that point ends the rows without an error, because the step's rows are already complete.

A caller still has to read the rows to the end (call next() until it returns None) for the connection to be reused. A Rows dropped after its first row still drops the body early.

Testing

  • New unit test owned_step_reads_the_response_to_its_end. It fails without the change.
  • Against a local libsql-server, 50 query("SELECT 1") calls read to the end opened 50 connections before and 2 after.

Written by an agent (Claude Code, claude-opus-5-5).

OwnedCursorStep dropped its cursor, and the HTTP response body with it,
on the step_end entry. hyper had not read the final chunk, so it closed
the connection, and every remote query opened a new one. The step now
reads the rest of the response before it reports no more rows.

Fixes tursodatabase#2300

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
zkochan added a commit to pnpm/pnpm that referenced this pull request Oct 23, 2026
Every token lookup against a remote libsql database opened a new TCP
connection, and against Turso a new TLS handshake. A query's rows come
from a chunked `/v3/cursor` response. libsql dropped the response body
when it read the `step_end` entry, before hyper had read the final
chunk, so hyper closed the connection instead of pooling it. pnpr's
lookups also dropped their rows after the first one.

- vendor/libsql carries libsql 0.9.30 with the fix from
  tursodatabase/libsql#2301: after `step_end` the step reads the
  response to its end. `src/lib.rs` allows warnings, because cargo does
  not cap the lints of path dependencies and the workspace builds with
  `-D warnings`.
- The auth backend reads every single-row query through `first_row`,
  which reads the row's values and then the rest of the rows. A local
  row reads the live statement, so its values are taken first.
- The pre-commit formatter skips `vendor/`, so a vendored crate keeps
  its upstream formatting and edition.

Against a local libsql-server, 200 authenticated requests opened 193
new connections before and none after.

Closes pnpm/tasks#192

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
zkochan added a commit to pnpm/pnpm that referenced this pull request Oct 23, 2026
Every token lookup against a remote libsql database opened a new TCP
connection, and against Turso a new TLS handshake. A query's rows come
from a chunked `/v3/cursor` response. libsql dropped the response body
when it read the `step_end` entry, before hyper had read the final
chunk, so hyper closed the connection instead of pooling it. pnpr's
lookups also dropped their rows after the first one.

- vendor/libsql carries libsql 0.9.30 with the fix from
  tursodatabase/libsql#2301: after `step_end` the step reads the
  response to its end. `src/lib.rs` allows warnings, because cargo does
  not cap the lints of path dependencies and the workspace builds with
  `-D warnings`.
- The auth backend reads every single-row query through `first_row`,
  which reads the row's values and then the rest of the rows. A local
  row reads the live statement, so its values are taken first.
- The pre-commit formatter skips `vendor/`, so a vendored crate keeps
  its upstream formatting and edition.

Against a local libsql-server, 200 authenticated requests opened 193
new connections before and none after.

Closes pnpm/tasks#192

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
zkochan added a commit to pnpm/pnpm that referenced this pull request Oct 24, 2026
The pnpm-0.9.30 branch of pnpm/libsql is the 0.9.30 release commit plus
the cursor fix from tursodatabase/libsql#2301. `[patch.crates-io]` pins
it by revision, as it does pnpm/node-semver-rs, so the repository no
longer carries a copy of the crate. Cargo caps the lints of git
dependencies, so the fork needs no allow-warnings change.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
zkochan added a commit to pnpm/pnpm that referenced this pull request Oct 24, 2026
Every token lookup against a remote libsql database opened a new TCP
connection, and against Turso a new TLS handshake. A query's rows come
from a chunked `/v3/cursor` response. libsql dropped the response body
when it read the `step_end` entry, before hyper had read the final
chunk, so hyper closed the connection instead of pooling it. pnpr's
lookups also dropped their rows after the first one.

- libsql comes from the pnpm-0.9.30 branch of pnpm/libsql: the 0.9.30
  release plus the fix from tursodatabase/libsql#2301, after which a
  step reads the cursor response to its end.
- The auth backend reads every single-row query through `first_row`,
  which reads the row's values and then the rest of the rows. A local
  row reads the live statement, so its values are taken first.

Against a local libsql-server, 200 authenticated requests opened 193
new connections before and none after.

Part of pnpm/tasks#192
zkochan added a commit to pnpm/pnpm that referenced this pull request Oct 26, 2026
Windows Rust CI aborted pnpr-auth libsql tests with 0xc0000005 (four
times recently: registration_cap_is_strict_across_backend_instances
three times, sync_server::bulk_revocation_refreshes_a_replica_whose_tokens_went_elsewhere
once).

libsql's LibsqlConnection::drop calls disconnect() and then the inner
local::Connection's own Drop calls disconnect() again. Both see the
drop_ref Arc as unique, so sqlite3_close_v2 runs a second time on a
handle SQLite already freed. Usually it reads freed memory as "closed"
and reports misuse. When another thread reused that memory, it read
garbage and crashed. The tests that crashed run several connections
concurrently, which makes reuse likely.

Upstream fixed it in tursodatabase/libsql#2261 (released in 0.10.0),
which nulls the handle after the first close. pnpr stays on the pnpm
fork because tursodatabase/libsql#2301 is still unreleased, so the fix
is cherry-picked onto the fork's pnpm-0.9.30 branch.

The new test installs an SQLite log callback, drops one connection and
asserts no misuse was logged. It logged two before the fork bump.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
zkochan added a commit to pnpm/pnpm that referenced this pull request Oct 26, 2026
Windows Rust CI aborted pnpr-auth libsql tests with 0xc0000005 (four
times recently: registration_cap_is_strict_across_backend_instances
three times, sync_server::bulk_revocation_refreshes_a_replica_whose_tokens_went_elsewhere
once).

libsql's LibsqlConnection::drop calls disconnect() and then the inner
local::Connection's own Drop calls disconnect() again. Both see the
drop_ref Arc as unique, so sqlite3_close_v2 runs a second time on a
handle SQLite already freed. Usually it reads freed memory as "closed"
and reports misuse. When another thread reused that memory, it read
garbage and crashed. The tests that crashed run several connections
concurrently, which makes reuse likely.

Upstream fixed it in tursodatabase/libsql#2261 (released in 0.10.0),
which nulls the handle after the first close. pnpr stays on the pnpm
fork because tursodatabase/libsql#2301 is still unreleased, so the fix
is cherry-picked onto the fork's pnpm-0.9.30 branch.

The new test installs an SQLite log callback, drops one connection and
asserts no misuse was logged. It logged two before the fork bump.

Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>

This branch has not been deployed

No deployments
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.

Remote queries open a new HTTP connection for every query

1 participant