Never do socket I/O in finalizers; map ssl_mode onto real Connector/C options - #241

Merged
quinnj merged 1 commit into
mainfrom
jq/no-finalizer-io
Aug 2, 2026
Merged

Never do socket I/O in finalizers; map ssl_mode onto real Connector/C options#241
quinnj merged 1 commit into
mainfrom
jq/no-finalizer-io

Conversation

@quinnj

Copy link
Copy Markdown
Member

Fixes#220. Fixes#240. Related: #234.

Two fixes, both rooted in the investigation written up in this comment on #220 and in #240.

1. Finalizers no longer do socket I/O (#220)

The API.MYSQL_STMT finalizer called mysql_stmt_close, which sends COM_STMT_CLOSE over the connection's socket. Finalizers run on whatever thread triggers GC — concurrently with an in-flight mysql_* call on another thread — and a MYSQL* is not thread-safe, so an abandoned statement's finalizer raced legitimate, even lock-serialized, use of the same connection. Under TLS the two threads interleave inside one SSL*, corrupting OpenSSL state: bad record mac errors, wedged connections, and the double-free aborts reported in #220 (crash reports show the GC-finalizer thread and a mysql_commit thread simultaneously inside ma_tls_close → SSL_free). API.MYSQL_RES (mysql_free_result reads un-fetched rows off the wire) and API.MYSQL (mysql_close sends COM_QUIT) had the same problem.

The invariant this PR establishes: finalizers never touch the socket.

  • MYSQL now owns a small reap queue (Threads.SpinLock + two Vector{Ptr{Cvoid}}). Statement and result wrappers keep a reference to their parent MYSQL.
  • The MYSQL_STMT/MYSQL_RES finalizers only park their raw handle on the queue (no libmariadb call) and are gone. If the connection is already closed, the statement finalizer calls mysql_stmt_close directly — mysql_close has invalidated the handles, so that's a purely local free; an un-drained result in that state is leaked rather than read through a freed MYSQL*.
  • API.reap!(mysql) drains the queue — it runs at the top of clear!(conn), i.e. inside DBInterface.prepare/DBInterface.execute/cursor operations, which the caller already serializes with all other use of the connection. The socket-touching closes happen there, outside the spinlock.
  • The MYSQL finalizer performs the full teardown (free parked results, close parked statements, mysql_close) — safe because an unreachable connection wrapper has no in-flight calls. All lock acquisition in finalizers uses the manual's trylock-or-re-register pattern, so a finalizer can never deadlock against a thread holding the reap lock.
  • Explicit paths (DBInterface.close!(stmt), DBInterface.close!(conn), clear!'s result cleanup) now call immediate API.close!/API.free! instead of finalize(...), preserving their old eager semantics; the still-registered finalizer no-ops once ptr is C_NULL.

Validation

The reproducer from the #220 comment (6 tasks doing prepared-statement work behind one ReentrantLock, one thread applying GC pressure, -t 8, mysql:8 in Docker, TLS on):

  • before: SIGABRT (malloc: double free in SSL_write/SSL_free) or SIGSEGV on every run within 1–4 twenty-second rounds, plus (2026): TLS/SSL error: ssl/tls alert bad record mac rounds;
  • after: 8/8 rounds clean with TLS confirmed active (TLS_AES_256_GCM_SHA384), no errors of any kind.

New tests assert that abandoned statements are parked (not closed mid-GC) and reaped by the next operation, that closing a connection with parked handles is safe, and (when JULIA_NUM_THREADS > 1) run a 5-second lock-serialized concurrency smoke test that aborts the process on the old code.

2. ssl_mode mapped onto options libmariadb actually has (#240)

API.MYSQL_OPT_SSL_MODE does not exist in libmariadb — the enum entry's ordinal (7025) collided with MARIADB_OPT_SKIP_READ_RESPONSE, so ssl_mode=SSL_MODE_DISABLED was a silent no-op and any other mode set skip-read-response to true, corrupting the protocol. The entry is removed from mysql_option (technically breaking for direct API users, but every possible use was a bug), and the ssl_mode keyword now maps onto real Connector/C options:

modeeffect
SSL_MODE_DISABLED@warn (libmariadb 3.4+ cannot disable TLS client-side; see #240)
SSL_MODE_PREFERREDno-op (Connector/C default)
SSL_MODE_REQUIREDMYSQL_OPT_SSL_ENFORCE = true
SSL_MODE_VERIFY_CA / SSL_MODE_VERIFY_IDENTITYMYSQL_OPT_SSL_ENFORCE = true + MYSQL_OPT_SSL_VERIFY_SERVER_CERT = true

The mapping runs after the ssl_verify_server_cert/ssl_enforce blocks so an explicit mode wins over the ssl_verify_server_cert=false default from #235. The ssl_mode keyword is now documented in the connect docstring.

One observation for a follow-up decision

While validating on current main I noticed that with the #235 default ssl_verify_server_cert=false, connections to a TLS-capable server negotiate plaintext (libmariadb 3.4's TLS-by-default only engages when certificate verification is on — verified empirically: Ssl_cipher is empty by default, and non-empty with ssl_verify_server_cert=true, ssl_enforce=true, or ssl_mode=SSL_MODE_REQUIRED). This PR leaves that default untouched, but it may be worth calling out in the README/docs since 1.5.1 behavior (TLS on by default with the 3.4 jll) differs.

Version bumped to 1.5.3.

🤖 Generated with Claude Code

… options
MYSQL_STMT/MYSQL_RES finalizers sent COM_STMT_CLOSE / read pending rows over
the connection's socket from whatever thread triggered GC, racing in-flight
mysql_* calls on other threads and corrupting TLS state (double-free aborts,
bad record mac). Finalizers now park raw handles on a connection-owned reap
queue drained inside the next user-initiated (caller-serialized) operation;
connection teardown drains the queue before mysql_close.
MYSQL_OPT_SSL_MODE does not exist in libmariadb (its ordinal collided with
MARIADB_OPT_SKIP_READ_RESPONSE); the ssl_mode keyword is now mapped onto
MYSQL_OPT_SSL_ENFORCE / MYSQL_OPT_SSL_VERIFY_SERVER_CERT, with a warning for
the unimplementable SSL_MODE_DISABLED.
Fixes#220Fixes#240
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@codecov

codecovBot commented Aug 2, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 97.11538% with 3 lines in your changes missing coverage. Please review.
✅ Project coverage is 71.50%. Comparing base (d69e2d6) to head (1ccda90).

Files with missing linesPatch %Lines
src/api/apitypes.jl96.51%3 Missing ⚠️
Additional details and impacted files
@@ Coverage Diff @@## main #241 +/- ##
==========================================
+ Coverage 69.89% 71.50% +1.60% 
==========================================
Files 10 10 Lines 1186 1260 +74 ==========================================
+ Hits 829 901 +72 - Misses 357 359 +2 

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@quinnj
quinnj merged commit 6144a81 into mainAug 2, 2026
6 checks passed
@quinnj
quinnj deleted the jq/no-finalizer-io branch August 2, 2026 05:35
Sign up for freeto 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.

ssl_mode maps to libmariadb's MARIADB_OPT_SKIP_READ_RESPONSE: SSL_MODE_DISABLED is a no-op, other modes corrupt the protocol Double free in mariadb

1 participant

@quinnj
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Add copy buttons to all
 blocks\n(function() {\n function addCopyButtons() {\n document.querySelectorAll('pre code').forEach(function(codeBlock) {\n if (codeBlock.parentElement.hasAttribute('data-copy-added')) return;\n codeBlock.parentElement.setAttribute('data-copy-added', 'true');\n \n var btn = document.createElement('button');\n btn.textContent = 'Copy';\n btn.style.cssText = 'position:absolute;top:4px;right:4px;padding:2px 8px;font-size:11px;background:#4ecdc4;border:none;border-radius:4px;color:#1a1a2e;cursor:pointer;opacity:0.7;transition:opacity 0.2s;';\n btn.onmouseover = function() { this.style.opacity = '1'; };\n btn.onmouseout = function() { this.style.opacity = '0.7'; };\n btn.onclick = function() {\n navigator.clipboard.writeText(codeBlock.textContent).then(function() {\n btn.textContent = 'Copied!';\n setTimeout(function() { btn.textContent = 'Copy'; }, 1500);\n });\n };\n codeBlock.parentElement.style.position = 'relative';\n codeBlock.parentElement.appendChild(btn);\n });\n }\n \n addCopyButtons();\n \n // Re-run on dynamic content\n var observer = new MutationObserver(addCopyButtons);\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Add Copy Buttons to Code Blocks");
}
} catch(__e) { console.warn('[Userscript:Add Copy Buttons to Code Blocks]', __e); }
})();
(function(){
try {
var __m = "github.com";
var __re = new RegExp('^' + "github\\.com" + '
Skip to content

Never do socket I/O in finalizers; map ssl_mode onto real Connector/C options - #241

Merged
quinnj merged 1 commit into
mainfrom
jq/no-finalizer-io
Aug 2, 2026
Merged

Never do socket I/O in finalizers; map ssl_mode onto real Connector/C options#241
quinnj merged 1 commit into
mainfrom
jq/no-finalizer-io

Conversation

@quinnj

Copy link
Copy Markdown
Member

Fixes#220. Fixes#240. Related: #234.

Two fixes, both rooted in the investigation written up in this comment on #220 and in #240.

1. Finalizers no longer do socket I/O (#220)

The API.MYSQL_STMT finalizer called mysql_stmt_close, which sends COM_STMT_CLOSE over the connection's socket. Finalizers run on whatever thread triggers GC — concurrently with an in-flight mysql_* call on another thread — and a MYSQL* is not thread-safe, so an abandoned statement's finalizer raced legitimate, even lock-serialized, use of the same connection. Under TLS the two threads interleave inside one SSL*, corrupting OpenSSL state: bad record mac errors, wedged connections, and the double-free aborts reported in #220 (crash reports show the GC-finalizer thread and a mysql_commit thread simultaneously inside ma_tls_close → SSL_free). API.MYSQL_RES (mysql_free_result reads un-fetched rows off the wire) and API.MYSQL (mysql_close sends COM_QUIT) had the same problem.

The invariant this PR establishes: finalizers never touch the socket.

  • MYSQL now owns a small reap queue (Threads.SpinLock + two Vector{Ptr{Cvoid}}). Statement and result wrappers keep a reference to their parent MYSQL.
  • The MYSQL_STMT/MYSQL_RES finalizers only park their raw handle on the queue (no libmariadb call) and are gone. If the connection is already closed, the statement finalizer calls mysql_stmt_close directly — mysql_close has invalidated the handles, so that's a purely local free; an un-drained result in that state is leaked rather than read through a freed MYSQL*.
  • API.reap!(mysql) drains the queue — it runs at the top of clear!(conn), i.e. inside DBInterface.prepare/DBInterface.execute/cursor operations, which the caller already serializes with all other use of the connection. The socket-touching closes happen there, outside the spinlock.
  • The MYSQL finalizer performs the full teardown (free parked results, close parked statements, mysql_close) — safe because an unreachable connection wrapper has no in-flight calls. All lock acquisition in finalizers uses the manual's trylock-or-re-register pattern, so a finalizer can never deadlock against a thread holding the reap lock.
  • Explicit paths (DBInterface.close!(stmt), DBInterface.close!(conn), clear!'s result cleanup) now call immediate API.close!/API.free! instead of finalize(...), preserving their old eager semantics; the still-registered finalizer no-ops once ptr is C_NULL.

Validation

The reproducer from the #220 comment (6 tasks doing prepared-statement work behind one ReentrantLock, one thread applying GC pressure, -t 8, mysql:8 in Docker, TLS on):

  • before: SIGABRT (malloc: double free in SSL_write/SSL_free) or SIGSEGV on every run within 1–4 twenty-second rounds, plus (2026): TLS/SSL error: ssl/tls alert bad record mac rounds;
  • after: 8/8 rounds clean with TLS confirmed active (TLS_AES_256_GCM_SHA384), no errors of any kind.

New tests assert that abandoned statements are parked (not closed mid-GC) and reaped by the next operation, that closing a connection with parked handles is safe, and (when JULIA_NUM_THREADS > 1) run a 5-second lock-serialized concurrency smoke test that aborts the process on the old code.

2. ssl_mode mapped onto options libmariadb actually has (#240)

API.MYSQL_OPT_SSL_MODE does not exist in libmariadb — the enum entry's ordinal (7025) collided with MARIADB_OPT_SKIP_READ_RESPONSE, so ssl_mode=SSL_MODE_DISABLED was a silent no-op and any other mode set skip-read-response to true, corrupting the protocol. The entry is removed from mysql_option (technically breaking for direct API users, but every possible use was a bug), and the ssl_mode keyword now maps onto real Connector/C options:

modeeffect
SSL_MODE_DISABLED@warn (libmariadb 3.4+ cannot disable TLS client-side; see #240)
SSL_MODE_PREFERREDno-op (Connector/C default)
SSL_MODE_REQUIREDMYSQL_OPT_SSL_ENFORCE = true
SSL_MODE_VERIFY_CA / SSL_MODE_VERIFY_IDENTITYMYSQL_OPT_SSL_ENFORCE = true + MYSQL_OPT_SSL_VERIFY_SERVER_CERT = true

The mapping runs after the ssl_verify_server_cert/ssl_enforce blocks so an explicit mode wins over the ssl_verify_server_cert=false default from #235. The ssl_mode keyword is now documented in the connect docstring.

One observation for a follow-up decision

While validating on current main I noticed that with the #235 default ssl_verify_server_cert=false, connections to a TLS-capable server negotiate plaintext (libmariadb 3.4's TLS-by-default only engages when certificate verification is on — verified empirically: Ssl_cipher is empty by default, and non-empty with ssl_verify_server_cert=true, ssl_enforce=true, or ssl_mode=SSL_MODE_REQUIRED). This PR leaves that default untouched, but it may be worth calling out in the README/docs since 1.5.1 behavior (TLS on by default with the 3.4 jll) differs.

Version bumped to 1.5.3.

🤖 Generated with Claude Code

… options
MYSQL_STMT/MYSQL_RES finalizers sent COM_STMT_CLOSE / read pending rows over
the connection's socket from whatever thread triggered GC, racing in-flight
mysql_* calls on other threads and corrupting TLS state (double-free aborts,
bad record mac). Finalizers now park raw handles on a connection-owned reap
queue drained inside the next user-initiated (caller-serialized) operation;
connection teardown drains the queue before mysql_close.
MYSQL_OPT_SSL_MODE does not exist in libmariadb (its ordinal collided with
MARIADB_OPT_SKIP_READ_RESPONSE); the ssl_mode keyword is now mapped onto
MYSQL_OPT_SSL_ENFORCE / MYSQL_OPT_SSL_VERIFY_SERVER_CERT, with a warning for
the unimplementable SSL_MODE_DISABLED.
Fixes#220Fixes#240
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@codecov

codecovBot commented Aug 2, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 97.11538% with 3 lines in your changes missing coverage. Please review.
✅ Project coverage is 71.50%. Comparing base (d69e2d6) to head (1ccda90).

Files with missing linesPatch %Lines
src/api/apitypes.jl96.51%3 Missing ⚠️
Additional details and impacted files
@@ Coverage Diff @@## main #241 +/- ##
==========================================
+ Coverage 69.89% 71.50% +1.60% 
==========================================
Files 10 10 Lines 1186 1260 +74 ==========================================
+ Hits 829 901 +72 - Misses 357 359 +2 

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@quinnj
quinnj merged commit 6144a81 into mainAug 2, 2026
6 checks passed
@quinnj
quinnj deleted the jq/no-finalizer-io branch August 2, 2026 05:35
Sign up for freeto 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.

ssl_mode maps to libmariadb's MARIADB_OPT_SKIP_READ_RESPONSE: SSL_MODE_DISABLED is a no-op, other modes corrupt the protocol Double free in mariadb

1 participant

@quinnj
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Force GitHub README to respect dark mode\n(function() {\n var style = document.createElement('style');\n style.textContent = '\n .markdown-body {\n color-scheme: dark light;\n }\n .markdown-body pre { background: #161b22 !important; }\n .markdown-body code { background: rgba(110, 118, 129, 0.4) !important; }\n .markdown-body table th, .markdown-body table td { border-color: #30363d !important; }\n .markdown-body img { background: #0d1117; }\n .markdown-body blockquote { border-left-color: #8b949e; }\n .markdown-body hr { border-color: #30363d; }\n ';\n document.head.appendChild(style);\n})();", "GitHub Dark Mode README Fix"); } } catch(__e) { console.warn('[Userscript:GitHub Dark Mode README Fix]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

Never do socket I/O in finalizers; map ssl_mode onto real Connector/C options - #241

Merged
quinnj merged 1 commit into
mainfrom
jq/no-finalizer-io
Aug 2, 2026
Merged

Never do socket I/O in finalizers; map ssl_mode onto real Connector/C options#241
quinnj merged 1 commit into
mainfrom
jq/no-finalizer-io

Conversation

@quinnj

Copy link
Copy Markdown
Member

Fixes#220. Fixes#240. Related: #234.

Two fixes, both rooted in the investigation written up in this comment on #220 and in #240.

1. Finalizers no longer do socket I/O (#220)

The API.MYSQL_STMT finalizer called mysql_stmt_close, which sends COM_STMT_CLOSE over the connection's socket. Finalizers run on whatever thread triggers GC — concurrently with an in-flight mysql_* call on another thread — and a MYSQL* is not thread-safe, so an abandoned statement's finalizer raced legitimate, even lock-serialized, use of the same connection. Under TLS the two threads interleave inside one SSL*, corrupting OpenSSL state: bad record mac errors, wedged connections, and the double-free aborts reported in #220 (crash reports show the GC-finalizer thread and a mysql_commit thread simultaneously inside ma_tls_close → SSL_free). API.MYSQL_RES (mysql_free_result reads un-fetched rows off the wire) and API.MYSQL (mysql_close sends COM_QUIT) had the same problem.

The invariant this PR establishes: finalizers never touch the socket.

  • MYSQL now owns a small reap queue (Threads.SpinLock + two Vector{Ptr{Cvoid}}). Statement and result wrappers keep a reference to their parent MYSQL.
  • The MYSQL_STMT/MYSQL_RES finalizers only park their raw handle on the queue (no libmariadb call) and are gone. If the connection is already closed, the statement finalizer calls mysql_stmt_close directly — mysql_close has invalidated the handles, so that's a purely local free; an un-drained result in that state is leaked rather than read through a freed MYSQL*.
  • API.reap!(mysql) drains the queue — it runs at the top of clear!(conn), i.e. inside DBInterface.prepare/DBInterface.execute/cursor operations, which the caller already serializes with all other use of the connection. The socket-touching closes happen there, outside the spinlock.
  • The MYSQL finalizer performs the full teardown (free parked results, close parked statements, mysql_close) — safe because an unreachable connection wrapper has no in-flight calls. All lock acquisition in finalizers uses the manual's trylock-or-re-register pattern, so a finalizer can never deadlock against a thread holding the reap lock.
  • Explicit paths (DBInterface.close!(stmt), DBInterface.close!(conn), clear!'s result cleanup) now call immediate API.close!/API.free! instead of finalize(...), preserving their old eager semantics; the still-registered finalizer no-ops once ptr is C_NULL.

Validation

The reproducer from the #220 comment (6 tasks doing prepared-statement work behind one ReentrantLock, one thread applying GC pressure, -t 8, mysql:8 in Docker, TLS on):

  • before: SIGABRT (malloc: double free in SSL_write/SSL_free) or SIGSEGV on every run within 1–4 twenty-second rounds, plus (2026): TLS/SSL error: ssl/tls alert bad record mac rounds;
  • after: 8/8 rounds clean with TLS confirmed active (TLS_AES_256_GCM_SHA384), no errors of any kind.

New tests assert that abandoned statements are parked (not closed mid-GC) and reaped by the next operation, that closing a connection with parked handles is safe, and (when JULIA_NUM_THREADS > 1) run a 5-second lock-serialized concurrency smoke test that aborts the process on the old code.

2. ssl_mode mapped onto options libmariadb actually has (#240)

API.MYSQL_OPT_SSL_MODE does not exist in libmariadb — the enum entry's ordinal (7025) collided with MARIADB_OPT_SKIP_READ_RESPONSE, so ssl_mode=SSL_MODE_DISABLED was a silent no-op and any other mode set skip-read-response to true, corrupting the protocol. The entry is removed from mysql_option (technically breaking for direct API users, but every possible use was a bug), and the ssl_mode keyword now maps onto real Connector/C options:

modeeffect
SSL_MODE_DISABLED@warn (libmariadb 3.4+ cannot disable TLS client-side; see #240)
SSL_MODE_PREFERREDno-op (Connector/C default)
SSL_MODE_REQUIREDMYSQL_OPT_SSL_ENFORCE = true
SSL_MODE_VERIFY_CA / SSL_MODE_VERIFY_IDENTITYMYSQL_OPT_SSL_ENFORCE = true + MYSQL_OPT_SSL_VERIFY_SERVER_CERT = true

The mapping runs after the ssl_verify_server_cert/ssl_enforce blocks so an explicit mode wins over the ssl_verify_server_cert=false default from #235. The ssl_mode keyword is now documented in the connect docstring.

One observation for a follow-up decision

While validating on current main I noticed that with the #235 default ssl_verify_server_cert=false, connections to a TLS-capable server negotiate plaintext (libmariadb 3.4's TLS-by-default only engages when certificate verification is on — verified empirically: Ssl_cipher is empty by default, and non-empty with ssl_verify_server_cert=true, ssl_enforce=true, or ssl_mode=SSL_MODE_REQUIRED). This PR leaves that default untouched, but it may be worth calling out in the README/docs since 1.5.1 behavior (TLS on by default with the 3.4 jll) differs.

Version bumped to 1.5.3.

🤖 Generated with Claude Code

… options
MYSQL_STMT/MYSQL_RES finalizers sent COM_STMT_CLOSE / read pending rows over
the connection's socket from whatever thread triggered GC, racing in-flight
mysql_* calls on other threads and corrupting TLS state (double-free aborts,
bad record mac). Finalizers now park raw handles on a connection-owned reap
queue drained inside the next user-initiated (caller-serialized) operation;
connection teardown drains the queue before mysql_close.
MYSQL_OPT_SSL_MODE does not exist in libmariadb (its ordinal collided with
MARIADB_OPT_SKIP_READ_RESPONSE); the ssl_mode keyword is now mapped onto
MYSQL_OPT_SSL_ENFORCE / MYSQL_OPT_SSL_VERIFY_SERVER_CERT, with a warning for
the unimplementable SSL_MODE_DISABLED.
Fixes#220Fixes#240
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@codecov

codecovBot commented Aug 2, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 97.11538% with 3 lines in your changes missing coverage. Please review.
✅ Project coverage is 71.50%. Comparing base (d69e2d6) to head (1ccda90).

Files with missing linesPatch %Lines
src/api/apitypes.jl96.51%3 Missing ⚠️
Additional details and impacted files
@@ Coverage Diff @@## main #241 +/- ##
==========================================
+ Coverage 69.89% 71.50% +1.60% 
==========================================
Files 10 10 Lines 1186 1260 +74 ==========================================
+ Hits 829 901 +72 - Misses 357 359 +2 

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@quinnj
quinnj merged commit 6144a81 into mainAug 2, 2026
6 checks passed
@quinnj
quinnj deleted the jq/no-finalizer-io branch August 2, 2026 05:35
Sign up for freeto 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.

ssl_mode maps to libmariadb's MARIADB_OPT_SKIP_READ_RESPONSE: SSL_MODE_DISABLED is a no-op, other modes corrupt the protocol Double free in mariadb

1 participant

@quinnj
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Highlight search terms from Google/DuckDuckGo/Bing referrer\n(function() {\n var ref = document.referrer;\n var terms = [];\n \n if (ref.includes('google.com') || ref.includes('duckduckgo.com') || ref.includes('bing.com')) {\n var url = new URL(ref);\n var q = url.searchParams.get('q') || url.searchParams.get('p');\n if (q) {\n terms = q.split(/\\s+/).filter(function(t) { return t.length > 2; });\n }\n }\n \n if (terms.length === 0) return;\n \n var style = document.createElement('style');\n style.textContent = '.userscript-highlight { background: #fbbf24; color: #1a1a2e; padding: 1px 3px; border-radius: 2px; }';\n document.head.appendChild(style);\n \n function highlight(node) {\n if (node.nodeType === 3) { // text node\n var text = node.textContent;\n var found = false;\n terms.forEach(function(term) {\n var regex = new RegExp('(' + term.replace(/[.*+?^${}()|[\\]\\\\]/g, '\\\\') + ')', 'gi');\n if (regex.test(text)) {\n found = true;\n var frag = document.createDocumentFragment();\n var parts = text.split(regex);\n parts.forEach(function(part, i) {\n if (i % 2 === 0) {\n frag.appendChild(document.createTextNode(part));\n } else {\n var span = document.createElement('span');\n span.className = 'userscript-highlight';\n span.textContent = part;\n frag.appendChild(span);\n }\n });\n node.parentNode.replaceChild(frag, node);\n }\n });\n } else if (node.nodeType === 1 && node.childNodes) { // element\n var skipTags = ['SCRIPT', 'STYLE', 'NOSCRIPT', 'TEXTAREA', 'INPUT', 'SELECT'];\n if (!skipTags.includes(node.tagName)) {\n Array.from(node.childNodes).forEach(highlight);\n }\n }\n }\n \n highlight(document.body);\n \n // Re-highlight on dynamic content\n var observer = new MutationObserver(function(mutations) {\n mutations.forEach(function(m) {\n m.addedNodes.forEach(function(node) {\n if (node.nodeType === 1 || node.nodeType === 3) highlight(node);\n });\n });\n });\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Highlight Search Terms"); } } catch(__e) { console.warn('[Userscript:Highlight Search Terms]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

Never do socket I/O in finalizers; map ssl_mode onto real Connector/C options - #241

Merged
quinnj merged 1 commit into
mainfrom
jq/no-finalizer-io
Aug 2, 2026
Merged

Never do socket I/O in finalizers; map ssl_mode onto real Connector/C options#241
quinnj merged 1 commit into
mainfrom
jq/no-finalizer-io

Conversation

@quinnj

Copy link
Copy Markdown
Member

Fixes#220. Fixes#240. Related: #234.

Two fixes, both rooted in the investigation written up in this comment on #220 and in #240.

1. Finalizers no longer do socket I/O (#220)

The API.MYSQL_STMT finalizer called mysql_stmt_close, which sends COM_STMT_CLOSE over the connection's socket. Finalizers run on whatever thread triggers GC — concurrently with an in-flight mysql_* call on another thread — and a MYSQL* is not thread-safe, so an abandoned statement's finalizer raced legitimate, even lock-serialized, use of the same connection. Under TLS the two threads interleave inside one SSL*, corrupting OpenSSL state: bad record mac errors, wedged connections, and the double-free aborts reported in #220 (crash reports show the GC-finalizer thread and a mysql_commit thread simultaneously inside ma_tls_close → SSL_free). API.MYSQL_RES (mysql_free_result reads un-fetched rows off the wire) and API.MYSQL (mysql_close sends COM_QUIT) had the same problem.

The invariant this PR establishes: finalizers never touch the socket.

  • MYSQL now owns a small reap queue (Threads.SpinLock + two Vector{Ptr{Cvoid}}). Statement and result wrappers keep a reference to their parent MYSQL.
  • The MYSQL_STMT/MYSQL_RES finalizers only park their raw handle on the queue (no libmariadb call) and are gone. If the connection is already closed, the statement finalizer calls mysql_stmt_close directly — mysql_close has invalidated the handles, so that's a purely local free; an un-drained result in that state is leaked rather than read through a freed MYSQL*.
  • API.reap!(mysql) drains the queue — it runs at the top of clear!(conn), i.e. inside DBInterface.prepare/DBInterface.execute/cursor operations, which the caller already serializes with all other use of the connection. The socket-touching closes happen there, outside the spinlock.
  • The MYSQL finalizer performs the full teardown (free parked results, close parked statements, mysql_close) — safe because an unreachable connection wrapper has no in-flight calls. All lock acquisition in finalizers uses the manual's trylock-or-re-register pattern, so a finalizer can never deadlock against a thread holding the reap lock.
  • Explicit paths (DBInterface.close!(stmt), DBInterface.close!(conn), clear!'s result cleanup) now call immediate API.close!/API.free! instead of finalize(...), preserving their old eager semantics; the still-registered finalizer no-ops once ptr is C_NULL.

Validation

The reproducer from the #220 comment (6 tasks doing prepared-statement work behind one ReentrantLock, one thread applying GC pressure, -t 8, mysql:8 in Docker, TLS on):

  • before: SIGABRT (malloc: double free in SSL_write/SSL_free) or SIGSEGV on every run within 1–4 twenty-second rounds, plus (2026): TLS/SSL error: ssl/tls alert bad record mac rounds;
  • after: 8/8 rounds clean with TLS confirmed active (TLS_AES_256_GCM_SHA384), no errors of any kind.

New tests assert that abandoned statements are parked (not closed mid-GC) and reaped by the next operation, that closing a connection with parked handles is safe, and (when JULIA_NUM_THREADS > 1) run a 5-second lock-serialized concurrency smoke test that aborts the process on the old code.

2. ssl_mode mapped onto options libmariadb actually has (#240)

API.MYSQL_OPT_SSL_MODE does not exist in libmariadb — the enum entry's ordinal (7025) collided with MARIADB_OPT_SKIP_READ_RESPONSE, so ssl_mode=SSL_MODE_DISABLED was a silent no-op and any other mode set skip-read-response to true, corrupting the protocol. The entry is removed from mysql_option (technically breaking for direct API users, but every possible use was a bug), and the ssl_mode keyword now maps onto real Connector/C options:

modeeffect
SSL_MODE_DISABLED@warn (libmariadb 3.4+ cannot disable TLS client-side; see #240)
SSL_MODE_PREFERREDno-op (Connector/C default)
SSL_MODE_REQUIREDMYSQL_OPT_SSL_ENFORCE = true
SSL_MODE_VERIFY_CA / SSL_MODE_VERIFY_IDENTITYMYSQL_OPT_SSL_ENFORCE = true + MYSQL_OPT_SSL_VERIFY_SERVER_CERT = true

The mapping runs after the ssl_verify_server_cert/ssl_enforce blocks so an explicit mode wins over the ssl_verify_server_cert=false default from #235. The ssl_mode keyword is now documented in the connect docstring.

One observation for a follow-up decision

While validating on current main I noticed that with the #235 default ssl_verify_server_cert=false, connections to a TLS-capable server negotiate plaintext (libmariadb 3.4's TLS-by-default only engages when certificate verification is on — verified empirically: Ssl_cipher is empty by default, and non-empty with ssl_verify_server_cert=true, ssl_enforce=true, or ssl_mode=SSL_MODE_REQUIRED). This PR leaves that default untouched, but it may be worth calling out in the README/docs since 1.5.1 behavior (TLS on by default with the 3.4 jll) differs.

Version bumped to 1.5.3.

🤖 Generated with Claude Code

… options
MYSQL_STMT/MYSQL_RES finalizers sent COM_STMT_CLOSE / read pending rows over
the connection's socket from whatever thread triggered GC, racing in-flight
mysql_* calls on other threads and corrupting TLS state (double-free aborts,
bad record mac). Finalizers now park raw handles on a connection-owned reap
queue drained inside the next user-initiated (caller-serialized) operation;
connection teardown drains the queue before mysql_close.
MYSQL_OPT_SSL_MODE does not exist in libmariadb (its ordinal collided with
MARIADB_OPT_SKIP_READ_RESPONSE); the ssl_mode keyword is now mapped onto
MYSQL_OPT_SSL_ENFORCE / MYSQL_OPT_SSL_VERIFY_SERVER_CERT, with a warning for
the unimplementable SSL_MODE_DISABLED.
Fixes#220Fixes#240
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@codecov

codecovBot commented Aug 2, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 97.11538% with 3 lines in your changes missing coverage. Please review.
✅ Project coverage is 71.50%. Comparing base (d69e2d6) to head (1ccda90).

Files with missing linesPatch %Lines
src/api/apitypes.jl96.51%3 Missing ⚠️
Additional details and impacted files
@@ Coverage Diff @@## main #241 +/- ##
==========================================
+ Coverage 69.89% 71.50% +1.60% 
==========================================
Files 10 10 Lines 1186 1260 +74 ==========================================
+ Hits 829 901 +72 - Misses 357 359 +2 

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@quinnj
quinnj merged commit 6144a81 into mainAug 2, 2026
6 checks passed
@quinnj
quinnj deleted the jq/no-finalizer-io branch August 2, 2026 05:35
Sign up for freeto 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.

ssl_mode maps to libmariadb's MARIADB_OPT_SKIP_READ_RESPONSE: SSL_MODE_DISABLED is a no-op, other modes corrupt the protocol Double free in mariadb

1 participant

@quinnj
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Strip utm_, fbclid, gclid, etc. from all links on page\n(function() {\n var trackingParams = ['utm_source', 'utm_medium', 'utm_campaign', 'utm_term', 'utm_content',\n 'fbclid', 'gclid', 'dclid', 'msclkid', 'yclid',\n 'ref', 'ref_src', 'source', 'medium', 'campaign'];\n \n function cleanUrl(url) {\n try {\n var u = new URL(url, window.location.origin);\n var changed = false;\n trackingParams.forEach(function(p) {\n if (u.searchParams.has(p)) {\n u.searchParams.delete(p);\n changed = true;\n }\n });\n return changed ? u.toString() : url;\n } catch (e) {\n return url;\n }\n }\n \n function cleanLinks() {\n document.querySelectorAll('a[href]').forEach(function(a) {\n var clean = cleanUrl(a.href);\n if (clean !== a.href) a.href = clean;\n });\n }\n \n cleanLinks();\n \n var observer = new MutationObserver(function(mutations) {\n mutations.forEach(function(m) {\n m.addedNodes.forEach(function(node) {\n if (node.nodeType === 1) {\n if (node.tagName === 'A') cleanLinks();\n node.querySelectorAll('a[href]').forEach(function(a) {\n var clean = cleanUrl(a.href);\n if (clean !== a.href) a.href = clean;\n });\n }\n });\n });\n });\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Remove Tracking Parameters from Links"); } } catch(__e) { console.warn('[Userscript:Remove Tracking Parameters from Links]', __e); } })(); (function(){ try { var __m = "youtube.com"; var __re = new RegExp('^' + "youtube\\.com" + '
Skip to content

Never do socket I/O in finalizers; map ssl_mode onto real Connector/C options - #241

Merged
quinnj merged 1 commit into
mainfrom
jq/no-finalizer-io
Aug 2, 2026
Merged

Never do socket I/O in finalizers; map ssl_mode onto real Connector/C options#241
quinnj merged 1 commit into
mainfrom
jq/no-finalizer-io

Conversation

@quinnj

Copy link
Copy Markdown
Member

Fixes#220. Fixes#240. Related: #234.

Two fixes, both rooted in the investigation written up in this comment on #220 and in #240.

1. Finalizers no longer do socket I/O (#220)

The API.MYSQL_STMT finalizer called mysql_stmt_close, which sends COM_STMT_CLOSE over the connection's socket. Finalizers run on whatever thread triggers GC — concurrently with an in-flight mysql_* call on another thread — and a MYSQL* is not thread-safe, so an abandoned statement's finalizer raced legitimate, even lock-serialized, use of the same connection. Under TLS the two threads interleave inside one SSL*, corrupting OpenSSL state: bad record mac errors, wedged connections, and the double-free aborts reported in #220 (crash reports show the GC-finalizer thread and a mysql_commit thread simultaneously inside ma_tls_close → SSL_free). API.MYSQL_RES (mysql_free_result reads un-fetched rows off the wire) and API.MYSQL (mysql_close sends COM_QUIT) had the same problem.

The invariant this PR establishes: finalizers never touch the socket.

  • MYSQL now owns a small reap queue (Threads.SpinLock + two Vector{Ptr{Cvoid}}). Statement and result wrappers keep a reference to their parent MYSQL.
  • The MYSQL_STMT/MYSQL_RES finalizers only park their raw handle on the queue (no libmariadb call) and are gone. If the connection is already closed, the statement finalizer calls mysql_stmt_close directly — mysql_close has invalidated the handles, so that's a purely local free; an un-drained result in that state is leaked rather than read through a freed MYSQL*.
  • API.reap!(mysql) drains the queue — it runs at the top of clear!(conn), i.e. inside DBInterface.prepare/DBInterface.execute/cursor operations, which the caller already serializes with all other use of the connection. The socket-touching closes happen there, outside the spinlock.
  • The MYSQL finalizer performs the full teardown (free parked results, close parked statements, mysql_close) — safe because an unreachable connection wrapper has no in-flight calls. All lock acquisition in finalizers uses the manual's trylock-or-re-register pattern, so a finalizer can never deadlock against a thread holding the reap lock.
  • Explicit paths (DBInterface.close!(stmt), DBInterface.close!(conn), clear!'s result cleanup) now call immediate API.close!/API.free! instead of finalize(...), preserving their old eager semantics; the still-registered finalizer no-ops once ptr is C_NULL.

Validation

The reproducer from the #220 comment (6 tasks doing prepared-statement work behind one ReentrantLock, one thread applying GC pressure, -t 8, mysql:8 in Docker, TLS on):

  • before: SIGABRT (malloc: double free in SSL_write/SSL_free) or SIGSEGV on every run within 1–4 twenty-second rounds, plus (2026): TLS/SSL error: ssl/tls alert bad record mac rounds;
  • after: 8/8 rounds clean with TLS confirmed active (TLS_AES_256_GCM_SHA384), no errors of any kind.

New tests assert that abandoned statements are parked (not closed mid-GC) and reaped by the next operation, that closing a connection with parked handles is safe, and (when JULIA_NUM_THREADS > 1) run a 5-second lock-serialized concurrency smoke test that aborts the process on the old code.

2. ssl_mode mapped onto options libmariadb actually has (#240)

API.MYSQL_OPT_SSL_MODE does not exist in libmariadb — the enum entry's ordinal (7025) collided with MARIADB_OPT_SKIP_READ_RESPONSE, so ssl_mode=SSL_MODE_DISABLED was a silent no-op and any other mode set skip-read-response to true, corrupting the protocol. The entry is removed from mysql_option (technically breaking for direct API users, but every possible use was a bug), and the ssl_mode keyword now maps onto real Connector/C options:

modeeffect
SSL_MODE_DISABLED@warn (libmariadb 3.4+ cannot disable TLS client-side; see #240)
SSL_MODE_PREFERREDno-op (Connector/C default)
SSL_MODE_REQUIREDMYSQL_OPT_SSL_ENFORCE = true
SSL_MODE_VERIFY_CA / SSL_MODE_VERIFY_IDENTITYMYSQL_OPT_SSL_ENFORCE = true + MYSQL_OPT_SSL_VERIFY_SERVER_CERT = true

The mapping runs after the ssl_verify_server_cert/ssl_enforce blocks so an explicit mode wins over the ssl_verify_server_cert=false default from #235. The ssl_mode keyword is now documented in the connect docstring.

One observation for a follow-up decision

While validating on current main I noticed that with the #235 default ssl_verify_server_cert=false, connections to a TLS-capable server negotiate plaintext (libmariadb 3.4's TLS-by-default only engages when certificate verification is on — verified empirically: Ssl_cipher is empty by default, and non-empty with ssl_verify_server_cert=true, ssl_enforce=true, or ssl_mode=SSL_MODE_REQUIRED). This PR leaves that default untouched, but it may be worth calling out in the README/docs since 1.5.1 behavior (TLS on by default with the 3.4 jll) differs.

Version bumped to 1.5.3.

🤖 Generated with Claude Code

… options
MYSQL_STMT/MYSQL_RES finalizers sent COM_STMT_CLOSE / read pending rows over
the connection's socket from whatever thread triggered GC, racing in-flight
mysql_* calls on other threads and corrupting TLS state (double-free aborts,
bad record mac). Finalizers now park raw handles on a connection-owned reap
queue drained inside the next user-initiated (caller-serialized) operation;
connection teardown drains the queue before mysql_close.
MYSQL_OPT_SSL_MODE does not exist in libmariadb (its ordinal collided with
MARIADB_OPT_SKIP_READ_RESPONSE); the ssl_mode keyword is now mapped onto
MYSQL_OPT_SSL_ENFORCE / MYSQL_OPT_SSL_VERIFY_SERVER_CERT, with a warning for
the unimplementable SSL_MODE_DISABLED.
Fixes#220Fixes#240
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@codecov

codecovBot commented Aug 2, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 97.11538% with 3 lines in your changes missing coverage. Please review.
✅ Project coverage is 71.50%. Comparing base (d69e2d6) to head (1ccda90).

Files with missing linesPatch %Lines
src/api/apitypes.jl96.51%3 Missing ⚠️
Additional details and impacted files
@@ Coverage Diff @@## main #241 +/- ##
==========================================
+ Coverage 69.89% 71.50% +1.60% 
==========================================
Files 10 10 Lines 1186 1260 +74 ==========================================
+ Hits 829 901 +72 - Misses 357 359 +2 

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@quinnj
quinnj merged commit 6144a81 into mainAug 2, 2026
6 checks passed
@quinnj
quinnj deleted the jq/no-finalizer-io branch August 2, 2026 05:35
Sign up for freeto 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.

ssl_mode maps to libmariadb's MARIADB_OPT_SKIP_READ_RESPONSE: SSL_MODE_DISABLED is a no-op, other modes corrupt the protocol Double free in mariadb

1 participant

@quinnj
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Auto-enable theater mode on YouTube\n(function() {\n function tryTheater() {\n var btn = document.querySelector('button[aria-label=\"Theater mode\"], ytd-player #player button[title=\"Theater mode\"]');\n if (btn && !btn.classList.contains('activated')) {\n btn.click();\n }\n }\n \n // Try immediately\n tryTheater();\n \n // Try after navigation (SPA)\n var lastUrl = location.href;\n setInterval(function() {\n if (location.href !== lastUrl) {\n lastUrl = location.href;\n setTimeout(tryTheater, 500);\n }\n }, 1000);\n \n // Also try on player load\n var observer = new MutationObserver(tryTheater);\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "YouTube Theater Mode Default"); } } catch(__e) { console.warn('[Userscript:YouTube Theater Mode Default]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

Never do socket I/O in finalizers; map ssl_mode onto real Connector/C options - #241

Merged
quinnj merged 1 commit into
mainfrom
jq/no-finalizer-io
Aug 2, 2026
Merged

Never do socket I/O in finalizers; map ssl_mode onto real Connector/C options#241
quinnj merged 1 commit into
mainfrom
jq/no-finalizer-io

Conversation

@quinnj

Copy link
Copy Markdown
Member

Fixes#220. Fixes#240. Related: #234.

Two fixes, both rooted in the investigation written up in this comment on #220 and in #240.

1. Finalizers no longer do socket I/O (#220)

The API.MYSQL_STMT finalizer called mysql_stmt_close, which sends COM_STMT_CLOSE over the connection's socket. Finalizers run on whatever thread triggers GC — concurrently with an in-flight mysql_* call on another thread — and a MYSQL* is not thread-safe, so an abandoned statement's finalizer raced legitimate, even lock-serialized, use of the same connection. Under TLS the two threads interleave inside one SSL*, corrupting OpenSSL state: bad record mac errors, wedged connections, and the double-free aborts reported in #220 (crash reports show the GC-finalizer thread and a mysql_commit thread simultaneously inside ma_tls_close → SSL_free). API.MYSQL_RES (mysql_free_result reads un-fetched rows off the wire) and API.MYSQL (mysql_close sends COM_QUIT) had the same problem.

The invariant this PR establishes: finalizers never touch the socket.

  • MYSQL now owns a small reap queue (Threads.SpinLock + two Vector{Ptr{Cvoid}}). Statement and result wrappers keep a reference to their parent MYSQL.
  • The MYSQL_STMT/MYSQL_RES finalizers only park their raw handle on the queue (no libmariadb call) and are gone. If the connection is already closed, the statement finalizer calls mysql_stmt_close directly — mysql_close has invalidated the handles, so that's a purely local free; an un-drained result in that state is leaked rather than read through a freed MYSQL*.
  • API.reap!(mysql) drains the queue — it runs at the top of clear!(conn), i.e. inside DBInterface.prepare/DBInterface.execute/cursor operations, which the caller already serializes with all other use of the connection. The socket-touching closes happen there, outside the spinlock.
  • The MYSQL finalizer performs the full teardown (free parked results, close parked statements, mysql_close) — safe because an unreachable connection wrapper has no in-flight calls. All lock acquisition in finalizers uses the manual's trylock-or-re-register pattern, so a finalizer can never deadlock against a thread holding the reap lock.
  • Explicit paths (DBInterface.close!(stmt), DBInterface.close!(conn), clear!'s result cleanup) now call immediate API.close!/API.free! instead of finalize(...), preserving their old eager semantics; the still-registered finalizer no-ops once ptr is C_NULL.

Validation

The reproducer from the #220 comment (6 tasks doing prepared-statement work behind one ReentrantLock, one thread applying GC pressure, -t 8, mysql:8 in Docker, TLS on):

  • before: SIGABRT (malloc: double free in SSL_write/SSL_free) or SIGSEGV on every run within 1–4 twenty-second rounds, plus (2026): TLS/SSL error: ssl/tls alert bad record mac rounds;
  • after: 8/8 rounds clean with TLS confirmed active (TLS_AES_256_GCM_SHA384), no errors of any kind.

New tests assert that abandoned statements are parked (not closed mid-GC) and reaped by the next operation, that closing a connection with parked handles is safe, and (when JULIA_NUM_THREADS > 1) run a 5-second lock-serialized concurrency smoke test that aborts the process on the old code.

2. ssl_mode mapped onto options libmariadb actually has (#240)

API.MYSQL_OPT_SSL_MODE does not exist in libmariadb — the enum entry's ordinal (7025) collided with MARIADB_OPT_SKIP_READ_RESPONSE, so ssl_mode=SSL_MODE_DISABLED was a silent no-op and any other mode set skip-read-response to true, corrupting the protocol. The entry is removed from mysql_option (technically breaking for direct API users, but every possible use was a bug), and the ssl_mode keyword now maps onto real Connector/C options:

modeeffect
SSL_MODE_DISABLED@warn (libmariadb 3.4+ cannot disable TLS client-side; see #240)
SSL_MODE_PREFERREDno-op (Connector/C default)
SSL_MODE_REQUIREDMYSQL_OPT_SSL_ENFORCE = true
SSL_MODE_VERIFY_CA / SSL_MODE_VERIFY_IDENTITYMYSQL_OPT_SSL_ENFORCE = true + MYSQL_OPT_SSL_VERIFY_SERVER_CERT = true

The mapping runs after the ssl_verify_server_cert/ssl_enforce blocks so an explicit mode wins over the ssl_verify_server_cert=false default from #235. The ssl_mode keyword is now documented in the connect docstring.

One observation for a follow-up decision

While validating on current main I noticed that with the #235 default ssl_verify_server_cert=false, connections to a TLS-capable server negotiate plaintext (libmariadb 3.4's TLS-by-default only engages when certificate verification is on — verified empirically: Ssl_cipher is empty by default, and non-empty with ssl_verify_server_cert=true, ssl_enforce=true, or ssl_mode=SSL_MODE_REQUIRED). This PR leaves that default untouched, but it may be worth calling out in the README/docs since 1.5.1 behavior (TLS on by default with the 3.4 jll) differs.

Version bumped to 1.5.3.

🤖 Generated with Claude Code

… options
MYSQL_STMT/MYSQL_RES finalizers sent COM_STMT_CLOSE / read pending rows over
the connection's socket from whatever thread triggered GC, racing in-flight
mysql_* calls on other threads and corrupting TLS state (double-free aborts,
bad record mac). Finalizers now park raw handles on a connection-owned reap
queue drained inside the next user-initiated (caller-serialized) operation;
connection teardown drains the queue before mysql_close.
MYSQL_OPT_SSL_MODE does not exist in libmariadb (its ordinal collided with
MARIADB_OPT_SKIP_READ_RESPONSE); the ssl_mode keyword is now mapped onto
MYSQL_OPT_SSL_ENFORCE / MYSQL_OPT_SSL_VERIFY_SERVER_CERT, with a warning for
the unimplementable SSL_MODE_DISABLED.
Fixes#220Fixes#240
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@codecov

codecovBot commented Aug 2, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 97.11538% with 3 lines in your changes missing coverage. Please review.
✅ Project coverage is 71.50%. Comparing base (d69e2d6) to head (1ccda90).

Files with missing linesPatch %Lines
src/api/apitypes.jl96.51%3 Missing ⚠️
Additional details and impacted files
@@ Coverage Diff @@## main #241 +/- ##
==========================================
+ Coverage 69.89% 71.50% +1.60% 
==========================================
Files 10 10 Lines 1186 1260 +74 ==========================================
+ Hits 829 901 +72 - Misses 357 359 +2 

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@quinnj
quinnj merged commit 6144a81 into mainAug 2, 2026
6 checks passed
@quinnj
quinnj deleted the jq/no-finalizer-io branch August 2, 2026 05:35
Sign up for freeto 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.

ssl_mode maps to libmariadb's MARIADB_OPT_SKIP_READ_RESPONSE: SSL_MODE_DISABLED is a no-op, other modes corrupt the protocol Double free in mariadb

1 participant

@quinnj
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Remove or un-stick sticky/fixed headers that block content\n(function() {\n function unstick() {\n document.querySelectorAll('header, nav, [role=\"banner\"], .header, .navbar, .sticky, .fixed-top, [style*=\"position: fixed\"], [style*=\"position:sticky\"]').forEach(function(el) {\n if (el.style.position === 'fixed' || el.style.position === 'sticky' || \n getComputedStyle(el).position === 'fixed' || getComputedStyle(el).position === 'sticky') {\n el.style.position = 'static';\n el.style.top = 'auto';\n el.style.zIndex = 'auto';\n }\n });\n }\n \n unstick();\n \n var observer = new MutationObserver(unstick);\n observer.observe(document.body, { childList: true, subtree: true, attributes: true, attributeFilter: ['style', 'class'] });\n})();", "Kill Sticky Headers"); } } catch(__e) { console.warn('[Userscript:Kill Sticky Headers]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

Never do socket I/O in finalizers; map ssl_mode onto real Connector/C options - #241

Merged
quinnj merged 1 commit into
mainfrom
jq/no-finalizer-io
Aug 2, 2026
Merged

Never do socket I/O in finalizers; map ssl_mode onto real Connector/C options#241
quinnj merged 1 commit into
mainfrom
jq/no-finalizer-io

Conversation

@quinnj

Copy link
Copy Markdown
Member

Fixes#220. Fixes#240. Related: #234.

Two fixes, both rooted in the investigation written up in this comment on #220 and in #240.

1. Finalizers no longer do socket I/O (#220)

The API.MYSQL_STMT finalizer called mysql_stmt_close, which sends COM_STMT_CLOSE over the connection's socket. Finalizers run on whatever thread triggers GC — concurrently with an in-flight mysql_* call on another thread — and a MYSQL* is not thread-safe, so an abandoned statement's finalizer raced legitimate, even lock-serialized, use of the same connection. Under TLS the two threads interleave inside one SSL*, corrupting OpenSSL state: bad record mac errors, wedged connections, and the double-free aborts reported in #220 (crash reports show the GC-finalizer thread and a mysql_commit thread simultaneously inside ma_tls_close → SSL_free). API.MYSQL_RES (mysql_free_result reads un-fetched rows off the wire) and API.MYSQL (mysql_close sends COM_QUIT) had the same problem.

The invariant this PR establishes: finalizers never touch the socket.

  • MYSQL now owns a small reap queue (Threads.SpinLock + two Vector{Ptr{Cvoid}}). Statement and result wrappers keep a reference to their parent MYSQL.
  • The MYSQL_STMT/MYSQL_RES finalizers only park their raw handle on the queue (no libmariadb call) and are gone. If the connection is already closed, the statement finalizer calls mysql_stmt_close directly — mysql_close has invalidated the handles, so that's a purely local free; an un-drained result in that state is leaked rather than read through a freed MYSQL*.
  • API.reap!(mysql) drains the queue — it runs at the top of clear!(conn), i.e. inside DBInterface.prepare/DBInterface.execute/cursor operations, which the caller already serializes with all other use of the connection. The socket-touching closes happen there, outside the spinlock.
  • The MYSQL finalizer performs the full teardown (free parked results, close parked statements, mysql_close) — safe because an unreachable connection wrapper has no in-flight calls. All lock acquisition in finalizers uses the manual's trylock-or-re-register pattern, so a finalizer can never deadlock against a thread holding the reap lock.
  • Explicit paths (DBInterface.close!(stmt), DBInterface.close!(conn), clear!'s result cleanup) now call immediate API.close!/API.free! instead of finalize(...), preserving their old eager semantics; the still-registered finalizer no-ops once ptr is C_NULL.

Validation

The reproducer from the #220 comment (6 tasks doing prepared-statement work behind one ReentrantLock, one thread applying GC pressure, -t 8, mysql:8 in Docker, TLS on):

  • before: SIGABRT (malloc: double free in SSL_write/SSL_free) or SIGSEGV on every run within 1–4 twenty-second rounds, plus (2026): TLS/SSL error: ssl/tls alert bad record mac rounds;
  • after: 8/8 rounds clean with TLS confirmed active (TLS_AES_256_GCM_SHA384), no errors of any kind.

New tests assert that abandoned statements are parked (not closed mid-GC) and reaped by the next operation, that closing a connection with parked handles is safe, and (when JULIA_NUM_THREADS > 1) run a 5-second lock-serialized concurrency smoke test that aborts the process on the old code.

2. ssl_mode mapped onto options libmariadb actually has (#240)

API.MYSQL_OPT_SSL_MODE does not exist in libmariadb — the enum entry's ordinal (7025) collided with MARIADB_OPT_SKIP_READ_RESPONSE, so ssl_mode=SSL_MODE_DISABLED was a silent no-op and any other mode set skip-read-response to true, corrupting the protocol. The entry is removed from mysql_option (technically breaking for direct API users, but every possible use was a bug), and the ssl_mode keyword now maps onto real Connector/C options:

modeeffect
SSL_MODE_DISABLED@warn (libmariadb 3.4+ cannot disable TLS client-side; see #240)
SSL_MODE_PREFERREDno-op (Connector/C default)
SSL_MODE_REQUIREDMYSQL_OPT_SSL_ENFORCE = true
SSL_MODE_VERIFY_CA / SSL_MODE_VERIFY_IDENTITYMYSQL_OPT_SSL_ENFORCE = true + MYSQL_OPT_SSL_VERIFY_SERVER_CERT = true

The mapping runs after the ssl_verify_server_cert/ssl_enforce blocks so an explicit mode wins over the ssl_verify_server_cert=false default from #235. The ssl_mode keyword is now documented in the connect docstring.

One observation for a follow-up decision

While validating on current main I noticed that with the #235 default ssl_verify_server_cert=false, connections to a TLS-capable server negotiate plaintext (libmariadb 3.4's TLS-by-default only engages when certificate verification is on — verified empirically: Ssl_cipher is empty by default, and non-empty with ssl_verify_server_cert=true, ssl_enforce=true, or ssl_mode=SSL_MODE_REQUIRED). This PR leaves that default untouched, but it may be worth calling out in the README/docs since 1.5.1 behavior (TLS on by default with the 3.4 jll) differs.

Version bumped to 1.5.3.

🤖 Generated with Claude Code

… options
MYSQL_STMT/MYSQL_RES finalizers sent COM_STMT_CLOSE / read pending rows over
the connection's socket from whatever thread triggered GC, racing in-flight
mysql_* calls on other threads and corrupting TLS state (double-free aborts,
bad record mac). Finalizers now park raw handles on a connection-owned reap
queue drained inside the next user-initiated (caller-serialized) operation;
connection teardown drains the queue before mysql_close.
MYSQL_OPT_SSL_MODE does not exist in libmariadb (its ordinal collided with
MARIADB_OPT_SKIP_READ_RESPONSE); the ssl_mode keyword is now mapped onto
MYSQL_OPT_SSL_ENFORCE / MYSQL_OPT_SSL_VERIFY_SERVER_CERT, with a warning for
the unimplementable SSL_MODE_DISABLED.
Fixes#220Fixes#240
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@codecov

codecovBot commented Aug 2, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 97.11538% with 3 lines in your changes missing coverage. Please review.
✅ Project coverage is 71.50%. Comparing base (d69e2d6) to head (1ccda90).

Files with missing linesPatch %Lines
src/api/apitypes.jl96.51%3 Missing ⚠️
Additional details and impacted files
@@ Coverage Diff @@## main #241 +/- ##
==========================================
+ Coverage 69.89% 71.50% +1.60% 
==========================================
Files 10 10 Lines 1186 1260 +74 ==========================================
+ Hits 829 901 +72 - Misses 357 359 +2 

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@quinnj
quinnj merged commit 6144a81 into mainAug 2, 2026
6 checks passed
@quinnj
quinnj deleted the jq/no-finalizer-io branch August 2, 2026 05:35
Sign up for freeto 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.

ssl_mode maps to libmariadb's MARIADB_OPT_SKIP_READ_RESPONSE: SSL_MODE_DISABLED is a no-op, other modes corrupt the protocol Double free in mariadb

1 participant

@quinnj
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Universal Dark Mode - works on any site\n(function() {\n var enabled = true;\n \n function applyDarkMode() {\n if (!enabled) return;\n \n // Create style element if it doesn't exist\n var style = document.getElementById('universal-dark-mode-style');\n if (!style) {\n style = document.createElement('style');\n style.id = 'universal-dark-mode-style';\n document.head.appendChild(style);\n }\n \n // Dark mode CSS - inverts colors but preserves images/video\n style.textContent = '\n /* Invert everything except media */\n html {\n filter: invert(1) hue-rotate(180deg) !important;\n background: #1a1a2e !important;\n }\n \n /* Restore images, videos, iframes, canvas */\n img, video, iframe, canvas, svg, picture, [style*=\"background-image\"] {\n filter: invert(1) hue-rotate(180deg) !important;\n }\n \n /* Preserve specific elements that should not be inverted */\n .no-dark-mode, .no-dark-mode *,\n [data-theme=\"light\"], [data-theme=\"light\"],\n .ace_editor, .ace_editor *,\n .CodeMirror, .CodeMirror *,\n .monaco-editor, .monaco-editor *,\n .markdown-body pre, .markdown-body pre *,\n .highlight, .highlight *,\n pre code, pre code * {\n filter: none !important;\n }\n \n /* Fix common UI elements */\n .modal, .popup, .dropdown-menu, .tooltip, .popover {\n filter: invert(1) hue-rotate(180deg) !important;\n background: #2d2d44 !important;\n border-color: #444 !important;\n }\n \n /* Scrollbars */\n ::-webkit-scrollbar { background: #1a1a2e !important; }\n ::-webkit-scrollbar-thumb { background: #444 !important; }\n ::-webkit-scrollbar-thumb:hover { background: #555 !important; }\n \n /* Selection */\n ::selection { background: #4ecdc4 !important; color: #1a1a2e !important; }\n ::-moz-selection { background: #4ecdc4 !important; color: #1a1a2e !important; }\n ';\n }\n \n function removeDarkMode() {\n var style = document.getElementById('universal-dark-mode-style');\n if (style) style.remove();\n }\n \n // Toggle with Alt+Shift+D\n document.addEventListener('keydown', function(e) {\n if (e.altKey && e.shiftKey && e.key === 'D') {\n e.preventDefault();\n enabled = !enabled;\n if (enabled) {\n applyDarkMode();\n console.log('[Universal Dark Mode] Enabled');\n } else {\n removeDarkMode();\n console.log('[Universal Dark Mode] Disabled');\n }\n }\n });\n \n // Apply on load\n applyDarkMode();\n \n // Re-apply on dynamic content\n var observer = new MutationObserver(function(mutations) {\n if (enabled && !document.getElementById('universal-dark-mode-style')) {\n applyDarkMode();\n }\n });\n observer.observe(document.head, { childList: true });\n \n console.log('[Universal Dark Mode] Loaded - Press Alt+Shift+D to toggle');\n})();", "Universal Dark Mode"); } } catch(__e) { console.warn('[Userscript:Universal Dark Mode]', __e); } })(); })();
Skip to content

Never do socket I/O in finalizers; map ssl_mode onto real Connector/C options - #241

Merged
quinnj merged 1 commit into
mainfrom
jq/no-finalizer-io
Aug 2, 2026
Merged

Never do socket I/O in finalizers; map ssl_mode onto real Connector/C options#241
quinnj merged 1 commit into
mainfrom
jq/no-finalizer-io

Conversation

@quinnj

Copy link
Copy Markdown
Member

Fixes#220. Fixes#240. Related: #234.

Two fixes, both rooted in the investigation written up in this comment on #220 and in #240.

1. Finalizers no longer do socket I/O (#220)

The API.MYSQL_STMT finalizer called mysql_stmt_close, which sends COM_STMT_CLOSE over the connection's socket. Finalizers run on whatever thread triggers GC — concurrently with an in-flight mysql_* call on another thread — and a MYSQL* is not thread-safe, so an abandoned statement's finalizer raced legitimate, even lock-serialized, use of the same connection. Under TLS the two threads interleave inside one SSL*, corrupting OpenSSL state: bad record mac errors, wedged connections, and the double-free aborts reported in #220 (crash reports show the GC-finalizer thread and a mysql_commit thread simultaneously inside ma_tls_close → SSL_free). API.MYSQL_RES (mysql_free_result reads un-fetched rows off the wire) and API.MYSQL (mysql_close sends COM_QUIT) had the same problem.

The invariant this PR establishes: finalizers never touch the socket.

  • MYSQL now owns a small reap queue (Threads.SpinLock + two Vector{Ptr{Cvoid}}). Statement and result wrappers keep a reference to their parent MYSQL.
  • The MYSQL_STMT/MYSQL_RES finalizers only park their raw handle on the queue (no libmariadb call) and are gone. If the connection is already closed, the statement finalizer calls mysql_stmt_close directly — mysql_close has invalidated the handles, so that's a purely local free; an un-drained result in that state is leaked rather than read through a freed MYSQL*.
  • API.reap!(mysql) drains the queue — it runs at the top of clear!(conn), i.e. inside DBInterface.prepare/DBInterface.execute/cursor operations, which the caller already serializes with all other use of the connection. The socket-touching closes happen there, outside the spinlock.
  • The MYSQL finalizer performs the full teardown (free parked results, close parked statements, mysql_close) — safe because an unreachable connection wrapper has no in-flight calls. All lock acquisition in finalizers uses the manual's trylock-or-re-register pattern, so a finalizer can never deadlock against a thread holding the reap lock.
  • Explicit paths (DBInterface.close!(stmt), DBInterface.close!(conn), clear!'s result cleanup) now call immediate API.close!/API.free! instead of finalize(...), preserving their old eager semantics; the still-registered finalizer no-ops once ptr is C_NULL.

Validation

The reproducer from the #220 comment (6 tasks doing prepared-statement work behind one ReentrantLock, one thread applying GC pressure, -t 8, mysql:8 in Docker, TLS on):

  • before: SIGABRT (malloc: double free in SSL_write/SSL_free) or SIGSEGV on every run within 1–4 twenty-second rounds, plus (2026): TLS/SSL error: ssl/tls alert bad record mac rounds;
  • after: 8/8 rounds clean with TLS confirmed active (TLS_AES_256_GCM_SHA384), no errors of any kind.

New tests assert that abandoned statements are parked (not closed mid-GC) and reaped by the next operation, that closing a connection with parked handles is safe, and (when JULIA_NUM_THREADS > 1) run a 5-second lock-serialized concurrency smoke test that aborts the process on the old code.

2. ssl_mode mapped onto options libmariadb actually has (#240)

API.MYSQL_OPT_SSL_MODE does not exist in libmariadb — the enum entry's ordinal (7025) collided with MARIADB_OPT_SKIP_READ_RESPONSE, so ssl_mode=SSL_MODE_DISABLED was a silent no-op and any other mode set skip-read-response to true, corrupting the protocol. The entry is removed from mysql_option (technically breaking for direct API users, but every possible use was a bug), and the ssl_mode keyword now maps onto real Connector/C options:

modeeffect
SSL_MODE_DISABLED@warn (libmariadb 3.4+ cannot disable TLS client-side; see #240)
SSL_MODE_PREFERREDno-op (Connector/C default)
SSL_MODE_REQUIREDMYSQL_OPT_SSL_ENFORCE = true
SSL_MODE_VERIFY_CA / SSL_MODE_VERIFY_IDENTITYMYSQL_OPT_SSL_ENFORCE = true + MYSQL_OPT_SSL_VERIFY_SERVER_CERT = true

The mapping runs after the ssl_verify_server_cert/ssl_enforce blocks so an explicit mode wins over the ssl_verify_server_cert=false default from #235. The ssl_mode keyword is now documented in the connect docstring.

One observation for a follow-up decision

While validating on current main I noticed that with the #235 default ssl_verify_server_cert=false, connections to a TLS-capable server negotiate plaintext (libmariadb 3.4's TLS-by-default only engages when certificate verification is on — verified empirically: Ssl_cipher is empty by default, and non-empty with ssl_verify_server_cert=true, ssl_enforce=true, or ssl_mode=SSL_MODE_REQUIRED). This PR leaves that default untouched, but it may be worth calling out in the README/docs since 1.5.1 behavior (TLS on by default with the 3.4 jll) differs.

Version bumped to 1.5.3.

🤖 Generated with Claude Code

… options
MYSQL_STMT/MYSQL_RES finalizers sent COM_STMT_CLOSE / read pending rows over
the connection's socket from whatever thread triggered GC, racing in-flight
mysql_* calls on other threads and corrupting TLS state (double-free aborts,
bad record mac). Finalizers now park raw handles on a connection-owned reap
queue drained inside the next user-initiated (caller-serialized) operation;
connection teardown drains the queue before mysql_close.
MYSQL_OPT_SSL_MODE does not exist in libmariadb (its ordinal collided with
MARIADB_OPT_SKIP_READ_RESPONSE); the ssl_mode keyword is now mapped onto
MYSQL_OPT_SSL_ENFORCE / MYSQL_OPT_SSL_VERIFY_SERVER_CERT, with a warning for
the unimplementable SSL_MODE_DISABLED.
Fixes#220Fixes#240
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@codecov

codecovBot commented Aug 2, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 97.11538% with 3 lines in your changes missing coverage. Please review.
✅ Project coverage is 71.50%. Comparing base (d69e2d6) to head (1ccda90).

Files with missing linesPatch %Lines
src/api/apitypes.jl96.51%3 Missing ⚠️
Additional details and impacted files
@@ Coverage Diff @@## main #241 +/- ##
==========================================
+ Coverage 69.89% 71.50% +1.60% 
==========================================
Files 10 10 Lines 1186 1260 +74 ==========================================
+ Hits 829 901 +72 - Misses 357 359 +2 

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@quinnj
quinnj merged commit 6144a81 into mainAug 2, 2026
6 checks passed
@quinnj
quinnj deleted the jq/no-finalizer-io branch August 2, 2026 05:35
Sign up for freeto 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.

ssl_mode maps to libmariadb's MARIADB_OPT_SKIP_READ_RESPONSE: SSL_MODE_DISABLED is a no-op, other modes corrupt the protocol Double free in mariadb

1 participant

@quinnj