From 406234367544a4207141230683dddaccd98fb21a Mon Sep 17 00:00:00 2001 From: Azul Date: Fri, 21 Jul 2017 08:19:20 +0200 Subject: fix: filedescriptor leak from http_adapters Now we reuse a single adapter for all requests triggered by an incoming request. Then we .terminate the adapter. Includes a regression test. --- test/functional/sample_test.rb | 58 +++++++++++++++++++++++++++++++++++++ test/remote/hkp_source_test.rb | 11 +++++-- test/remote/nicknym_source_test.rb | 4 ++- test/support/functional_test.rb | 41 ++++++++++++++++++++++++++ test/support/http_adapter_helper.rb | 19 ++++++++++++ test/support/http_stub_helper.rb | 4 +-- 6 files changed, 132 insertions(+), 5 deletions(-) create mode 100644 test/functional/sample_test.rb create mode 100644 test/support/functional_test.rb create mode 100644 test/support/http_adapter_helper.rb (limited to 'test') diff --git a/test/functional/sample_test.rb b/test/functional/sample_test.rb new file mode 100644 index 0000000..5886349 --- /dev/null +++ b/test/functional/sample_test.rb @@ -0,0 +1,58 @@ +require 'support/functional_test' + +class SampleTest < FunctionalTest + # don't parallize me... Hard to get that right with nickserver start & stop + + def run(*args) + nickserver :start + super + ensure + nickserver :stop + end + + def test_running + assert_running + end + + # def test_invalid + # assert_lookup_status 400, 'invalid' + # end + + def test_nicknym + assert_lookup_status 200, 'test@mail.bitmask.net' + end + + # Regression Tests + + def test_no_file_descriptors_leak + lookup 'test@mail.bitmask.net' + before = open_files_count + lookup 'test@mail.bitmask.net' + assert_equal before, open_files_count, 'Filedescriptors leaked' + end + + protected + + def assert_lookup_status(status, address) + assert_equal status, lookup(address).to_i + end + + def lookup(address) + run_command %Q(curl localhost:6425 #{curl_opts} -d "address=#{address}") + end + + def curl_opts + '--silent -w "%{http_code}" -o /dev/null' + end + + def open_files_count + `lsof | grep " #{nickserver_pid} " | wc -l`.to_i + end + + def run_command(command) + `#{command} 2>&1`.tap do |out| + assert ($?.exitstatus == 0), + "failed to run '#{command}':\n #{out}" + end + end +end diff --git a/test/remote/hkp_source_test.rb b/test/remote/hkp_source_test.rb index 103b8ad..ff61513 100644 --- a/test/remote/hkp_source_test.rb +++ b/test/remote/hkp_source_test.rb @@ -1,8 +1,10 @@ require 'test_helper' require 'support/celluloid_test' +require 'support/http_adapter_helper' require 'nickserver/hkp/source' class RemoteHkpSourceTest < CelluloidTest + include HttpAdapterHelper def test_key_info uid = 'elijah@riseup.net' @@ -31,12 +33,17 @@ class RemoteHkpSourceTest < CelluloidTest protected - def assert_key_info_for_uid(uid, &block) - Nickserver::Hkp::Source.new.search uid do |status, keys| + def assert_key_info_for_uid(uid) + source.search uid do |status, keys| assert_equal 200, status yield keys end rescue HTTP::ConnectionError => e skip "could not talk to hkp server: #{e}" end + + def source + Nickserver::Hkp::Source.new adapter + end + end diff --git a/test/remote/nicknym_source_test.rb b/test/remote/nicknym_source_test.rb index 7840e10..4ca3033 100644 --- a/test/remote/nicknym_source_test.rb +++ b/test/remote/nicknym_source_test.rb @@ -1,5 +1,6 @@ require 'test_helper' require 'support/celluloid_test' +require 'support/http_adapter_helper' require 'nickserver/nicknym/source' require 'nickserver/email_address' @@ -7,6 +8,7 @@ require 'nickserver/email_address' # Please note the Readme.md file in this directory # class RemoteNicknymSourceTest < CelluloidTest + include HttpAdapterHelper def test_availablility_check source.available_for? 'mail.bitmask.net' @@ -42,7 +44,7 @@ class RemoteNicknymSourceTest < CelluloidTest end def source - @source ||= Nickserver::Nicknym::Source.new + @source ||= Nickserver::Nicknym::Source.new adapter end def email_with_key diff --git a/test/support/functional_test.rb b/test/support/functional_test.rb new file mode 100644 index 0000000..4ebc40a --- /dev/null +++ b/test/support/functional_test.rb @@ -0,0 +1,41 @@ +require 'minitest/autorun' +require 'minitest/pride' + +class FunctionalTest < Minitest::Test + + protected + + def nickserver_pid + status = nickserver "status" + /process id (\d*)\./.match(status)[1] + end + + def assert_running + status = nickserver "status" + assert_includes status, "Nickserver running" + end + + def assert_stopped + status = nickserver "status" + assert_includes status, "No nickserver processes are running." + end + + def assert_command_runs(command) + out = nickserver command + assert ($?.exitstatus == 0), + "failed to run 'nickserver #{command}':\n #{out}" + end + + def nickserver(command) + self.class.nickserver command + end + + def self.nickserver(command) + `#{path_to_executable} #{command} 2>&1` + end + + def self.path_to_executable + File.expand_path(File.dirname(__FILE__) + '/../../bin/nickserver') + end + +end diff --git a/test/support/http_adapter_helper.rb b/test/support/http_adapter_helper.rb new file mode 100644 index 0000000..6817e1e --- /dev/null +++ b/test/support/http_adapter_helper.rb @@ -0,0 +1,19 @@ +require 'nickserver/adapters/celluloid_http' + +module HttpAdapterHelper + + def setup + super + @adapter = Nickserver::Adapters::CelluloidHttp.new + end + + def teardown + @adapter.terminate + super + end + + protected + + attr_reader :adapter + +end diff --git a/test/support/http_stub_helper.rb b/test/support/http_stub_helper.rb index dd3d1b2..c9f2bfa 100644 --- a/test/support/http_stub_helper.rb +++ b/test/support/http_stub_helper.rb @@ -1,9 +1,9 @@ -require 'nickserver/source' +require 'nickserver/reel_server' module HttpStubHelper def stubbing_http - Nickserver::Source::DEFAULT_ADAPTER_CLASS.stub :new, adapter do + Nickserver::ReelServer::DEFAULT_ADAPTER_CLASS.stub :new, adapter do yield end adapter.verify -- cgit v1.2.3