From eb834abefcd4ddd78cd97a8532c2f69a585c236c Mon Sep 17 00:00:00 2001 From: Hardik Joshi Date: Mon, 31 Oct 2022 13:38:10 +0530 Subject: [PATCH 1/3] Failing spec for interrupts and exceptions --- .../internal/async/network_connection.rb | 4 +- spec/integration/threads_use_cases_spec.rb | 49 +++++++++++++++++++ spec/spec_helper.rb | 3 ++ spec/support/driver_internal_data_accessor.rb | 12 +++++ spec/support/timeout_helper.rb | 7 +++ 5 files changed, 73 insertions(+), 2 deletions(-) create mode 100644 spec/integration/threads_use_cases_spec.rb create mode 100644 spec/support/driver_internal_data_accessor.rb create mode 100644 spec/support/timeout_helper.rb diff --git a/ruby/neo4j/driver/internal/async/network_connection.rb b/ruby/neo4j/driver/internal/async/network_connection.rb index bf77daa8..c3406385 100644 --- a/ruby/neo4j/driver/internal/async/network_connection.rb +++ b/ruby/neo4j/driver/internal/async/network_connection.rb @@ -16,7 +16,7 @@ def initialize(channel, channel_pool, logger, &on_pool_shutdown) @server_version = channel.attributes[:server_version] @protocol = Messaging::BoltProtocol.for_channel(channel) @channel_pool = channel_pool - @on_pool_shutdow = on_pool_shutdown + @on_pool_shutdown = on_pool_shutdown # @release_future = java.util.concurrent.CompletableFuture.new # @clock = clock # @connection_read_timeout = Connection::ChannelAttributes.connection_read_timeout(channel) || nil @@ -104,7 +104,7 @@ def write_to_channel(message, flush = false) terminate_and_release(e.message) @log.debug("Shutting down connection pool towards #{@server_address} due to error: #{e.message}") @channel_pool.shutdown(&:close) - @on_pool_shutdow.call + @on_pool_shutdown.call # should remove routing table entry as well raise Exceptions::SessionExpiredException, e.message end diff --git a/spec/integration/threads_use_cases_spec.rb b/spec/integration/threads_use_cases_spec.rb new file mode 100644 index 00000000..741a7752 --- /dev/null +++ b/spec/integration/threads_use_cases_spec.rb @@ -0,0 +1,49 @@ +# frozen_string_literal: true + +RSpec.describe 'Session' do + describe 'thread interrupted after acquiring connection' do + context 'with thread#raise' do + it 'releases connection back into pool' do + require 'pry' + driver = Neo4j::Driver::GraphDatabase.driver(uri, basic_auth_token) + session = driver.session + session.read_transaction {} + + channel_pool = DriverInternalDataAccessor.channel_pool_from_session(session) + expect(channel_pool.busy?).to be false + + thr = Thread.new { session.read_transaction { |_tx| sleep(60) } } + + wait_for { !channel_pool.busy? } + expect(channel_pool.busy?).to be true + binding.pry + thr.raise("Bhadako!") + + wait_for { channel_pool.busy? } + expect(channel_pool.busy?).to be false + end + end + + context 'with thread#raise' do + it 'releases connection back into pool' do + require 'pry' + driver = Neo4j::Driver::GraphDatabase.driver(uri, basic_auth_token) + session = driver.session + session.read_transaction {} + + channel_pool = DriverInternalDataAccessor.channel_pool_from_session(session) + expect(channel_pool.busy?).to be false + + thr = Thread.new { session.read_transaction { |_tx| sleep(60) } } + + wait_for { !channel_pool.busy? } + expect(channel_pool.busy?).to be true + binding.pry + thr.kill + + wait_for { channel_pool.busy? } + expect(channel_pool.busy?).to be false + end + end + end +end diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb index 84f89075..2a0e0b41 100644 --- a/spec/spec_helper.rb +++ b/spec/spec_helper.rb @@ -11,6 +11,8 @@ require 'rspec/its' require 'support/driver_helper' require 'support/neo4j_cleaner' +require 'support/driver_internal_data_accessor' +require 'support/timeout_helper' RSpec.configure do |config| # Enable flags like --only-failures and --next-failure @@ -28,6 +30,7 @@ # config.include Neo4jCleaner include DriverHelper::Helper include Neo4jCleaner + include TimeoutHelper config.define_derived_metadata do |metadata| metadata[:timeout] = 9999 end diff --git a/spec/support/driver_internal_data_accessor.rb b/spec/support/driver_internal_data_accessor.rb new file mode 100644 index 00000000..1bf337f4 --- /dev/null +++ b/spec/support/driver_internal_data_accessor.rb @@ -0,0 +1,12 @@ +# frozen_string_literal: true + +module DriverInternalDataAccessor + def self.channel_pool_from_session(session) + session.instance_variable_get(:@session).instance_variable_get(:@connection).connection.instance_variable_get :@channel_pool + end + + def self.channel_pool_from_driver(driver, uri) + hash = driver.session_factory.connection_provider.instance_variable_get(:@connection_pool).instance_variable_get(:@address_to_pool) + hash.find { |address, channel_pool| address.uri == uri }.last + end +end diff --git a/spec/support/timeout_helper.rb b/spec/support/timeout_helper.rb new file mode 100644 index 00000000..eaf1b2a2 --- /dev/null +++ b/spec/support/timeout_helper.rb @@ -0,0 +1,7 @@ +# frozen_string_literal: true + +module TimeoutHelper + def wait_for(timeout = 4, sleep_interval = 0.01, &condition) + Timeout.timeout(timeout) { sleep(sleep_interval) while(condition.call) } + end +end From ca08c69ead1a6a7a71d50dbe08d4496cb8603e26 Mon Sep 17 00:00:00 2001 From: Hardik Joshi Date: Mon, 31 Oct 2022 13:49:20 +0530 Subject: [PATCH 2/3] removed debuggers --- spec/integration/threads_use_cases_spec.rb | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/spec/integration/threads_use_cases_spec.rb b/spec/integration/threads_use_cases_spec.rb index 741a7752..023da9ca 100644 --- a/spec/integration/threads_use_cases_spec.rb +++ b/spec/integration/threads_use_cases_spec.rb @@ -4,7 +4,6 @@ describe 'thread interrupted after acquiring connection' do context 'with thread#raise' do it 'releases connection back into pool' do - require 'pry' driver = Neo4j::Driver::GraphDatabase.driver(uri, basic_auth_token) session = driver.session session.read_transaction {} @@ -16,7 +15,6 @@ wait_for { !channel_pool.busy? } expect(channel_pool.busy?).to be true - binding.pry thr.raise("Bhadako!") wait_for { channel_pool.busy? } @@ -24,9 +22,8 @@ end end - context 'with thread#raise' do + context 'with thread#kill' do it 'releases connection back into pool' do - require 'pry' driver = Neo4j::Driver::GraphDatabase.driver(uri, basic_auth_token) session = driver.session session.read_transaction {} @@ -38,7 +35,6 @@ wait_for { !channel_pool.busy? } expect(channel_pool.busy?).to be true - binding.pry thr.kill wait_for { channel_pool.busy? } From 3141df37e140ed887ebdc3731968bdf5de0ae585 Mon Sep 17 00:00:00 2001 From: Hardik Joshi Date: Wed, 2 Nov 2022 22:52:37 +0530 Subject: [PATCH 3/3] spec cleanup --- spec/integration/threads_use_cases_spec.rb | 19 +++++++++++++------ spec/support/timeout_helper.rb | 4 ++-- 2 files changed, 15 insertions(+), 8 deletions(-) diff --git a/spec/integration/threads_use_cases_spec.rb b/spec/integration/threads_use_cases_spec.rb index 023da9ca..55ca82f3 100644 --- a/spec/integration/threads_use_cases_spec.rb +++ b/spec/integration/threads_use_cases_spec.rb @@ -11,13 +11,17 @@ channel_pool = DriverInternalDataAccessor.channel_pool_from_session(session) expect(channel_pool.busy?).to be false - thr = Thread.new { session.read_transaction { |_tx| sleep(60) } } + thr = Thread.new do + thr_session = driver.session + thr_session.read_transaction { |_tx| sleep(60) } + end - wait_for { !channel_pool.busy? } + wait_till { channel_pool.busy? } expect(channel_pool.busy?).to be true + thr.raise("Bhadako!") - wait_for { channel_pool.busy? } + wait_till { !channel_pool.busy? } expect(channel_pool.busy?).to be false end end @@ -31,13 +35,16 @@ channel_pool = DriverInternalDataAccessor.channel_pool_from_session(session) expect(channel_pool.busy?).to be false - thr = Thread.new { session.read_transaction { |_tx| sleep(60) } } + thr = Thread.new do + thr_session = driver.session + thr_session.read_transaction { |_tx| sleep(60) } + end - wait_for { !channel_pool.busy? } + wait_till { channel_pool.busy? } expect(channel_pool.busy?).to be true thr.kill - wait_for { channel_pool.busy? } + wait_till { !channel_pool.busy? } expect(channel_pool.busy?).to be false end end diff --git a/spec/support/timeout_helper.rb b/spec/support/timeout_helper.rb index eaf1b2a2..e3e62e06 100644 --- a/spec/support/timeout_helper.rb +++ b/spec/support/timeout_helper.rb @@ -1,7 +1,7 @@ # frozen_string_literal: true module TimeoutHelper - def wait_for(timeout = 4, sleep_interval = 0.01, &condition) - Timeout.timeout(timeout) { sleep(sleep_interval) while(condition.call) } + def wait_till(timeout = 4, sleep_interval = 0.01, &condition) + Timeout.timeout(timeout) { sleep(sleep_interval) while(!condition.call) } end end