From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on dcvr.yhbt.net X-Spam-Level: X-Spam-ASN: X-Spam-Status: No, score=-4.0 required=3.0 tests=ALL_TRUSTED,BAYES_00 shortcircuit=no autolearn=unavailable autolearn_force=no version=3.4.0 Received: from localhost (dcvr.yhbt.net [127.0.0.1]) by dcvr.yhbt.net (Postfix) with ESMTP id F1B251FEAC for ; Mon, 6 Jun 2016 05:14:26 +0000 (UTC) From: Eric Wong To: yahns-public@yhbt.net Subject: [PATCH 2/2] wbuf_lite: fix write retries for OpenSSL sockets Date: Mon, 6 Jun 2016 05:14:21 +0000 Message-Id: <20160606051421.7082-3-e@80x24.org> In-Reply-To: <20160606051421.7082-1-e@80x24.org> References: <20160606051421.7082-1-e@80x24.org> List-Id: OpenSSL can't handle write retries if we append to an existing string. Thus we must preserve the same string object upon retrying. Do that by utilizing the underlying Wbuf class which could already handles it transparently using trysendfile. However, we still avoiding the subtlety of wbuf_close_common reliance we previously used. ref: commit 551e670281bea77e727a732ba94275265ccae5f6 ("fix output buffering with SSL_write") --- lib/yahns/wbuf_lite.rb | 46 ++++++++++++++++-------------------- test/test_proxy_pass_no_buffering.rb | 7 +++++- 2 files changed, 26 insertions(+), 27 deletions(-) diff --git a/lib/yahns/wbuf_lite.rb b/lib/yahns/wbuf_lite.rb index 5f25b2e..afee1e9 100644 --- a/lib/yahns/wbuf_lite.rb +++ b/lib/yahns/wbuf_lite.rb @@ -2,49 +2,43 @@ # Copyright (C) 2016 all contributors # License: GPL-3.0+ # frozen_string_literal: true -require_relative 'wbuf_common' +require_relative 'wbuf' # This is only used for "proxy_buffering: false" -class Yahns::WbufLite # :nodoc: - include Yahns::WbufCommon +class Yahns::WbufLite < Yahns::Wbuf # :nodoc: attr_reader :busy def initialize(req_res) + super(nil, :ignore) @req_res = req_res - @busy = false - @buf = nil end - # called only by proxy_write in proxy_http_response def wbuf_write(client, buf) - buf = buf.join if Array === buf - buf = @buf << buf if @buf # unlikely - do_write(client, buf) # try our best to avoid string copying/appending + super + rescue + @req_res = @req_res.close if @req_res + raise end - def do_write(client, buf) - case rv = client.kgio_trywrite(buf) - when String - buf = rv # continue looping - when :wait_writable, :wait_readable - @buf = buf - return @busy = rv - when nil - @buf = nil - return @busy = false - end while true + def wbuf_flush(client) + super + rescue + @req_res = @req_res.close if @req_res + raise end # called by Yahns::HttpClient#step_write - def wbuf_flush(client) - sym = do_write(client, @buf) and return sym # :wait_writable/:wait_readable + def wbuf_close(client) + wbuf_abort - # resume reading - client.hijack_cleanup - Thread.current[:yahns_queue].queue_mod(@req_res, Yahns::Queue::QEV_RD) + # resume reading when @blocked is empty + if @req_res + client.hijack_cleanup + Thread.current[:yahns_queue].queue_mod(@req_res, Yahns::Queue::QEV_RD) + end :ignore rescue - @req_res.close + @req_res = @req_res.close if @req_res raise end end diff --git a/test/test_proxy_pass_no_buffering.rb b/test/test_proxy_pass_no_buffering.rb index 48b8241..b2c8b48 100644 --- a/test/test_proxy_pass_no_buffering.rb +++ b/test/test_proxy_pass_no_buffering.rb @@ -79,6 +79,7 @@ def test_proxy_pass_no_buffering req = "GET /giant-body HTTP/1.1\r\nHost: example.com\r\n" \ "Connection: close\r\n\r\n" s.write(req) + bufs = [] sleep 1 10.times do sleep 0.1 @@ -91,10 +92,14 @@ def test_proxy_pass_no_buffering [ deleted1, deleted2 ].each do |ary| ary.delete_if { |x| x =~ /\.(?:err|out) \(deleted\)/ } end - assert_equal 0, deleted1.size, "pid1=#{deleted1.inspect}" + assert_equal 1, deleted1.size, "pid1=#{deleted1.inspect}" assert_equal 0, deleted2.size, "pid2=#{deleted2.inspect}" + bufs.push(deleted1[0]) end end + before = bufs.size + bufs.uniq! + assert bufs.size < before, 'unlinked buffer should not grow' buf = ''.dup slow = Digest::MD5.new ft = Thread.new do