RUBY-2132 stop connecting connections in ensure_connected (#1937)

* RUBY-2132 stop connecting connections in ensure_connected

* fix perished connection tests

* remove this test since it does not terminate thread prematurely as it claims

Co-authored-by: Oleg Pudeyev <oleg@bsdpower.com>

Author: p-mongo

lib/mongo/error.rb

@@ -169,6 +169,7 @@ def add_label(label)
 require 'mongo/error/bulk_write_error'
 require 'mongo/error/closed_stream'
 require 'mongo/error/connection_check_out_timeout'
+require 'mongo/error/connection_perished'
 require 'mongo/error/credential_check_error'
 require 'mongo/error/crypt_error'
 require 'mongo/error/extra_file_chunk'

lib/mongo/error/connection_perished.rb

@@ -0,0 +1,23 @@
+# Copyright (C) 2020 MongoDB Inc.
+#
+# Licensed under the Apache License, Version 2.0 (the "License");
+# you may not use this file except in compliance with the License.
+# You may obtain a copy of the License at
+#
+#   http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+
+module Mongo
+  class Error
+
+    # Exception raised when trying to perform operations on a connection that
+    # experienced a network error.
+    class ConnectionPerished < Error
+    end
+  end
+end

lib/mongo/server/connection.rb

@@ -152,12 +152,12 @@ def error?
       #
       # @since 2.0.0
       def connect!
+        if error?
+          raise Error::ConnectionPerished, "Connection #{generation}:#{id} for #{address.seed} is perished. Reconnecting closed or errored connections is no longer supported"
+        end
+
         if closed?
-          if Lint.enabled?
-            raise Error::LintError, "Reconnecting closed connections is no longer supported (for #{address})"
-          else
-            log_warn("Reconnecting closed connections is deprecated (for #{address})")
-          end
+          raise Error::ConnectionPerished, "Connection #{generation}:#{id} for #{address.seed} is closed. Reconnecting closed or errored connections is no longer supported"
         end
 
         unless @socket

lib/mongo/server/connection_common.rb

@@ -110,13 +110,18 @@ def ssl_options
 
       def ensure_connected
         begin
-          connect!
+          unless socket
+            raise ArgumentError, "Connection #{generation}:#{id} for #{address.seed} is not connected"
+          end
+          if @error
+            raise Error::ConnectionPerished, "Connection #{generation}:#{id} for #{address.seed} is perished"
+          end
           result = yield socket
           success = true
           result
         ensure
           unless success
-            disconnect!(reason: :error)
+            @error = true
           end
         end
       end

spec/mongo/server/connection_pool_spec.rb

@@ -735,37 +735,6 @@ def create_pool(min_pool_size)
     end
   end
 
-  context 'when the connection does not finish authenticating before the thread is killed' do
-    let!(:pool) do
-      server.pool
-    end
-
-    let(:options) do
-      { min_pool_size:0, max_pool_size: 1 }
-    end
-
-    before do
-      pool
-      ClientRegistry.instance.close_all_clients
-    end
-
-    it 'creates a new connection' do
-      t = Thread.new do
-        expect {
-          pool.with_connection do |c|
-            expect(c).to receive(:connect!).and_raise(Mongo::Error)
-            c.send(:ensure_connected) { |socket| socket}
-          end
-        }.to raise_error(Mongo::Error)
-        expect(pool.size).to be(0)
-      end
-      t.join
-
-      expect(pool.check_out).to be_a(Mongo::Server::Connection)
-      expect(pool.size).to be(1)
-    end
-  end
-
   describe '#close_idle_sockets' do
     let!(:pool) do
       server.pool

spec/mongo/server/connection_spec.rb

@@ -687,44 +687,22 @@ class ConnectionSpecTestException < Exception; end
           /Got response for request ID \d+ but expected response for request ID \d+/)
       end
 
-      context 'linting' do
-        require_linting
-
-        it 'does not permit sending over the connection' do
-          expect {
-            connection.dispatch([ query_alice ])
-          }.to raise_error(Mongo::Error::UnexpectedResponse)
-
-          expect do
-            connection.dispatch([ query_alice ]).documents
-          end.to raise_error(Mongo::Error::LintError, /Trying to deliver a message over a disconnected connection/)
-        end
-
-        it 'marks the connection no longer usable' do
-          expect {
-            connection.dispatch([ query_alice ])
-          }.to raise_error(Mongo::Error::UnexpectedResponse)
+      it 'marks connection perished' do
+        expect {
+          connection.dispatch([ query_alice ])
+        }.to raise_error(Mongo::Error::UnexpectedResponse)
 
-          expect do
-            connection.connect!
-          end.to raise_error(Mongo::Error::LintError, /Reconnecting closed connections is no longer supported/)
-        end
+        connection.should be_error
       end
 
-      context 'not linting' do
-        skip_if_linting
-
-        it 'does not affect subsequent requests but warns' do
-          expect(Mongo::Logger.logger).to receive(:warn).once.and_call_original
-
-          expect {
-            connection.dispatch([ query_alice ])
-          }.to raise_error(Mongo::Error::UnexpectedResponse)
+      it 'makes the connection no longer usable' do
+        expect {
+          connection.dispatch([ query_alice ])
+        }.to raise_error(Mongo::Error::UnexpectedResponse)
 
-          docs = connection.dispatch([ query_alice ]).documents
-          expect(docs).to_not be_empty
-          expect(docs.first['name']).to eq('alice')
-        end
+        expect {
+          connection.dispatch([ query_alice ])
+        }.to raise_error(Mongo::Error::ConnectionPerished)
       end
     end
 
@@ -847,9 +825,9 @@ class ConnectionSpecTestException < Exception; end
           end.to raise_error(Mongo::Error::SocketError)
         end
 
-        it 'disconnects and raises the exception' do
+        it 'marks connection perished' do
           result
-          expect(connection).to_not be_connected
+          expect(connection).to be_error
         end
 
         it 'disconnects connection pool' do
@@ -881,9 +859,9 @@ class ConnectionSpecTestException < Exception; end
           end.to raise_error(Mongo::Error::SocketTimeoutError)
         end
 
-        it 'disconnects the used connection' do
+        it 'marks connection perished' do
           result
-          expect(connection).to_not be_connected
+          expect(connection).to be_error
         end
 
 =begin These assertions require a working cluster with working SDAM flow, which the tests do not configure