diff --git a/CHANGELOG.md b/CHANGELOG.md index b77b2e6..0b4e902 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,92 @@ All notable changes to `informix-db`. Versioning is [CalVer](https://calver.org/) — `YYYY.MM.DD` for date-based releases, `YYYY.MM.DD.N` for same-day post-releases per PEP 440. +## 2026.09.02 — A systematic review, and eleven bugs it found + +No new features. This is the result of going back over the driver looking for the *shape* of past bugs rather than for new symptoms, and it turned up more than expected — including two that silently returned wrong data and one where `rollback()` did nothing at all. + +### Rows never checked that they consumed their own payload + +Fourteen framing bugs had reached users before this release. Every one was the same defect — a column reading the wrong number of bytes — and not one of them raised where the mistake happened. The wrong width produced a plausible value and corrupted whatever came *next*, so the damage always surfaced in a different column, a different row, or a different statement. + +All fourteen were detectable for free. The payload is a fully extracted `bytes` of known length, so after decoding N columns the offset has to land exactly on the end. Now it's checked, on every row, in all three decode paths, and a mismatch names the column shape and the byte delta instead of handing back plausible garbage. + +Turning it on found two more bugs on the first run: + +**Smart LOBs were read as a flat 72-byte field.** They actually use the UDT envelope — 149 bytes when populated, 5 when NULL. The driver consumed 72 and left 77 behind, so a `BLOB` or `CLOB` anywhere but the final column position shifted every column after it. The 144 envelope bytes are the 72-byte locator *hex-encoded*, which means `BlobLocator.raw` had never held a locator, only the first half of the hex text. Nobody noticed because `read_blob_column` resolves LOBs server-side and never reads it. + +**NULL composite UDTs skipped their length field**, byte for byte the LVARCHAR NULL bug in a branch twelve lines away. Both are now the one `_read_udt_envelope`, which also rejects a negative length rather than rewinding into bytes it already decoded. + +### `rollback()` could silently do nothing + +`cursor.execute("BEGIN WORK")` is a reasonable thing to write, and with autocommit on it opened a real transaction that the connection never learned about. `commit()` and `rollback()` are both guarded by that flag, so **`rollback()` returned successfully having sent nothing, and the rows it was asked to discard survived.** The connection then went back to the pool holding an open transaction and its locks, because the pool's cleanup is guarded by the same flag. + +With autocommit off it failed instead, and for a sillier reason: the driver's implicit `SQ_BEGIN` fired first, so the caller's `BEGIN WORK` got `-535`, "already in transaction". The driver and the user competing to open the same transaction, and the user losing. + +The server labels these statements and always has. Both spellings of all three now update the connection's state, and the implicit begin is skipped when the caller is doing the job themselves. + +### Five ordinary query forms failed with a nonsense error + +The driver decided whether to open a cursor by checking if the first word of the SQL was `SELECT`. That gets wrong: + +```sql +-- a leading comment → -260 +/* of any of the three flavours */ → -260 +{ that Informix accepts } → -260 +(SELECT parenthesized) → -260 +WITH cte AS (...) SELECT ... → -260 +(SELECT a) UNION (SELECT b) → -260 +``` + +`-260` is "Cursor name already in use", which describes neither the cause nor anything the caller did. It says "cursor" because the driver sent `SQ_EXECUTE` where the server was waiting to open one. + +The server reports the statement type in the first field of every DESCRIBE response, and the driver had been parsing it into a metadata dict that nothing read. The decision now uses it. `EXECUTE FUNCTION` gets fixed as a side effect — it used to run down the DML path and discard its return value, and now yields it. + +### A scrollable cursor took the connection down with it + +Informix gives a session one statement slot. A scrollable cursor holds it open on purpose, so any other statement on that connection got `-285` — **and destroyed the scrollable cursor too**, whose next fetch came back `-267`, "the transaction has been rolled back, all locks released". Two unattributable failures from code that reads as entirely ordinary: iterate a large result set, run a lookup partway through. + +It is now a `ProgrammingError` that explains the constraint and leaves the cursor alone. Re-executing the *same* scrollable cursor is still allowed and now closes its own server-side cursor first, because it collided with itself as well. + +### Statements that failed were sometimes never released + +A failed statement stays allocated server-side and collides with the next `PREPARE`, after which every call on that connection returns a nonsense error pointing at the *previous* SQL. That guard existed at six exits and was missing from two, both easy to reach: + +- The parameterized-`SELECT` bind drain. Passing a string where the column is an `INT` encodes cleanly, so the rejection comes from the server (`-1213`, `-415`) *after* the guarded step. +- Opening a scrollable cursor — no guard at all, and the worst place to lack one, since the GC-time fallback is armed on the line after. + +All six now share one implementation, which also fixed a defect in the copies that had a cursor to close: `CLOSE` and `RELEASE` shared a single suppression, so a failing `CLOSE` skipped the `RELEASE` — losing the half that actually matters. + +### Cleanup could land inside somebody else's statement + +`SQ_CLOSE` and `SQ_RELEASE` act on whatever statement is current; neither names one. Two consequences. + +A cursor finalizer that can't take the wire lock queues its cleanup for the next operation to flush — but that queue was flushed before *every* PDU, and a finalizer queues precisely because another thread is mid-statement. The flush landed inside that thread's own statement and released it (`-208` before the first fetch, `-267` between fetch batches). Flushing only at a statement boundary is both correct and sufficient. + +And the finalizer's own lock probe couldn't see its own thread. `RLock.acquire(blocking=False)` grants a reentrant acquire to the owner, so when GC fired on a thread that was mid-statement, the finalizer concluded it had exclusive access and sent `CLOSE`/`RELEASE` into the running query. Refcounting hides this — a dropped cursor is freed at the drop — but a cursor caught in a reference cycle waits for a collection, and cycles are ordinary. Any traceback holding a cursor makes one. + +A stale queue entry was separately fatal: the leftover `CLOSE` draws `-267`, an `OperationalError`, which was treated as a dead wire and force-closed a perfectly healthy connection. + +### The async layer borrowed the whole process's thread pool + +Every blocking call went through `asyncio.to_thread`, which runs on the event loop's default executor — sized `min(32, cpu_count + 4)`, so six threads on a two-CPU container. + +`asyncio.to_thread` cannot interrupt a worker, so a cancelled await leaves the thread running the wire call until the read timeout. Cancellation is ordinary in a web app; a client disconnect cancels the request task. Measured: six cancelled calls against a six-worker default executor starve an unrelated `to_thread` indefinitely, and with the executor held, **four concurrent driver queries never ran at all**. Pool concurrency was capped by the same unrelated number — `max_size=20` on a two-CPU box ran six queries at a time. + +Each connection now owns one thread. That's the right size rather than a compromise, since the connection serializes every wire operation on its own lock anyway, and it avoids a deadlock a shared pool-sized executor invites. + +### Two readers shared one stream without agreeing + +`IfxSocket` owns a read-ahead buffer that the buffered reader fills, while the login path and the connection-level drain read straight from the socket without looking at it. Buffered bytes would be skipped, and skipped bytes in a length-framed protocol don't announce themselves. + +Nothing triggers it today, because the server sends one response per request. That's a property of the traffic, not of the code — pipelined `executemany` already puts several responses in flight, and the buffer is connection-scoped precisely so read-ahead *can* cross response boundaries. The two paths now agree by construction. + +Also here: `fill_recv_buf` believed whatever byte count it was handed, and that count is almost always a length field straight off the wire — a garbage `0x7FFFFFFF` read as a 2 GB request and sat in `recv` until the read timeout. It now refuses above `IFX_MAX_READ_BYTES` (256 MiB default) with an error naming the number. + +### Verified + +**457** integration tests on 15.0.1.0.3DE and 14.10.FC7W1DE, **456** on 12.10.FC12W1DE (one skip — 12.10 has no CTEs) — up from 414. Every fix has at least one test that fails against the previous release. + ## 2026.09.01 — TLS traffic fuzzed; no bugs found Tests and docs only — no behaviour change. TLS was the last untested surface, and it came back clean. diff --git a/pyproject.toml b/pyproject.toml index 998329c..566dbe4 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -1,6 +1,6 @@ [project] name = "informix-driver" -version = "2026.09.01" +version = "2026.09.02" description = "Pure-Python driver for IBM Informix IDS — speaks the SQLI wire protocol over raw sockets. No CSDK, no JVM, no native libraries." readme = "README.md" license = { text = "MIT" } diff --git a/uv.lock b/uv.lock index 513cbfc..c8a5862 100644 --- a/uv.lock +++ b/uv.lock @@ -34,7 +34,7 @@ wheels = [ [[package]] name = "informix-driver" -version = "2026.9.1" +version = "2026.9.2" source = { editable = "." } [package.optional-dependencies]