From 0c535abdba5f264efd14ccb17843b3faa3ed5dfe Mon Sep 17 00:00:00 2001 From: Alex Le Date: Fri, 14 Aug 2026 16:02:59 -0700 Subject: [PATCH] fix: drain in-flight commands before close frees the handle (#212, race #2) Close-vs-close was already serialized via `@close_lock`, but close-vs-in-flight-command was not. A thread inside `Bindings.command` / `Bindings.batch` / `Bindings.invoke_script` (all `blocking: true`, so they release the GVL and run in native code) could still hold the raw client handle when another thread's `close` freed it via `Bindings.close_client`, tearing down the connection under a live dispatch. Observed in CI as a `malloc(): unaligned tcache chunk detected` SIGABRT. Add a drain protocol guarded by `@close_lock`: - `@inflight` counter + `@drain_cv` ConditionVariable, initialized alongside `@close_lock` in `connection!`. - `send_command`, `send_batch_commands`, and `invoke_script` bracket their FFI dispatch with `acquire_connection_slot` / `release_connection_slot`. The lock is held only around the counter bump/decrement, never across the FFI call, so concurrent in-flight commands are not serialized. - `close` is two-phase: phase 1 sets `@closing` under `try_lock` (trap-safe), phase 2 waits on `@drain_cv` until `@inflight` reaches zero, then nulls the handle and calls `close_client`. Degrades to a lock-free best-effort close on `ThreadError` so `Signal.trap` shutdown still works. `acquire_connection_slot` refuses new slots once `@closing` is set, which is what makes the drain finite. `connection!` is retained for the fast fail-fast path. Add `test_close_drains_in_flight_commands_no_crash`, which races many worker threads issuing commands against a concurrent `close` and asserts the client ends up cleanly closed with a balanced in-flight counter. Signed-off-by: Alex Le --- lib/valkey.rb | 443 +++++++++++++--------- lib/valkey/commands/scripting_commands.rb | 72 ++-- test/valkey/connection_lifecycle_test.rb | 64 ++++ 3 files changed, 372 insertions(+), 207 deletions(-) diff --git a/lib/valkey.rb b/lib/valkey.rb index d328b2b9..02e21924 100644 --- a/lib/valkey.rb +++ b/lib/valkey.rb @@ -272,8 +272,23 @@ def initialize(options = {}) end @connection = res[:conn_ptr] - # Lock for serializing close() (issue #212). + # Concurrency primitives for close-vs-in-flight-command safety (issue #212, race #2). + # + # `@close_lock` guards `@closing`, `@inflight`, and `@connection` writes. + # `@drain_cv` signals when `@inflight` drops to zero so a pending `close` + # can free the handle without racing an in-flight `Bindings.command` / + # `Bindings.batch` / `Bindings.invoke_script` (all `blocking: true`, so + # they release the GVL and run in native code without holding this lock). + # + # The lock is held only around the counter bump / decrement — never across + # an FFI dispatch — so throughput is unaffected. `close` waits on + # `@drain_cv` until the last in-flight command releases its slot, then + # nulls the handle and calls `Bindings.close_client`. See `#close`, + # `#acquire_connection_slot`, `#release_connection_slot`. @close_lock = Mutex.new + @drain_cv = ConditionVariable.new + @inflight = 0 + @closing = false Bindings.free_connection_response(response_ptr) # Store cluster mode flag for response handling (MAP returns Hash in cluster, Array in standalone) @@ -294,16 +309,50 @@ def initialize(options = {}) end # Closes the client and frees the native connection. + # + # Idempotent and safe to call from any thread. Blocks until every in-flight + # command has released its slot, so `Bindings.close_client` never runs + # concurrently with a still-live `Bindings.command` / `Bindings.batch` / + # `Bindings.invoke_script` on the same handle (issue #212, race #2 — those + # FFI calls declare `blocking: true` and release the GVL, but + # `close_client` does not, so two threads could otherwise be in Rust + # simultaneously and tear down the connection under a live dispatch — the + # `malloc(): unaligned tcache chunk detected` abort observed in CI). + # + # Safe to call from a signal-trap handler even though most `Mutex` methods + # raise `ThreadError` there: phase 1 uses only `try_lock`/`unlock`, and if + # phase 2's `synchronize` raises we fall back to a best-effort close (no + # drain). The common trap-shutdown case has no in-flight commands, so the + # fallback still frees the handle correctly. def close return unless @close_lock&.try_lock begin + return if @closing + + @closing = true + ensure + @close_lock.unlock + end + + # Phase 2: drain in-flight commands, then free the handle. `synchronize` + # + CV wait is not trap-safe; degrade to a lock-free close on ThreadError. + begin + @close_lock.synchronize do + @drain_cv.wait(@close_lock) while @inflight.positive? + + conn = @connection + @connection = nil + + Bindings.close_client(conn) unless conn.nil? || conn.null? + end + rescue ThreadError + # In trap context (or wherever synchronize can't run). @closing is + # already set so no new commands can start; free the handle directly. conn = @connection @connection = nil Bindings.close_client(conn) unless conn.nil? || conn.null? - ensure - @close_lock.unlock end end @@ -361,110 +410,123 @@ def statistics # explicit receiver to issue commands with no dedicated wrapper method yet # (e.g. `DEBUG SLEEP`, raw `HSET` in vector search fixtures). def send_command(command_type, command_args = [], route: nil, &block) - conn = connection! - - channel = 0 - - # Handle empty command_args case - if command_args.empty? - arg_ptrs = FFI::MemoryPointer.new(:pointer, 1) - arg_lens = FFI::MemoryPointer.new(:ulong, 1) - arg_ptrs.put_pointer(0, FFI::MemoryPointer.new(1)) - arg_lens.put_ulong(0, 0) - _buffers = [] # nothing to keep alive - flattened_args = command_args - else - arg_ptrs, arg_lens, _buffers, flattened_args = build_command_args(command_args) - end - - # Create OpenTelemetry span if sampling is enabled, as a child of the app's current - # span context when a parent_span_context_provider is registered (see Valkey::OpenTelemetry). - span_ptr = 0 - if OpenTelemetry.should_sample? - begin - parent_ctx = OpenTelemetry.parent_span_context - span_ptr = if parent_ctx - Bindings.create_otel_span_with_trace_context( - command_type, parent_ctx[:trace_id], parent_ctx[:span_id], - parent_ctx[:trace_flags], parent_ctx[:tracestate] - ) - else - Bindings.create_otel_span(command_type) - end - rescue StandardError => e - # Log error but continue execution - tracing is non-critical - warn "Failed to create OpenTelemetry span: #{e.message}" - span_ptr = 0 - end - end - + # Bracket the FFI dispatch with acquire_connection_slot / release_connection_slot + # so `close` can safely drain in-flight commands before freeing the native + # handle (issue #212, race #2). The lock is only held briefly around the + # counter bump / decrement — the FFI call itself runs without holding it + # so concurrent commands are not serialized (glide-core supports + # concurrent in-flight commands per client). + conn = acquire_connection_slot begin - if route - # Use command_with_route_info when an explicit route is provided. - route_info, _pinned_bufs = route.to_ffi - res = Bindings.command_with_route_info( - conn, - channel, - command_type, - flattened_args.size, - arg_ptrs, - arg_lens, - route_info.to_ptr, - FFI::Pointer::NULL, # response_buf (NULL = normal response path) - 0, # response_buf_len - span_ptr - ) + channel = 0 + + # Handle empty command_args case + if command_args.empty? + arg_ptrs = FFI::MemoryPointer.new(:pointer, 1) + arg_lens = FFI::MemoryPointer.new(:ulong, 1) + arg_ptrs.put_pointer(0, FFI::MemoryPointer.new(1)) + arg_lens.put_ulong(0, 0) + _buffers = [] # nothing to keep alive + flattened_args = command_args else - # Use legacy command() for unrouted calls to preserve existing behavior. - route_str = "" - route_buf = FFI::MemoryPointer.from_string(route_str) - res = Bindings.command( - conn, - channel, - command_type, - flattened_args.size, - arg_ptrs, - arg_lens, - route_buf, - route_str.bytesize, - span_ptr - ) + arg_ptrs, arg_lens, _buffers, flattened_args = build_command_args(command_args) end - result = convert_response(res, &block) - ensure - # Free the native CommandResult (arena + response + error) to prevent memory leak - Bindings.free_command_result(res) if res && !res.null? - - # Always drop the span if one was created, even if command fails - if span_ptr != 0 + # Create OpenTelemetry span if sampling is enabled, as a child of the app's current + # span context when a parent_span_context_provider is registered (see Valkey::OpenTelemetry). + span_ptr = 0 + if OpenTelemetry.should_sample? begin - Bindings.drop_otel_span(span_ptr) + parent_ctx = OpenTelemetry.parent_span_context + span_ptr = if parent_ctx + Bindings.create_otel_span_with_trace_context( + command_type, parent_ctx[:trace_id], parent_ctx[:span_id], + parent_ctx[:trace_flags], parent_ctx[:tracestate] + ) + else + Bindings.create_otel_span(command_type) + end rescue StandardError => e - # Log but don't raise - span cleanup errors shouldn't break command execution - warn "Failed to drop OpenTelemetry span: #{e.message}" + # Log error but continue execution - tracing is non-critical + warn "Failed to create OpenTelemetry span: #{e.message}" + span_ptr = 0 end end - end - # Track queued commands during MULTI (except for MULTI, EXEC, DISCARD, WATCH, UNWATCH) - if @in_multi && !@queued_commands.nil? - tx_commands = [ - RequestType::MULTI, RequestType::EXEC, RequestType::DISCARD, - RequestType::WATCH, RequestType::UNWATCH - ] - if !tx_commands.include?(command_type) && result == "QUEUED" - @queued_commands << [command_type, command_args.dup, block] + begin + if route + # Use command_with_route_info when an explicit route is provided. + route_info, _pinned_bufs = route.to_ffi + res = Bindings.command_with_route_info( + conn, + channel, + command_type, + flattened_args.size, + arg_ptrs, + arg_lens, + route_info.to_ptr, + FFI::Pointer::NULL, # response_buf (NULL = normal response path) + 0, # response_buf_len + span_ptr + ) + else + # Use legacy command() for unrouted calls to preserve existing behavior. + route_str = "" + route_buf = FFI::MemoryPointer.from_string(route_str) + res = Bindings.command( + conn, + channel, + command_type, + flattened_args.size, + arg_ptrs, + arg_lens, + route_buf, + route_str.bytesize, + span_ptr + ) + end + + result = convert_response(res, &block) + ensure + # Free the native CommandResult (arena + response + error) to prevent memory leak + Bindings.free_command_result(res) if res && !res.null? + + # Always drop the span if one was created, even if command fails + if span_ptr != 0 + begin + Bindings.drop_otel_span(span_ptr) + rescue StandardError => e + # Log but don't raise - span cleanup errors shouldn't break command execution + warn "Failed to drop OpenTelemetry span: #{e.message}" + end + end + end + + # Track queued commands during MULTI (except for MULTI, EXEC, DISCARD, WATCH, UNWATCH) + if @in_multi && !@queued_commands.nil? + tx_commands = [ + RequestType::MULTI, RequestType::EXEC, RequestType::DISCARD, + RequestType::WATCH, RequestType::UNWATCH + ] + if !tx_commands.include?(command_type) && result == "QUEUED" + @queued_commands << [command_type, command_args.dup, block] + end end - end - result + result + ensure + release_connection_slot + end end private # Returns the live native client handle, raising if the client has been - # closed. + # closed. Kept for callers that only need a fast fail-fast check without + # participating in the close-drain protocol (e.g. the early-out branch in + # `send_batch_commands` before it delegates back to `#send_command`). + # New FFI dispatch sites should use `#acquire_connection_slot` + + # `#release_connection_slot` instead so `#close` can drain them safely. def connection! conn = @connection raise ConnectionError, "the client is closed" if conn.nil? || conn.null? @@ -472,6 +534,33 @@ def connection! conn end + # Reserves a slot in the in-flight counter and returns the live native + # client handle. Every caller MUST pair this with a `#release_connection_slot` + # in an `ensure` block, or `#close` will block forever waiting for the + # counter to drain. + # + # Raises ConnectionError("the client is closed") if `#close` has already + # started, or if the handle is nil / null. Once `@closing` is set, no new + # slots are handed out — this is what makes the drain in `#close` finite. + def acquire_connection_slot + @close_lock.synchronize do + raise ConnectionError, "the client is closed" if @closing + raise ConnectionError, "the client is closed" if @connection.nil? || @connection.null? + + @inflight += 1 + @connection + end + end + + # Releases a slot previously reserved by `#acquire_connection_slot`. Wakes + # a pending `#close` if this was the last in-flight command. + def release_connection_slot + @close_lock.synchronize do + @inflight -= 1 + @drain_cv.broadcast if @inflight.zero? + end + end + # Read an SSL value # Accepts a file path (String), an OpenSSL object (#to_pem / #to_der), or a fallback #to_s. def read_ssl_value(value, label) @@ -515,98 +604,104 @@ def send_batch_commands(commands, exception: true, is_atomic: false) end end - # Checked before allocating any FFI memory below, so a closed client fails fast. - conn = connection! - - cmds = [] - blocks = [] - buffers = [] # Keep references to prevent GC - - commands.each do |command_type, command_args, block| - arg_ptrs, arg_lens, arg_bufs, flattened_args = build_command_args(command_args) - - cmd = Bindings::CmdInfo.new - cmd[:request_type] = command_type - cmd[:args] = arg_ptrs - cmd[:arg_count] = flattened_args.size - cmd[:args_len] = arg_lens - - cmds << cmd - blocks << block - buffers << [arg_ptrs, arg_lens, arg_bufs] # Prevent GC - end - - # Create array of pointers to CmdInfo structs - cmd_ptrs = FFI::MemoryPointer.new(:pointer, cmds.size) - cmds.each_with_index do |cmd, i| - cmd_ptrs[i].put_pointer(0, cmd.to_ptr) - end - - batch_info = Bindings::BatchInfo.new - batch_info[:cmd_count] = cmds.size - batch_info[:cmds] = cmd_ptrs - batch_info[:is_atomic] = is_atomic - - batch_options = Bindings::BatchOptionsInfo.new - batch_options[:retry_server_error] = true - batch_options[:retry_connection_error] = true - batch_options[:has_timeout] = false - batch_options[:timeout] = 0 # No timeout - batch_options[:route_info] = FFI::Pointer::NULL - - # Create OpenTelemetry span for batch operation if sampling is enabled, as a child of - # the app's current span context when a parent_span_context_provider is registered. - span_ptr = 0 - if OpenTelemetry.should_sample? - begin - parent_ctx = OpenTelemetry.parent_span_context - span_ptr = if parent_ctx - Bindings.create_batch_otel_span_with_trace_context( - parent_ctx[:trace_id], parent_ctx[:span_id], parent_ctx[:trace_flags], parent_ctx[:tracestate] - ) - else - Bindings.create_batch_otel_span - end - rescue StandardError => e - warn "Failed to create OpenTelemetry batch span: #{e.message}" - span_ptr = 0 + # Reserve a slot before allocating any FFI memory below, so a closed + # client fails fast — and so `#close` can safely drain us before freeing + # the native handle (issue #212, race #2). Paired with + # `release_connection_slot` in the outer `ensure`. + conn = acquire_connection_slot + begin + cmds = [] + blocks = [] + buffers = [] # Keep references to prevent GC + + commands.each do |command_type, command_args, block| + arg_ptrs, arg_lens, arg_bufs, flattened_args = build_command_args(command_args) + + cmd = Bindings::CmdInfo.new + cmd[:request_type] = command_type + cmd[:args] = arg_ptrs + cmd[:arg_count] = flattened_args.size + cmd[:args_len] = arg_lens + + cmds << cmd + blocks << block + buffers << [arg_ptrs, arg_lens, arg_bufs] # Prevent GC end - end - begin - res = Bindings.batch( - conn, - 0, - batch_info, - exception, - batch_options.to_ptr, - span_ptr - ) - - results = convert_response(res) - ensure - # Free the native CommandResult (arena + response + error) to prevent memory leak - Bindings.free_command_result(res) if res && !res.null? + # Create array of pointers to CmdInfo structs + cmd_ptrs = FFI::MemoryPointer.new(:pointer, cmds.size) + cmds.each_with_index do |cmd, i| + cmd_ptrs[i].put_pointer(0, cmd.to_ptr) + end - # Always drop the span if one was created - if span_ptr != 0 + batch_info = Bindings::BatchInfo.new + batch_info[:cmd_count] = cmds.size + batch_info[:cmds] = cmd_ptrs + batch_info[:is_atomic] = is_atomic + + batch_options = Bindings::BatchOptionsInfo.new + batch_options[:retry_server_error] = true + batch_options[:retry_connection_error] = true + batch_options[:has_timeout] = false + batch_options[:timeout] = 0 # No timeout + batch_options[:route_info] = FFI::Pointer::NULL + + # Create OpenTelemetry span for batch operation if sampling is enabled, as a child of + # the app's current span context when a parent_span_context_provider is registered. + span_ptr = 0 + if OpenTelemetry.should_sample? begin - Bindings.drop_otel_span(span_ptr) + parent_ctx = OpenTelemetry.parent_span_context + span_ptr = if parent_ctx + Bindings.create_batch_otel_span_with_trace_context( + parent_ctx[:trace_id], parent_ctx[:span_id], parent_ctx[:trace_flags], parent_ctx[:tracestate] + ) + else + Bindings.create_batch_otel_span + end rescue StandardError => e - warn "Failed to drop OpenTelemetry batch span: #{e.message}" + warn "Failed to create OpenTelemetry batch span: #{e.message}" + span_ptr = 0 end end - end - # An inline error slot (see the ResponseType::ERROR case in - # convert_response above) must be left alone here - e.g. Utils::Boolify - # would otherwise silently coerce a CommandError object to `true` - # (`value != 0` is true for any non-numeric object), hiding the error. - blocks.each_with_index do |block, i| - results[i] = block.call(results[i]) if block && !results[i].is_a?(CommandError) - end + begin + res = Bindings.batch( + conn, + 0, + batch_info, + exception, + batch_options.to_ptr, + span_ptr + ) - results + results = convert_response(res) + ensure + # Free the native CommandResult (arena + response + error) to prevent memory leak + Bindings.free_command_result(res) if res && !res.null? + + # Always drop the span if one was created + if span_ptr != 0 + begin + Bindings.drop_otel_span(span_ptr) + rescue StandardError => e + warn "Failed to drop OpenTelemetry batch span: #{e.message}" + end + end + end + + # An inline error slot (see the ResponseType::ERROR case in + # convert_response above) must be left alone here - e.g. Utils::Boolify + # would otherwise silently coerce a CommandError object to `true` + # (`value != 0` is true for any non-numeric object), hiding the error. + blocks.each_with_index do |block, i| + results[i] = block.call(results[i]) if block && !results[i].is_a?(CommandError) + end + + results + ensure + release_connection_slot + end end # Builds the `periodic_checks` extra_options_json value. Accepts diff --git a/lib/valkey/commands/scripting_commands.rb b/lib/valkey/commands/scripting_commands.rb index b6c8ddc1..0cd7b987 100644 --- a/lib/valkey/commands/scripting_commands.rb +++ b/lib/valkey/commands/scripting_commands.rb @@ -260,42 +260,48 @@ def evalsha_ro(sha, *rest, keys: nil, args: nil) end def invoke_script(script, args: [], keys: []) - # Checked before allocating any FFI memory below, so a closed client fails fast. - conn = connection! - - # Must hold onto the returned buffers (_arg_bufs/_keys_bufs) for the - # lifetime of this method - they back arg_ptrs/keys_ptrs, and letting - # them go out of scope (e.g. by only capturing the first 2 return - # values) makes them eligible for GC before the native call below - # reads through those pointers, corrupting ARGV/KEYS with freed memory. - arg_ptrs, arg_lens, _arg_bufs, flattened_args = build_command_args(args) - keys_ptrs, keys_lens, _keys_bufs, flattened_keys = build_command_args(keys) - - route = "" - route_buf = FFI::MemoryPointer.from_string(route) - - # Use from_string to ensure proper null termination - sha = FFI::MemoryPointer.from_string(script) + # Reserve a slot before allocating any FFI memory below. Paired with + # `release_connection_slot` in the outer `ensure` so `#close` can drain + # us safely before freeing the native handle (issue #212, race #2). + conn = acquire_connection_slot begin - res = Bindings.invoke_script( - conn, - 0, - sha, - flattened_keys.size, - keys_ptrs, - keys_lens, - flattened_args.size, - arg_ptrs, - arg_lens, - route_buf, - route.bytesize, - 0 # span_ptr for OpenTelemetry (0 = no span) - ) - - convert_response(res) + # Must hold onto the returned buffers (_arg_bufs/_keys_bufs) for the + # lifetime of this method - they back arg_ptrs/keys_ptrs, and letting + # them go out of scope (e.g. by only capturing the first 2 return + # values) makes them eligible for GC before the native call below + # reads through those pointers, corrupting ARGV/KEYS with freed memory. + arg_ptrs, arg_lens, _arg_bufs, flattened_args = build_command_args(args) + keys_ptrs, keys_lens, _keys_bufs, flattened_keys = build_command_args(keys) + + route = "" + route_buf = FFI::MemoryPointer.from_string(route) + + # Use from_string to ensure proper null termination + sha = FFI::MemoryPointer.from_string(script) + + begin + res = Bindings.invoke_script( + conn, + 0, + sha, + flattened_keys.size, + keys_ptrs, + keys_lens, + flattened_args.size, + arg_ptrs, + arg_lens, + route_buf, + route.bytesize, + 0 # span_ptr for OpenTelemetry (0 = no span) + ) + + convert_response(res) + ensure + Bindings.free_command_result(res) if res && !res.null? + end ensure - Bindings.free_command_result(res) if res && !res.null? + release_connection_slot end end diff --git a/test/valkey/connection_lifecycle_test.rb b/test/valkey/connection_lifecycle_test.rb index e4e89046..a66f35b0 100644 --- a/test/valkey/connection_lifecycle_test.rb +++ b/test/valkey/connection_lifecycle_test.rb @@ -146,6 +146,11 @@ def test_concurrent_close_releases_native_handle_once_traced pointer = FFI::Pointer.new(1) client.instance_variable_set(:@connection, pointer) client.instance_variable_set(:@close_lock, Mutex.new) + # Also set the drain-tracking state introduced for issue #212 race #2: + # `close` now drains in-flight commands under @close_lock before freeing. + client.instance_variable_set(:@drain_cv, ConditionVariable.new) + client.instance_variable_set(:@inflight, 0) + client.instance_variable_set(:@closing, false) source_path, source_line = Valkey.instance_method(:close).source_location # Line of `@connection = nil` inside the `begin` block. Locate it @@ -196,6 +201,65 @@ def test_concurrent_close_releases_native_handle_once_traced assert_same pointer, close_calls.first end + # Stronger version of the mid-flight-close test: many worker threads all + # issuing GVL-releasing FFI commands (`Bindings.command`, `Bindings.batch`, + # `Bindings.invoke_script`) simultaneously with `close`, which must drain + # them (issue #212, race #2). Without the drain, `close_client` runs on + # the same native handle a still-live `blocking: true` FFI call is holding + # and glibc aborts with `malloc(): unaligned tcache chunk detected` + # (exit 134) — the CI signature the existing single-thread test misses. + # Fast (<= a few seconds) so we run it in the default suite. + def test_close_drains_in_flight_commands_no_crash + skip("connection lifecycle tests only run on standalone mode") if cluster_mode? + skip("skipped on JRuby: FFI + blocking: true semantics differ") if RUBY_PLATFORM == "java" + key = "lifecycle:drain" + thread_count = 16 + cmd_loops = 40 + + client = _new_client + client.set(key, "v" * 4096) + + unexpected = Queue.new + workers = Array.new(thread_count) do + Thread.new do + Thread.current.report_on_exception = false + cmd_loops.times do + case rand(3) + when 0 then client.get(key) + when 1 + client.pipelined do |p| + p.get(key) + p.get(key) + end + else client.eval("return 1") + end + end + rescue Valkey::BaseError + nil # expected once the client is closed + rescue StandardError => e + unexpected << e + end + end + + # Let commands start dispatching before racing close. + sleep 0.005 + client.close + workers.each(&:join) + + # Reaching here at all means the VM survived. Any StandardError other + # than a Valkey::BaseError is a fix regression. + raise unexpected.pop unless unexpected.empty? + + # And the client is left properly closed — new commands raise, not crash. + error = assert_raises(Valkey::ConnectionError) { client.get(key) } + assert_match(/the client is closed/, error.message) + + # And the counter stayed balanced (every acquire paired with a release); + # otherwise a future `close` on a fresh client instance with same-slot + # state would deadlock in the drain wait. + assert_equal 0, client.instance_variable_get(:@inflight) + end + # a thread closing the client while another is mid-command must raise a # catchable ConnectionError, never segfault the VM. This is the TOCTOU half # of issue #212 (R3-10): the guard reads the handle into a local, so a