Repository navigation
fix(conformance): close leaked namespaces in the harness - #134
Merged
Merged
Conversation
Fixes #120. Part of #54. - Wrap statements and metadata so Statement.getConnection() and DatabaseMetaData.getConnection() return the same connection proxy, and closing through either releases the namespace once. - Track the underlying connections per namespace. stop() now fails when a namespace still has an open connection, and reports namespaces whose cleanup failed instead of clearing them silently. Reconcile an escaped raw connection closed through unwrap before declaring a leak. - Treat SQLite file: memory URIs (file::memory:, ?mode=memory) as per-connection namespaces and include the query string in the file namespace identity, so options isolate databases instead of being ignored. - Add XerialHarnessTest: wrapped identity, release-once, stop failure on a leaked connection, swallowed cleanup failure reporting, file memory isolation, and query-string isolation. No production driver code changed. The JSON1Test and CachedRowSetTest connection closes stay in the vendored copies: they leak top-level connections, which the stricter stop() must reject, not connections obtained through statements or metadata. Contract: Hrana 3 is the transport and value source; java.sql defines the connection and wrapper surface; xerial remains the conformance reference. The query string is namespace identity only because sqld has no URL pragma surface. Evidence: - Red: ./mvnw -pl libsql-conformance -am test-compile failed; the harness test could not compile against the old driver/server APIs. - Green: ./mvnw -pl libsql-conformance -am verify -> 123 tests, 0 failures, 1 skipped. - Green: ./mvnw -pl libsql-conformance -am -Pxerial-control verify -> 100 tests, 0 failures, 1 skipped. - Green: XerialHarnessTest -> 8 tests, 0 failures. - Green: ./mvnw -pl libsql-jdbc test -> 235 tests, 0 failures. - Green: ./mvnw test, ./mvnw -pl libsql-jdbc -am package -DskipTests, ./mvnw spotless:check, ./tools/export-upstream --verify.
The full module run interleaves XerialTestServerCleanupTest with the new harness self-tests. The cleanup tests deliberately leave a namespace registered after asserting reuse and cleanup failures, and stopForTest cleared the whole ledger, so the leftover only surfaced when the real session stop ran before the self-test. Reset only the namespaces a test creates, and make stopForTest read the ledger without clearing it. A real leak is no longer masked by a later test, and the full suite leaves no namespace registered. Verified with XerialTestServerCleanupTest alone, XerialHarnessTest alone, and the full conformance verify.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #120. Part of #54.
What changed
Statement.getConnection()andDatabaseMetaData.getConnection()so both return the same connection proxy.getConnection() == connholds, and closing through either releases the namespace once.XerialTestServer.stop()now fails while any namespace still has an open connection, and reports namespaces whose cleanup failed instead of clearing them silently. An escaped raw connection closed throughunwrapis reconciled before the leak check.file:URIs.file::memory:?cache=sharedand?mode=memorymap to fresh per-connection namespaces instead of retained file namespaces. The query string is part of a file namespace's identity, so different options address different databases.XerialHarnessTestwith eight self-tests: wrapped identity through statements and metadata, release-once,stop()failure on a leaked connection, swallowed cleanup-failure reporting, file-memory isolation, query-string isolation, andunwrapidentity.No production driver code changed.
Contract
Hrana 3 is the transport and value source.
java.sqldefines the connection and wrapper surface. xerial/sqlite-jdbc remains the conformance reference. The query string is namespace identity only because sqld exposes no URL pragma surface;file::memory:?cache=sharedis isolated per connection rather than shared across them.Divergences
JSON1TestandCachedRowSetTestconnection closes stay in the vendored copies. Those tests leak top-level connections, which the stricterstop()must reject. The new wrapper handles connections reached through statements and metadata, not a top-level leak whose reference the test still owns.Files outside the plan block
libsql-conformance/src/test/java/com/github/magnusp/libsql/conformance/XerialHarnessTest.javais the harness self-test the plan's unit box requires.Evidence
Base
0e83a5793020d09fa58581477ca9a73ac7bb593e, head12d2c8e0d4bc89927f50ce24d55f45199d8caae3.Red:
./mvnw -pl libsql-conformance -am test-compilefailed: the old harness had noplanorstopForTestseam, and the old mapping treatedfile::memory:?cache=sharedas a retained file path and ignored query strings. Log:/tmp/xerial-parity-i120/red-compile.log.Green:
./mvnw -pl libsql-conformance -am verify-> 123 tests, 0 failures, 1 skipped (/tmp/xerial-parity-i120/lane-2-specific.log)../mvnw -pl libsql-conformance -am -Pxerial-control verify-> 100 tests, 0 failures, 1 skipped (/tmp/xerial-parity-i120/lane-5-control.log)../mvnw -pl libsql-conformance -am -Dit.test=XerialHarnessTest verify-> 8 tests, 0 failures (/tmp/xerial-parity-i120/lane-9-specific.log,/tmp/xerial-parity-i120/lane-3-specific.log)../mvnw -pl libsql-jdbc test-> 235 tests, 0 failures (/tmp/xerial-parity-i120/lane-8-unit.log)../mvnw -pl libsql-conformance -am test-> BUILD SUCCESS (/tmp/xerial-parity-i120/lane-4-module.log)../mvnw test-> BUILD SUCCESS (/tmp/xerial-parity-i120/lane-10-reactor.log)../mvnw -pl libsql-jdbc -am package -DskipTests-> BUILD SUCCESS (/tmp/xerial-parity-i120/lane-package.log)../mvnw spotless:check-> no violations (/tmp/xerial-parity-i120/lane-7-spotless.log)../tools/export-upstream --verify-> verified (/tmp/xerial-parity-i120/lane-6-verify.log).