From 6f01e3ff115c659f8c8e2a06c634dc11085e2178 Mon Sep 17 00:00:00 2001 From: Samuel Williams Date: Sat, 18 Apr 2020 22:36:14 +1200 Subject: [PATCH 01/12] Add specs for server-side timeout. --- lib/async/http/protocol/http1/server.rb | 4 +++- spec/async/http/protocol/shared_examples.rb | 15 +++++++++++++++ spec/async/http/server_context.rb | 2 +- 3 files changed, 19 insertions(+), 2 deletions(-) diff --git a/lib/async/http/protocol/http1/server.rb b/lib/async/http/protocol/http1/server.rb index 902d4cbd..bbdf4922 100644 --- a/lib/async/http/protocol/http1/server.rb +++ b/lib/async/http/protocol/http1/server.rb @@ -45,7 +45,9 @@ def next_request return request rescue Async::TimeoutError - fail_request(408) + # For an interesting discussion about this behaviour, see https://trac.nginx.org/nginx/ticket/1005 + # If you enable this, you will see some spec failures... + # fail_request(408) raise rescue fail_request(400) diff --git a/spec/async/http/protocol/shared_examples.rb b/spec/async/http/protocol/shared_examples.rb index 8db26a0a..7e496647 100644 --- a/spec/async/http/protocol/shared_examples.rb +++ b/spec/async/http/protocol/shared_examples.rb @@ -167,6 +167,21 @@ expect(server.scheme).to be == "http" end + it "disconnects slow clients" do + response = client.get("/") + response.read + + # We expect this connection to be closed: + connection = response.connection + + reactor.sleep(1.0) + + response = client.get("/") + response.read + + expect(connection).to_not be_reusable + end + context 'using GET method' do let(:expected) {"GET #{protocol::VERSION}"} diff --git a/spec/async/http/server_context.rb b/spec/async/http/server_context.rb index 12e9e5d9..80251bd4 100644 --- a/spec/async/http/server_context.rb +++ b/spec/async/http/server_context.rb @@ -26,7 +26,7 @@ include_context Async::RSpec::Reactor let(:protocol) {described_class} - let(:endpoint) {Async::HTTP::Endpoint.parse('http://127.0.0.1:9294', reuse_port: true)} + let(:endpoint) {Async::HTTP::Endpoint.parse('http://127.0.0.1:9294', timeout: 0.8, reuse_port: true)} let(:retries) {1} let!(:client) {Async::HTTP::Client.new(endpoint, protocol, retries: retries)} From 5b6ed31ad7a2bedc63c75c93cd8cb7fe0638c4fe Mon Sep 17 00:00:00 2001 From: Samuel Williams Date: Sat, 18 Apr 2020 22:37:02 +1200 Subject: [PATCH 02/12] Patch version bump. --- lib/async/http/version.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/async/http/version.rb b/lib/async/http/version.rb index 3d208fb8..a20a9b1a 100644 --- a/lib/async/http/version.rb +++ b/lib/async/http/version.rb @@ -22,6 +22,6 @@ module Async module HTTP - VERSION = "0.51.2" + VERSION = "0.51.3" end end From db14af66a0c5aba6c92307586adfabfe4461f510 Mon Sep 17 00:00:00 2001 From: Samuel Williams Date: Sun, 19 Apr 2020 13:02:52 +1200 Subject: [PATCH 03/12] Annotate server loop. --- lib/async/http/protocol/http1/server.rb | 2 ++ 1 file changed, 2 insertions(+) diff --git a/lib/async/http/protocol/http1/server.rb b/lib/async/http/protocol/http1/server.rb index bbdf4922..bd16ffdf 100644 --- a/lib/async/http/protocol/http1/server.rb +++ b/lib/async/http/protocol/http1/server.rb @@ -56,6 +56,8 @@ def next_request # Server loop. def each(task: Task.current) + task.annotate("#{version} reading requests for #{self.class}.") + while request = next_request response = yield(request, self) From 67bfb751d9e0257b1518cc1e53e84739a6a14bf0 Mon Sep 17 00:00:00 2001 From: Samuel Williams Date: Sun, 19 Apr 2020 13:26:26 +1200 Subject: [PATCH 04/12] Patch version bump. --- lib/async/http/version.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/async/http/version.rb b/lib/async/http/version.rb index a20a9b1a..15080383 100644 --- a/lib/async/http/version.rb +++ b/lib/async/http/version.rb @@ -22,6 +22,6 @@ module Async module HTTP - VERSION = "0.51.3" + VERSION = "0.51.4" end end From 06db9a443c75728516ac1a50288ff9dbd5e213a7 Mon Sep 17 00:00:00 2001 From: Samuel Williams Date: Sun, 19 Apr 2020 21:18:34 +1200 Subject: [PATCH 05/12] Try to improve robustness of HTTP/2 request queue. --- lib/async/http/protocol/http2/server.rb | 13 ++++++++----- spec/async/http/protocol/shared_examples.rb | 4 ++++ 2 files changed, 12 insertions(+), 5 deletions(-) diff --git a/lib/async/http/protocol/http2/server.rb b/lib/async/http/protocol/http2/server.rb index f3b8ea7c..4caa877e 100644 --- a/lib/async/http/protocol/http2/server.rb +++ b/lib/async/http/protocol/http2/server.rb @@ -52,17 +52,18 @@ def accept_stream(stream_id) end def close(error = nil) - # This invokes Framer#close which closes the stream: - super - if @requests # Stop the request loop: - @requests.enqueue nil + @requests.enqueue(nil) @requests = nil end + + super end - def each + def each(task: Task.current) + task.annotate("#{version} reading requests for #{self.class}.") + # It's possible the connection has died before we get here... @requests&.async do |task, request| task.annotate("Incoming request: #{request.method} #{request.path.inspect}.") @@ -80,6 +81,8 @@ def each request.send_response(response) end end + + # Maybe we should add some synchronisation here - i.e. only exit once all requests are finished. end end end diff --git a/spec/async/http/protocol/shared_examples.rb b/spec/async/http/protocol/shared_examples.rb index 7e496647..6c7326dd 100644 --- a/spec/async/http/protocol/shared_examples.rb +++ b/spec/async/http/protocol/shared_examples.rb @@ -180,6 +180,10 @@ response.read expect(connection).to_not be_reusable + + # client.close + # reactor.sleep(0.1) + # reactor.print_hierarchy end context 'using GET method' do From fa048923ecddd0de10062040f491c65c79134cb7 Mon Sep 17 00:00:00 2001 From: Samuel Williams Date: Sun, 19 Apr 2020 22:28:36 +1200 Subject: [PATCH 06/12] Patch version bump. --- lib/async/http/version.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/async/http/version.rb b/lib/async/http/version.rb index 15080383..e500692c 100644 --- a/lib/async/http/version.rb +++ b/lib/async/http/version.rb @@ -22,6 +22,6 @@ module Async module HTTP - VERSION = "0.51.4" + VERSION = "0.51.5" end end From c290ee368f474d7e34156c321650004edee8aece Mon Sep 17 00:00:00 2001 From: Samuel Williams Date: Tue, 21 Apr 2020 00:11:13 +1200 Subject: [PATCH 07/12] Improved annotations. --- async-http.gemspec | 2 +- lib/async/http/body/pipe.rb | 4 ++-- lib/async/http/client.rb | 2 +- lib/async/http/protocol/http1/server.rb | 2 +- lib/async/http/protocol/http2/connection.rb | 8 ++++++-- lib/async/http/protocol/http2/server.rb | 2 +- spec/async/http/proxy_spec.rb | 10 ++++++---- 7 files changed, 18 insertions(+), 12 deletions(-) diff --git a/async-http.gemspec b/async-http.gemspec index e4f2b3b8..4e3b12b4 100644 --- a/async-http.gemspec +++ b/async-http.gemspec @@ -23,7 +23,7 @@ Gem::Specification.new do |spec| spec.add_dependency("protocol-http", "~> 0.18.0") spec.add_dependency("protocol-http1", "~> 0.12.0") - spec.add_dependency("protocol-http2", "~> 0.13.0") + spec.add_dependency("protocol-http2", "~> 0.14.0") # spec.add_dependency("openssl") diff --git a/lib/async/http/body/pipe.rb b/lib/async/http/body/pipe.rb index 4ac02229..266d9da3 100644 --- a/lib/async/http/body/pipe.rb +++ b/lib/async/http/body/pipe.rb @@ -60,7 +60,7 @@ def close def reader(task) @reader = task - task.annotate "pipe reader" + task.annotate "#{self.class} reader." while chunk = @input.read @head.write(chunk) @@ -80,7 +80,7 @@ def reader(task) def writer(task) @writer = task - task.annotate "pipe writer" + task.annotate "#{self.class} writer." while chunk = @head.read_partial @output.write(chunk) diff --git a/lib/async/http/client.rb b/lib/async/http/client.rb index 17e4dc1f..837f9c2b 100755 --- a/lib/async/http/client.rb +++ b/lib/async/http/client.rb @@ -83,7 +83,7 @@ def self.open(*arguments, **options, &block) def close while @pool.busy? - Async.logger.warn(self) {"Waiting for pool to drain: #{@pool}"} + Async.logger.warn(self) {"Waiting for #{@protocol} pool to drain: #{@pool}"} @pool.wait end diff --git a/lib/async/http/protocol/http1/server.rb b/lib/async/http/protocol/http1/server.rb index bd16ffdf..ada8484a 100644 --- a/lib/async/http/protocol/http1/server.rb +++ b/lib/async/http/protocol/http1/server.rb @@ -56,7 +56,7 @@ def next_request # Server loop. def each(task: Task.current) - task.annotate("#{version} reading requests for #{self.class}.") + task.annotate("Reading #{version} requests for #{self.class}.") while request = next_request response = yield(request, self) diff --git a/lib/async/http/protocol/http2/connection.rb b/lib/async/http/protocol/http2/connection.rb index 0d381635..0e1ad449 100644 --- a/lib/async/http/protocol/http2/connection.rb +++ b/lib/async/http/protocol/http2/connection.rb @@ -66,7 +66,7 @@ def http2? end def start_connection - @reader ||= read_in_background + @reader || read_in_background end def close(error = nil) @@ -93,7 +93,11 @@ def write_frames(&block) end def read_in_background(parent: Task.current) + raise RuntimeError, "Connection is closed!" if closed? + parent.async do |task| + @reader = task + task.annotate("#{version} reading data for #{self.class}.") begin @@ -130,7 +134,7 @@ def viable? end def reusable? - !(self.closed? || @stream.closed?) + !self.closed? end def version diff --git a/lib/async/http/protocol/http2/server.rb b/lib/async/http/protocol/http2/server.rb index 4caa877e..3db051fe 100644 --- a/lib/async/http/protocol/http2/server.rb +++ b/lib/async/http/protocol/http2/server.rb @@ -62,7 +62,7 @@ def close(error = nil) end def each(task: Task.current) - task.annotate("#{version} reading requests for #{self.class}.") + task.annotate("Reading #{version} requests for #{self.class}.") # It's possible the connection has died before we get here... @requests&.async do |task, request| diff --git a/spec/async/http/proxy_spec.rb b/spec/async/http/proxy_spec.rb index 41ea475e..e7e3c09d 100644 --- a/spec/async/http/proxy_spec.rb +++ b/spec/async/http/proxy_spec.rb @@ -142,23 +142,25 @@ upstream = Async::IO::Stream.new(endpoint.connect) Async.logger.debug(self) {"Connected to #{upstream}..."} - reader = Async do + reader = Async do |task| + task.annotate "Upstream reader." + while chunk = upstream.read_partial stream.write(chunk) stream.flush end - ensure Async.logger.debug(self) {"Finished reading from upstream..."} stream.close_write end - writer = Async do + writer = Async do |task| + task.annotate "Upstream writer." + while chunk = stream.read_partial upstream.write(chunk) upstream.flush end - rescue Async::Wrapper::Cancelled #ignore ensure From f210507ad9b687f7f34a7519e2049812b7a3632d Mon Sep 17 00:00:00 2001 From: Samuel Williams Date: Tue, 21 Apr 2020 00:11:24 +1200 Subject: [PATCH 08/12] Improved connection shutdown. --- lib/async/http/protocol/http2/connection.rb | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/lib/async/http/protocol/http2/connection.rb b/lib/async/http/protocol/http2/connection.rb index 0e1ad449..27dc9be6 100644 --- a/lib/async/http/protocol/http2/connection.rb +++ b/lib/async/http/protocol/http2/connection.rb @@ -70,7 +70,16 @@ def start_connection end def close(error = nil) - @reader = nil + if @reader + @reader = nil + + # unless @reader.current? + # @reader.stop + # @reader.wait + # end + + Async::Task.current.sleep(0.1) + end super end From 84777e1ecf89e19b14a8c976ca5290364ae5f656 Mon Sep 17 00:00:00 2001 From: Samuel Williams Date: Tue, 21 Apr 2020 01:10:55 +1200 Subject: [PATCH 09/12] Use `#close_write` to force output shutdown. --- lib/async/http/protocol/http2/connection.rb | 13 ++++--------- 1 file changed, 4 insertions(+), 9 deletions(-) diff --git a/lib/async/http/protocol/http2/connection.rb b/lib/async/http/protocol/http2/connection.rb index 27dc9be6..dab3aeaa 100644 --- a/lib/async/http/protocol/http2/connection.rb +++ b/lib/async/http/protocol/http2/connection.rb @@ -70,17 +70,12 @@ def start_connection end def close(error = nil) - if @reader - @reader = nil - - # unless @reader.current? - # @reader.stop - # @reader.wait - # end - - Async::Task.current.sleep(0.1) + if @stream.connected? + @stream.close_write end + @reader = nil + super end From 489ca37c01b79e1482fcbe096355e0c8775d9d6e Mon Sep 17 00:00:00 2001 From: Samuel Williams Date: Tue, 21 Apr 2020 01:11:40 +1200 Subject: [PATCH 10/12] Prefer github actions. --- .github/workflows/development.yml | 52 +++++++++++++++++++++++++++++++ 1 file changed, 52 insertions(+) create mode 100644 .github/workflows/development.yml diff --git a/.github/workflows/development.yml b/.github/workflows/development.yml new file mode 100644 index 00000000..4b73eaaa --- /dev/null +++ b/.github/workflows/development.yml @@ -0,0 +1,52 @@ +name: Development + +on: [push, pull_request] + +jobs: + test: + runs-on: ${{matrix.os}}-latest + continue-on-error: ${{matrix.experimental}} + + strategy: + matrix: + experimental: [false] + + os: + - ubuntu + - macos + + ruby: + - 2.5 + - 2.6 + - 2.7 + + include: + - experimental: true + os: ubuntu + ruby: truffleruby + - experimental: true + os: ubuntu + ruby: jruby + - experimental: true + os: ubuntu + ruby: head + - experimental: true + os: ubuntu + ruby: 2.6 + env: COVERAGE=PartialSummary,Coveralls + + steps: + - uses: actions/checkout@v1 + - uses: ruby/setup-ruby@v1 + with: + ruby-version: ${{matrix.ruby}} + + - name: Installing packages (ubuntu) + if: matrix.os == 'ubuntu' + run: sudo apt-get install apache2-utils + + - name: Install dependencies + run: ${{matrix.env}} bundle install + + - name: Run tests + run: ${{matrix.env}} bundle exec rspec From 7a3ca44e1c0d5e1393214bafd58e993c7d41cb91 Mon Sep 17 00:00:00 2001 From: Samuel Williams Date: Tue, 21 Apr 2020 12:16:02 +1200 Subject: [PATCH 11/12] Doesn't seem to have any impact. --- lib/async/http/protocol/http2/connection.rb | 4 ---- 1 file changed, 4 deletions(-) diff --git a/lib/async/http/protocol/http2/connection.rb b/lib/async/http/protocol/http2/connection.rb index dab3aeaa..0e1ad449 100644 --- a/lib/async/http/protocol/http2/connection.rb +++ b/lib/async/http/protocol/http2/connection.rb @@ -70,10 +70,6 @@ def start_connection end def close(error = nil) - if @stream.connected? - @stream.close_write - end - @reader = nil super From 05b1a6b65605d968fb55398da83e55fde75b3604 Mon Sep 17 00:00:00 2001 From: Samuel Williams Date: Tue, 21 Apr 2020 12:17:11 +1200 Subject: [PATCH 12/12] Patch version bump. --- lib/async/http/version.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/async/http/version.rb b/lib/async/http/version.rb index e500692c..dd0f06f6 100644 --- a/lib/async/http/version.rb +++ b/lib/async/http/version.rb @@ -22,6 +22,6 @@ module Async module HTTP - VERSION = "0.51.5" + VERSION = "0.51.6" end end