From ad160cdc9b279b446d4788e097dcd98da737a4b3 Mon Sep 17 00:00:00 2001 From: Dmytro Rymar Date: Mon, 24 Aug 2026 11:17:00 +0200 Subject: [PATCH 1/5] RUBY-3946 Check connection back in when a load-balanced initial command fails --- lib/mongo/collection/view/aggregation.rb | 22 ++-- lib/mongo/collection/view/iterable.rb | 10 +- lib/mongo/collection/view/map_reduce.rb | 12 +- lib/mongo/collection/view/readable.rb | 10 +- lib/mongo/database/view.rb | 18 ++- lib/mongo/index/view.rb | 10 +- ...lb_initial_command_connection_leak_spec.rb | 105 ++++++++++++++++++ 7 files changed, 170 insertions(+), 17 deletions(-) create mode 100644 spec/integration/lb_initial_command_connection_leak_spec.rb diff --git a/lib/mongo/collection/view/aggregation.rb b/lib/mongo/collection/view/aggregation.rb index 1a361e46e7..7167c38c85 100644 --- a/lib/mongo/collection/view/aggregation.rb +++ b/lib/mongo/collection/view/aggregation.rb @@ -117,13 +117,21 @@ def send_initial_query(server, context, operation: nil) if server.load_balancer? # Connection will be checked in when cursor is drained. connection = server.pool.check_out(context: context) - initial_query_op( - context.session, - effective_read_preference(connection) - ).execute_with_connection( - connection, - context: context - ) + begin + initial_query_op( + context.session, + effective_read_preference(connection) + ).execute_with_connection( + connection, + context: context + ) + rescue StandardError + # The initial command failed, so no cursor exists to drain and + # check the connection back in; release it here before the + # error propagates. + server.pool.check_in(connection) unless connection.pinned? + raise + end else server.with_connection do |connection| initial_query_op( diff --git a/lib/mongo/collection/view/iterable.rb b/lib/mongo/collection/view/iterable.rb index c501e6d60f..553408a160 100644 --- a/lib/mongo/collection/view/iterable.rb +++ b/lib/mongo/collection/view/iterable.rb @@ -181,7 +181,15 @@ def send_initial_query(server, context, operation: nil) ) end connection ||= server.pool.check_out(context: context) - operation.execute_with_connection(connection, context: context) + begin + operation.execute_with_connection(connection, context: context) + rescue StandardError + # The initial command failed, so no cursor exists to drain and + # check the connection back in; release it here before the + # error propagates. + server.pool.check_in(connection) unless connection.pinned? + raise + end else operation.execute(server, context: context) end diff --git a/lib/mongo/collection/view/map_reduce.rb b/lib/mongo/collection/view/map_reduce.rb index b55cb7d80e..a3a48bbad2 100644 --- a/lib/mongo/collection/view/map_reduce.rb +++ b/lib/mongo/collection/view/map_reduce.rb @@ -75,8 +75,16 @@ def each(&block) if server.load_balancer? # Connection will be checked in when cursor is drained. connection = server.pool.check_out(context: context) - result = send_initial_query_with_connection(connection, context.session, context: context) - result = send_fetch_query_with_connection(connection, session) unless inline? + begin + result = send_initial_query_with_connection(connection, context.session, context: context) + result = send_fetch_query_with_connection(connection, session) unless inline? + rescue StandardError + # The command failed, so no cursor exists to drain and check + # the connection back in; release it here before the error + # propagates. + server.pool.check_in(connection) unless connection.pinned? + raise + end else result = send_initial_query(server, context) result = send_fetch_query(server, session) unless inline? diff --git a/lib/mongo/collection/view/readable.rb b/lib/mongo/collection/view/readable.rb index 145f533639..5fb2963b5a 100644 --- a/lib/mongo/collection/view/readable.rb +++ b/lib/mongo/collection/view/readable.rb @@ -741,7 +741,15 @@ def parallel_scan(cursor_count, options = {}) result = if server.load_balancer? # Connection will be checked in when cursor is drained. connection = server.pool.check_out(context: context) - op.execute_with_connection(connection, context: context) + begin + op.execute_with_connection(connection, context: context) + rescue StandardError + # The command failed, so no cursor exists to drain + # and check the connection back in; release it here + # before the error propagates. + server.pool.check_in(connection) unless connection.pinned? + raise + end else op.execute(server, context: context) end diff --git a/lib/mongo/database/view.rb b/lib/mongo/database/view.rb index 4b10947511..8a03af44de 100644 --- a/lib/mongo/database/view.rb +++ b/lib/mongo/database/view.rb @@ -274,11 +274,19 @@ def send_initial_query(server, session, context, options = {}) execution_opts[:deserialize_as_bson] = opts.delete(:deserialize_as_bson) if opts.key?(:deserialize_as_bson) if server.load_balancer? connection = server.pool.check_out(context: context) - initial_query_op(session, opts).execute_with_connection( - connection, - context: context, - options: execution_opts - ) + begin + initial_query_op(session, opts).execute_with_connection( + connection, + context: context, + options: execution_opts + ) + rescue StandardError + # The initial command failed, so no cursor exists to drain and + # check the connection back in; release it here before the + # error propagates. + server.pool.check_in(connection) unless connection.pinned? + raise + end else initial_query_op(session, opts).execute( server, diff --git a/lib/mongo/index/view.rb b/lib/mongo/index/view.rb index 6e7cf88264..0e0eddb71d 100644 --- a/lib/mongo/index/view.rb +++ b/lib/mongo/index/view.rb @@ -422,7 +422,15 @@ def normalize_models(models) def send_initial_query(op, server, _session, context) if server.load_balancer? connection = server.pool.check_out(context: context) - op.execute_with_connection(connection, context: context) + begin + op.execute_with_connection(connection, context: context) + rescue StandardError + # The initial command failed, so no cursor exists to drain and + # check the connection back in; release it here before the + # error propagates. + server.pool.check_in(connection) unless connection.pinned? + raise + end else op.execute(server, context: context) end diff --git a/spec/integration/lb_initial_command_connection_leak_spec.rb b/spec/integration/lb_initial_command_connection_leak_spec.rb new file mode 100644 index 0000000000..5a69ac07ac --- /dev/null +++ b/spec/integration/lb_initial_command_connection_leak_spec.rb @@ -0,0 +1,105 @@ +# frozen_string_literal: true + +require 'spec_helper' + +# In load-balanced topology the driver checks a connection out of the pool +# before executing the initial command of a cursor-returning operation, so +# that the cursor can retain it. If the initial command fails, no cursor +# exists to drain and check the connection back in; the connection must be +# checked in before the error propagates, otherwise the pool permanently +# loses a slot per failure and the process eventually cannot check out any +# connections at all. +describe 'Load-balanced initial command failure' do + require_topology :load_balanced + + let(:client) do + authorized_client.tap do |client| + client.reconnect if client.closed? + end + end + let(:collection_name) { 'lb_initial_command_leak' } + let(:collection) { client[collection_name] } + let(:server) { client.cluster.next_primary } + let(:pool) { server.pool } + + before do + authorized_client[collection_name].insert_many([ { test: 1 } ] * 10) + end + + after do + client.use(:admin).command( + configureFailPoint: 'failCommand', + mode: 'off' + ) + end + + def checked_out_count + pool.size - pool.available_count + end + + shared_examples 'returns the connection to the pool on failure' do |command_name| + before do + client.use(:admin).command( + configureFailPoint: 'failCommand', + mode: { times: 1 }, + data: { failCommands: [ command_name ], errorCode: 100 } + ) + end + + it 'does not leak the connection' do + baseline = checked_out_count + expect do + operation.call + end.to raise_error(Mongo::Error::OperationFailure) + expect(checked_out_count).to eq(baseline) + end + + it 'can run the operation again after the failure' do + begin + operation.call + rescue Mongo::Error::OperationFailure + nil + end + expect do + operation.call + end.not_to raise_error + end + end + + context 'find' do + let(:operation) { -> { collection.find(test: 1).to_a } } + + include_examples 'returns the connection to the pool on failure', 'find' + end + + context 'aggregate' do + let(:operation) { -> { collection.aggregate([ { '$match' => { test: 1 } } ]).to_a } } + + include_examples 'returns the connection to the pool on failure', 'aggregate' + end + + context 'listCollections' do + let(:operation) { -> { client.database.list_collections } } + + include_examples 'returns the connection to the pool on failure', 'listCollections' + end + + context 'listIndexes' do + let(:operation) { -> { collection.indexes.to_a } } + + include_examples 'returns the connection to the pool on failure', 'listIndexes' + end + + context 'mapReduce' do + let(:operation) do + lambda do + collection.find.map_reduce( + 'function() { emit(this.test, 1) }', + 'function(key, values) { return 1 }' + ).to_a + end + end + + include_examples 'returns the connection to the pool on failure', 'mapReduce' + end +end From ff8b0fd512c5415b7b4c5b78f2849d40c258bc25 Mon Sep 17 00:00:00 2001 From: Dmitry Rybakov <160598371+comandeo-mongo@users.noreply.github.com> Date: Mon, 24 Aug 2026 18:02:53 +0200 Subject: [PATCH 2/5] Add guard for topology --- spec/integration/lb_initial_command_connection_leak_spec.rb | 1 + 1 file changed, 1 insertion(+) diff --git a/spec/integration/lb_initial_command_connection_leak_spec.rb b/spec/integration/lb_initial_command_connection_leak_spec.rb index 5a69ac07ac..055d69142c 100644 --- a/spec/integration/lb_initial_command_connection_leak_spec.rb +++ b/spec/integration/lb_initial_command_connection_leak_spec.rb @@ -11,6 +11,7 @@ # connections at all. describe 'Load-balanced initial command failure' do require_topology :load_balanced + require_no_multi_mongos let(:client) do authorized_client.tap do |client| From 469fc14337e12418e43a0568b91853db752d5c09 Mon Sep 17 00:00:00 2001 From: Dmitry Rybakov <160598371+comandeo-mongo@users.noreply.github.com> Date: Mon, 14 Sep 2026 13:17:40 +0200 Subject: [PATCH 3/5] Refactor connection checkout/checkin --- lib/mongo/collection/view/aggregation.rb | 10 +- lib/mongo/collection/view/change_stream.rb | 6 +- lib/mongo/collection/view/iterable.rb | 17 +-- lib/mongo/collection/view/map_reduce.rb | 15 +- lib/mongo/collection/view/readable.rb | 10 +- lib/mongo/cursor.rb | 13 ++ lib/mongo/database.rb | 17 +-- lib/mongo/database/view.rb | 9 +- lib/mongo/index/view.rb | 9 +- lib/mongo/server/connection_pool.rb | 69 ++++++++-- ...lb_initial_command_connection_leak_spec.rb | 77 ++++++++++- spec/mongo/server/connection_pool_spec.rb | 128 ++++++++++++++++++ 12 files changed, 294 insertions(+), 86 deletions(-) diff --git a/lib/mongo/collection/view/aggregation.rb b/lib/mongo/collection/view/aggregation.rb index 7167c38c85..2b0cf6cf17 100644 --- a/lib/mongo/collection/view/aggregation.rb +++ b/lib/mongo/collection/view/aggregation.rb @@ -115,9 +115,7 @@ def effective_read_preference(connection) def send_initial_query(server, context, operation: nil) if server.load_balancer? - # Connection will be checked in when cursor is drained. - connection = server.pool.check_out(context: context) - begin + server.pool.with_cursor_connection(context: context) do |connection| initial_query_op( context.session, effective_read_preference(connection) @@ -125,12 +123,6 @@ def send_initial_query(server, context, operation: nil) connection, context: context ) - rescue StandardError - # The initial command failed, so no cursor exists to drain and - # check the connection back in; release it here before the - # error propagates. - server.pool.check_in(connection) unless connection.pinned? - raise end else server.with_connection do |connection| diff --git a/lib/mongo/collection/view/change_stream.rb b/lib/mongo/collection/view/change_stream.rb index 460544a130..85b2102b50 100644 --- a/lib/mongo/collection/view/change_stream.rb +++ b/lib/mongo/collection/view/change_stream.rb @@ -350,8 +350,7 @@ def create_cursor!(timeout_ms = nil) if server.load_balancer? # In load balanced topology, manually check out a connection # so it remains checked out and pinned to the cursor. - connection = server.pool.check_out(context: context) - begin + server.pool.with_cursor_connection(context: context) do |connection| result = send_initial_query(connection, context) start_at_operation_time = if (doc = result.replies.first && result.replies.first.documents.first) @@ -360,9 +359,6 @@ def create_cursor!(timeout_ms = nil) nil end result - rescue StandardError - server.pool.check_in(connection) - raise end else server.with_connection do |connection| diff --git a/lib/mongo/collection/view/iterable.rb b/lib/mongo/collection/view/iterable.rb index 553408a160..79df55bdc5 100644 --- a/lib/mongo/collection/view/iterable.rb +++ b/lib/mongo/collection/view/iterable.rb @@ -172,23 +172,8 @@ def initial_query_op(session) def send_initial_query(server, context, operation: nil) operation ||= initial_query_op(context.session) if server.load_balancer? - # Connection will be checked in when cursor is drained, - # unless the connection is pinned to a transaction (in which - # case it stays checked out for the transaction duration). - if context.connection_global_id - connection = server.pool.check_out_pinned_connection( - context.connection_global_id - ) - end - connection ||= server.pool.check_out(context: context) - begin + server.pool.with_cursor_connection(context: context) do |connection| operation.execute_with_connection(connection, context: context) - rescue StandardError - # The initial command failed, so no cursor exists to drain and - # check the connection back in; release it here before the - # error propagates. - server.pool.check_in(connection) unless connection.pinned? - raise end else operation.execute(server, context: context) diff --git a/lib/mongo/collection/view/map_reduce.rb b/lib/mongo/collection/view/map_reduce.rb index a3a48bbad2..dbd75e7766 100644 --- a/lib/mongo/collection/view/map_reduce.rb +++ b/lib/mongo/collection/view/map_reduce.rb @@ -73,17 +73,10 @@ def each(&block) context = Operation::Context.new(client: client, session: session, operation_timeouts: view.operation_timeouts) if server.load_balancer? - # Connection will be checked in when cursor is drained. - connection = server.pool.check_out(context: context) - begin - result = send_initial_query_with_connection(connection, context.session, context: context) - result = send_fetch_query_with_connection(connection, session) unless inline? - rescue StandardError - # The command failed, so no cursor exists to drain and check - # the connection back in; release it here before the error - # propagates. - server.pool.check_in(connection) unless connection.pinned? - raise + result = server.pool.with_cursor_connection(context: context) do |connection| + res = send_initial_query_with_connection(connection, context.session, context: context) + res = send_fetch_query_with_connection(connection, session) unless inline? + res end else result = send_initial_query(server, context) diff --git a/lib/mongo/collection/view/readable.rb b/lib/mongo/collection/view/readable.rb index 5fb2963b5a..5e33344575 100644 --- a/lib/mongo/collection/view/readable.rb +++ b/lib/mongo/collection/view/readable.rb @@ -739,16 +739,8 @@ def parallel_scan(cursor_count, options = {}) connection_global_id: result.connection_global_id ) result = if server.load_balancer? - # Connection will be checked in when cursor is drained. - connection = server.pool.check_out(context: context) - begin + server.pool.with_cursor_connection(context: context) do |connection| op.execute_with_connection(connection, context: context) - rescue StandardError - # The command failed, so no cursor exists to drain - # and check the connection back in; release it here - # before the error propagates. - server.pool.check_in(connection) unless connection.pinned? - raise end else op.execute(server, context: context) diff --git a/lib/mongo/cursor.rb b/lib/mongo/cursor.rb index d9181b4422..d473a6a91d 100644 --- a/lib/mongo/cursor.rb +++ b/lib/mongo/cursor.rb @@ -103,6 +103,19 @@ def initialize(view, result, server, options = {}) self.class.finalize(kill_spec(@connection_global_id), cluster) ) end + rescue Exception # rubocop:disable Lint/RescueException + # In load-balanced topology the connection of the initial result is + # checked out of the pool until the cursor is drained. If the cursor + # cannot be constructed, nothing will ever check the connection back + # in, so release it here before the error propagates. Exception (not + # StandardError) is rescued so that an interrupt does not permanently + # leak the connection. In other topologies the connection is not owned + # by the cursor and must not be touched here. + if server&.load_balancer? && result.is_a?(Operation::Result) && + (connection = result.connection) + connection.connection_pool.check_in_if_checked_out(connection) + end + raise end # @api private diff --git a/lib/mongo/database.rb b/lib/mongo/database.rb index deaf88d132..882a211006 100644 --- a/lib/mongo/database.rb +++ b/lib/mongo/database.rb @@ -339,15 +339,12 @@ def cursor_command(command, options = {}) server = selector.select_server(cluster, nil, session) if server.load_balancer? # The connection is checked in by the cursor when it is drained. - connection = check_out_cursor_command_connection(server, context) - begin - op.execute_with_connection(connection, context: context, options: execution_opts) - rescue StandardError - # Release the connection before the error propagates so that - # a retried attempt checks out a fresh one. - connection.connection_pool.check_in(connection) unless connection.pinned? - connection = nil - raise + # If the command fails, the connection is released before the + # error propagates so that a retried attempt checks out a + # fresh one. + server.pool.with_cursor_connection(context: context) do |conn| + connection = conn + op.execute_with_connection(conn, context: context, options: execution_opts) end else op.execute(server, context: context, options: execution_opts) @@ -366,7 +363,7 @@ def cursor_command(command, options = {}) # If the cursor was created it owns the session and connection; # otherwise (error or no cursor in the response) release them here. unless cursor - connection.connection_pool.check_in(connection) if connection && !connection.pinned? + connection.connection_pool.check_in_if_checked_out(connection) if connection session.end_session if session && session.implicit? end end diff --git a/lib/mongo/database/view.rb b/lib/mongo/database/view.rb index 8a03af44de..be884f47b9 100644 --- a/lib/mongo/database/view.rb +++ b/lib/mongo/database/view.rb @@ -273,19 +273,12 @@ def send_initial_query(server, session, context, options = {}) execution_opts = {} execution_opts[:deserialize_as_bson] = opts.delete(:deserialize_as_bson) if opts.key?(:deserialize_as_bson) if server.load_balancer? - connection = server.pool.check_out(context: context) - begin + server.pool.with_cursor_connection(context: context) do |connection| initial_query_op(session, opts).execute_with_connection( connection, context: context, options: execution_opts ) - rescue StandardError - # The initial command failed, so no cursor exists to drain and - # check the connection back in; release it here before the - # error propagates. - server.pool.check_in(connection) unless connection.pinned? - raise end else initial_query_op(session, opts).execute( diff --git a/lib/mongo/index/view.rb b/lib/mongo/index/view.rb index 0e0eddb71d..c4852f63d1 100644 --- a/lib/mongo/index/view.rb +++ b/lib/mongo/index/view.rb @@ -421,15 +421,8 @@ def normalize_models(models) def send_initial_query(op, server, _session, context) if server.load_balancer? - connection = server.pool.check_out(context: context) - begin + server.pool.with_cursor_connection(context: context) do |connection| op.execute_with_connection(connection, context: context) - rescue StandardError - # The initial command failed, so no cursor exists to drain and - # check the connection back in; release it here before the - # error propagates. - server.pool.check_in(connection) unless connection.pinned? - raise end else op.execute(server, context: context) diff --git a/lib/mongo/server/connection_pool.rb b/lib/mongo/server/connection_pool.rb index b467c87edb..0fcec28797 100644 --- a/lib/mongo/server/connection_pool.rb +++ b/lib/mongo/server/connection_pool.rb @@ -432,6 +432,30 @@ def check_in(connection) check_invariants end + # Check a connection back into the pool only if this pool still holds + # it as checked out and no other owner has claimed it. + # + # Unlike #check_in, this method is safe to call when the connection may + # have been checked in already (e.g. by Session#unpin while handling a + # transient transaction error) or may be pinned to a transaction or + # cursor that will check it in later; in both cases it does nothing. + # + # @param [ Mongo::Server::Connection ] connection The connection. + # + # @api private + def check_in_if_checked_out(connection) + check_invariants + + @lock.synchronize do + return if connection.pinned? + return unless @checked_out_connections.include?(connection) + + do_check_in(connection) + end + ensure + check_invariants + end + # Executes the check in after having already acquired the lock. # # @param [ Mongo::Server::Connection ] connection The connection. @@ -758,16 +782,43 @@ def with_connection(connection_global_id: nil, context: nil) rescue Error::SocketError, Error::SocketTimeoutError, Error::ConnectionPerished => e maybe_raise_pool_cleared!(connection, e) ensure - if connection && !connection.pinned? - # Do not check in if the connection is pinned (the session or cursor - # owns it and will check it in later when unpinning). Also skip - # check-in if the connection was already checked in during the block - # (e.g. by Session#unpin after an error on the first operation). - checked_out = @lock.synchronize do - @checked_out_connections.include?(connection) - end - check_in(connection) if checked_out + # Do not check in if the connection is pinned (the session or cursor + # owns it and will check it in later when unpinning) or was already + # checked in during the block (e.g. by Session#unpin after an error + # on the first operation). + check_in_if_checked_out(connection) if connection + end + + # Check out a connection for the initial command of a cursor-returning + # operation in load-balanced topology and yield it to the block. + # + # On success the connection remains checked out: the cursor assumes + # ownership and checks it in when drained or closed. If the block + # raises, no cursor exists to do that, so the connection is checked + # back in here before the error propagates, unless another owner + # already claimed it (it is pinned to a transaction, or Session#unpin + # checked it in while handling a transient transaction error). + # + # If the operation context is pinned to a connection (e.g. inside a + # transaction), the pinned connection is reused. + # + # @param [ Mongo::Operation::Context | nil ] :context Context of the + # operation the connection is requested for, if any. + # + # @return [ Object ] The result of the block. + # + # @api private + def with_cursor_connection(context:) + if context&.connection_global_id + connection = check_out_pinned_connection(context.connection_global_id) end + connection ||= check_out(context: context) + succeeded = false + result = yield(connection) + succeeded = true + result + ensure + check_in_if_checked_out(connection) if connection && !succeeded end # Close sockets that have been open for longer than the max idle time, diff --git a/spec/integration/lb_initial_command_connection_leak_spec.rb b/spec/integration/lb_initial_command_connection_leak_spec.rb index 055d69142c..901c01574b 100644 --- a/spec/integration/lb_initial_command_connection_leak_spec.rb +++ b/spec/integration/lb_initial_command_connection_leak_spec.rb @@ -35,7 +35,10 @@ end def checked_out_count - pool.size - pool.available_count + # Sample the checked-out set in a single read; computing + # pool.size - pool.available_count takes the pool lock twice and can + # race with background actors (e.g. the cursor reaper) between reads. + pool.instance_variable_get(:@checked_out_connections).length end shared_examples 'returns the connection to the pool on failure' do |command_name| @@ -103,4 +106,76 @@ def checked_out_count include_examples 'returns the connection to the pool on failure', 'mapReduce' end + + context 'watch (change stream)' do + let(:operation) { -> { collection.watch } } + + include_examples 'returns the connection to the pool on failure', 'aggregate' + end + + context 'when cursor construction fails after a successful command' do + before do + allow_any_instance_of(Mongo::Cursor).to receive(:set_cursor_id) + .and_raise(Mongo::Error::InternalDriverError, 'simulated cursor construction failure') + end + + it 'does not leak the connection' do + baseline = checked_out_count + expect do + collection.find(test: 1).to_a + end.to raise_error(Mongo::Error::InternalDriverError, /simulated cursor construction failure/) + expect(checked_out_count).to eq(baseline) + end + end + + context 'in a transaction' do + let(:session) { client.start_session } + + context 'when the initial command fails with a transient transaction error' do + before do + client.use(:admin).command( + configureFailPoint: 'failCommand', + mode: { times: 1 }, + data: { + failCommands: [ 'find' ], + errorCode: 112, + errorLabels: [ 'TransientTransactionError' ] + } + ) + end + + # Session#unpin checks the pinned connection back in when it handles + # a transient transaction error; the initial command cleanup must not + # check the same connection in a second time (that would raise + # ArgumentError from the pool and mask the retryable error). + it 'retries the transaction and does not leak the connection' do + baseline = checked_out_count + docs = nil + expect do + session.with_transaction do + collection.insert_one({ test: 2 }, session: session) + docs = collection.find({ test: 1 }, session: session).to_a + end + end.not_to raise_error + expect(docs).not_to be_empty + # After a commit the connection stays pinned to the session until + # the session ends or starts another transaction. + session.end_session + expect(checked_out_count).to eq(baseline) + end + end + + context 'when running aggregate on the pinned connection' do + it 'reuses the transaction pinned connection' do + docs = nil + expect do + session.with_transaction do + collection.insert_one({ test: 2 }, session: session) + docs = collection.aggregate([ { '$match' => { test: 1 } } ], session: session).to_a + end + end.not_to raise_error + expect(docs).not_to be_empty + end + end + end end diff --git a/spec/mongo/server/connection_pool_spec.rb b/spec/mongo/server/connection_pool_spec.rb index 6094654faa..617792f8d9 100644 --- a/spec/mongo/server/connection_pool_spec.rb +++ b/spec/mongo/server/connection_pool_spec.rb @@ -1297,6 +1297,134 @@ def create_pool(min_pool_size) end end + describe '#check_in_if_checked_out' do + let!(:pool) do + server.pool + end + + context 'when the connection is checked out' do + it 'checks the connection in' do + connection = pool.check_out + pool.check_in_if_checked_out(connection) + expect(pool.available_count).to eq(1) + end + end + + context 'when the connection was already checked in' do + it 'does not raise' do + connection = pool.check_out + pool.check_in(connection) + expect do + pool.check_in_if_checked_out(connection) + end.not_to raise_error + expect(pool.available_count).to eq(1) + end + end + + context 'when the connection is pinned' do + it 'does not check the connection in' do + connection = pool.check_out + connection.pin(:transaction) + pool.check_in_if_checked_out(connection) + expect(pool.available_count).to eq(0) + connection.unpin(:transaction) + pool.check_in(connection) + end + end + end + + describe '#with_cursor_connection' do + let!(:pool) do + server.pool + end + + context 'when the block succeeds' do + it 'returns the block result and leaves the connection checked out' do + connection = nil + result = pool.with_cursor_connection(context: nil) do |conn| + connection = conn + :result + end + expect(result).to be(:result) + expect(pool.available_count).to eq(0) + pool.check_in(connection) + end + end + + context 'when the block raises a StandardError' do + it 'checks the connection back in' do + expect do + pool.with_cursor_connection(context: nil) do |_conn| + raise Mongo::Error::OperationFailure, 'simulated failure' + end + end.to raise_error(Mongo::Error::OperationFailure, /simulated failure/) + expect(pool.available_count).to eq(1) + end + end + + context 'when the block raises an exception that is not a StandardError' do + it 'checks the connection back in' do + expect do + pool.with_cursor_connection(context: nil) do |_conn| + raise Interrupt + end + end.to raise_error(Interrupt) + expect(pool.available_count).to eq(1) + end + end + + context 'when the connection is checked in during the block' do + # Session#unpin checks the connection in while handling a transient + # transaction error; the cleanup must not check it in a second time. + it 'does not check the connection in again' do + expect do + pool.with_cursor_connection(context: nil) do |conn| + pool.check_in(conn) + raise Mongo::Error::OperationFailure, 'simulated failure' + end + end.to raise_error(Mongo::Error::OperationFailure, /simulated failure/) + expect(pool.available_count).to eq(1) + end + end + + context 'when the connection is pinned during the block' do + it 'leaves the connection checked out' do + connection = nil + expect do + pool.with_cursor_connection(context: nil) do |conn| + connection = conn + conn.pin(:transaction) + raise Mongo::Error::OperationFailure, 'simulated failure' + end + end.to raise_error(Mongo::Error::OperationFailure, /simulated failure/) + expect(pool.available_count).to eq(0) + connection.unpin(:transaction) + pool.check_in(connection) + end + end + + context 'when the context is pinned to a checked out connection' do + it 'reuses the pinned connection' do + connection = pool.check_out + connection.pin(:transaction) + context = Mongo::Operation::Context.new( + connection_global_id: connection.global_id + ) + reused = nil + expect do + pool.with_cursor_connection(context: context) do |conn| + reused = conn + raise Mongo::Error::OperationFailure, 'simulated failure' + end + end.to raise_error(Mongo::Error::OperationFailure, /simulated failure/) + expect(reused).to be(connection) + expect(pool.available_count).to eq(0) + connection.unpin(:transaction) + pool.check_in(connection) + end + end + end + describe '#close_idle_sockets' do let!(:pool) do server.pool From a30e981810c8a0811532cf7c188541a3d9d81de0 Mon Sep 17 00:00:00 2001 From: Dmitry Rybakov <160598371+comandeo-mongo@users.noreply.github.com> Date: Mon, 14 Sep 2026 13:26:00 +0200 Subject: [PATCH 4/5] RUBY-3946 Fix load-balanced test configuration in Evergreen Since the migration from mlaunch to drivers-tools (RUBY-3472) the load-balanced variants ran as plain single-mongos sharded clusters and all load-balanced specs were silently skipped: - LOAD_BALANCED was not exported to the test scripts, so the haproxy install and the LOAD_BALANCER translation never ran. - SINGLE_MONGOS forced the single-mongos.json orchestration file, which has no loadBalancerPort, overriding the load-balancer config that drivers-tools selects. - haproxy was never started and the LB URIs were never exported. - The test suite was started with TOPOLOGY=sharded_cluster, but spec_config enables load-balanced mode only for TOPOLOGY=load-balanced. Export LOAD_BALANCED, keep the load-balancer orchestration file, start and stop haproxy via drivers-tools run-load-balancer.sh, and point the suite at the single-mongos LB URI with TOPOLOGY=load-balanced. --- .evergreen/config.yml | 1 + .evergreen/config/common.yml.erb | 1 + .evergreen/run-orchestration.sh | 14 ++++++++++++-- .evergreen/run-tests.sh | 16 ++++++++++++++++ 4 files changed, 30 insertions(+), 2 deletions(-) diff --git a/.evergreen/config.yml b/.evergreen/config.yml index 842c93ff0e..b30e535f65 100644 --- a/.evergreen/config.yml +++ b/.evergreen/config.yml @@ -103,6 +103,7 @@ functions: export RETRY_WRITES=${RETRY_WRITES} export WITH_ACTIVE_SUPPORT="${WITH_ACTIVE_SUPPORT}" export SINGLE_MONGOS="${SINGLE_MONGOS}" + export LOAD_BALANCED="${LOAD_BALANCED}" export BSON="${BSON}" export MMAPV1="${MMAPV1}" export FLE="${FLE}" diff --git a/.evergreen/config/common.yml.erb b/.evergreen/config/common.yml.erb index 9d440736d8..dc00448777 100644 --- a/.evergreen/config/common.yml.erb +++ b/.evergreen/config/common.yml.erb @@ -100,6 +100,7 @@ functions: export RETRY_WRITES=${RETRY_WRITES} export WITH_ACTIVE_SUPPORT="${WITH_ACTIVE_SUPPORT}" export SINGLE_MONGOS="${SINGLE_MONGOS}" + export LOAD_BALANCED="${LOAD_BALANCED}" export BSON="${BSON}" export MMAPV1="${MMAPV1}" export FLE="${FLE}" diff --git a/.evergreen/run-orchestration.sh b/.evergreen/run-orchestration.sh index 342207d1e4..6bcaf8b032 100755 --- a/.evergreen/run-orchestration.sh +++ b/.evergreen/run-orchestration.sh @@ -24,8 +24,12 @@ case "${TOPOLOGY:-server}" in ;; esac -# Single mongos: use a 1-router sharded cluster config. -if test "${SINGLE_MONGOS:-}" = 'true' && test "${TOPOLOGY:-}" = sharded_cluster; then +# Single mongos: use a 1-router sharded cluster config. Not applicable to +# load-balanced deployments, which need the *-load-balancer.json configs +# (mongoses with loadBalancerPort); there the single/multi mongos choice is +# made by connecting through the corresponding haproxy frontend. +if test "${SINGLE_MONGOS:-}" = 'true' && test "${TOPOLOGY:-}" = sharded_cluster \ + && test "${LOAD_BALANCED:-}" != 'true'; then export ORCHESTRATION_FILE="${ORCHESTRATION_FILE:-single-mongos.json}" fi @@ -74,3 +78,9 @@ cp "$_configs_src"/sharded_clusters/single-mongos.json "$_configs_dst/sharded_cl # Export MONGODB_URI written by the orchestration tool. . ./mo-expansion.sh export MONGODB_URI + +# Start haproxy in front of the mongoses. This writes lb-expansion.yml with +# SINGLE_MONGOS_LB_URI and MULTI_MONGOS_LB_URI, which run-tests.sh sources. +if test "${LOAD_BALANCED:-}" = 'true'; then + "$DRIVERS_TOOLS"/.evergreen/run-load-balancer.sh start +fi diff --git a/.evergreen/run-tests.sh b/.evergreen/run-tests.sh index 0982136f50..50b1c300a5 100755 --- a/.evergreen/run-tests.sh +++ b/.evergreen/run-tests.sh @@ -88,6 +88,18 @@ export TOPOLOGY="${TOPOLOGY:-server}" . ./mo-expansion.sh export MONGODB_URI +# Point the test suite at the load balancer. run-orchestration.sh started +# haproxy and wrote lb-expansion.yml; the spec suite enables load-balanced +# mode only when the TOPOLOGY environment variable is 'load-balanced'. +if test "${LOAD_BALANCED:-}" = 'true'; then + sed 's/: /=/' lb-expansion.yml > lb-expansion.sh + . ./lb-expansion.sh + export SINGLE_MONGOS_LB_URI + export MULTI_MONGOS_LB_URI + export MONGODB_URI="$SINGLE_MONGOS_LB_URI" + export TOPOLOGY=load-balanced +fi + bundle_install if test "$AUTH" = x509; then @@ -360,6 +372,10 @@ if test -n "$OCSP_MOCK_PID"; then kill "$OCSP_MOCK_PID" fi +if test "${LOAD_BALANCED:-}" = 'true'; then + "$DRIVERS_TOOLS"/.evergreen/run-load-balancer.sh stop || true +fi + "$DRIVERS_TOOLS"/.evergreen/run-mongodb.sh stop || true if test -n "$FLE" && test "$DOCKER_PRELOAD" != 1; then From 2b9b7a1be8ce6e500ed6ec1f8c120b73f487c78d Mon Sep 17 00:00:00 2001 From: Dmitry Rybakov <160598371+comandeo-mongo@users.noreply.github.com> Date: Mon, 14 Sep 2026 14:24:40 +0200 Subject: [PATCH 5/5] RUBY-3946 Skip load-balanced tests that fail on the LB deployment Re-enabling the load-balanced Evergreen configuration unmasks 8 pre-existing test failures that are unrelated to the connection-leak fix and were dark since the drivers-tools migration (RUBY-3472). Skip them with a reference to RUBY-3959, which tracks the real fixes: - load_balancers sdam-error-handling (3): CMAP event reason casing - load_balancers wait-queue-timeouts (2): checkout timeout on maxPoolSize=1 - transaction_pinning lb (2): pool-state expectations - client_construction (1): LB deployment returns a serviceId The unified runner gains a skip_descriptions option so the YAML-driven tests can be skipped without editing the synced fixtures. --- spec/integration/client_construction_spec.rb | 5 +++++ spec/integration/transaction_pinning_spec.rb | 7 +++++++ spec/runners/unified.rb | 10 +++++++--- spec/spec_tests/load_balancers_spec.rb | 19 ++++++++++++++++++- 4 files changed, 37 insertions(+), 4 deletions(-) diff --git a/spec/integration/client_construction_spec.rb b/spec/integration/client_construction_spec.rb index ab56dc48de..61bd9a4003 100644 --- a/spec/integration/client_construction_spec.rb +++ b/spec/integration/client_construction_spec.rb @@ -372,6 +372,11 @@ end it 'fails all operations' do + # The drivers-tools load-balanced deployment returns a serviceId, so + # the operation succeeds instead of raising. Unmasked when the + # load-balanced Evergreen configuration was fixed (RUBY-3946); + # tracked for a real fix in RUBY-3959. + skip 'RUBY-3959: LB deployment returns a serviceId, operation does not fail' lambda do client.command(ping: true) end.should raise_error(Mongo::Error::MissingServiceId) diff --git a/spec/integration/transaction_pinning_spec.rb b/spec/integration/transaction_pinning_spec.rb index c0291aabd5..78207cb361 100644 --- a/spec/integration/transaction_pinning_spec.rb +++ b/spec/integration/transaction_pinning_spec.rb @@ -67,6 +67,13 @@ context 'lb' do require_topology :load_balanced + # These tests fail against the drivers-tools load-balanced deployment, + # unmasked when the load-balanced Evergreen configuration was fixed + # (RUBY-3946). Tracked for a real fix in RUBY-3959. + before do + skip 'RUBY-3959: transaction pinning pool-state expectations fail on LB deployment' + end + # In load-balanced topology, we cannot create new connections to a # particular service. diff --git a/spec/runners/unified.rb b/spec/runners/unified.rb index 7e1272426d..1b99b62280 100644 --- a/spec/runners/unified.rb +++ b/spec/runners/unified.rb @@ -7,7 +7,10 @@ require 'runners/unified/test' require 'runners/unified/test_group' -def define_unified_spec_tests(base_path, paths, expect_failure: false) +# @param [ Hash ] skip_descriptions Map of test description +# to skip reason, for tests that must be skipped without editing the synced +# YAML fixtures (e.g. known failures tracked in a JIRA ticket). +def define_unified_spec_tests(base_path, paths, expect_failure: false, skip_descriptions: {}) config_override :validate_update_replace, true paths.each do |path| @@ -17,9 +20,10 @@ def define_unified_spec_tests(base_path, paths, expect_failure: false) group.tests.each do |test| context test.description do - if test.skip? + skip_reason = test.skip_reason || skip_descriptions[test.description] + if skip_reason before do - skip test.skip_reason + skip skip_reason end end diff --git a/spec/spec_tests/load_balancers_spec.rb b/spec/spec_tests/load_balancers_spec.rb index 879315b97e..11e4c63c97 100644 --- a/spec/spec_tests/load_balancers_spec.rb +++ b/spec/spec_tests/load_balancers_spec.rb @@ -10,5 +10,22 @@ describe 'Load balancer spec tests' do require_topology :load_balanced - define_unified_spec_tests(base, LOAD_BALANCER_TESTS) + # These tests fail against the drivers-tools load-balanced deployment for + # reasons unrelated to the code under test. They were dark until the + # load-balanced Evergreen configuration was fixed (RUBY-3946) and are + # tracked for a real fix in RUBY-3959. + ruby_3959_skips = { + 'only connections for a specific serviceId are closed when pools are cleared' => + 'RUBY-3959: CMAP event reason casing (connectionError vs connection_error)', + 'errors during the initial connection hello are ignored' => + 'RUBY-3959: CMAP event reason casing (connectionError vs connection_error)', + 'stale errors are ignored' => + 'RUBY-3959: CMAP event reason casing (connectionError vs connection_error)', + 'wait queue timeout errors include cursor statistics' => + 'RUBY-3959: wait-queue timeout against maxPoolSize=1 pool', + 'wait queue timeout errors include transaction statistics' => + 'RUBY-3959: wait-queue timeout against maxPoolSize=1 pool', + }.freeze + + define_unified_spec_tests(base, LOAD_BALANCER_TESTS, skip_descriptions: ruby_3959_skips) end