* [PATCH] ANOTHER round of proxy_pass fixes
@ 2016-07-19 22:24 Eric Wong
2016-07-19 22:24 ` [PATCH 1/2] wbuf_lite: prevent clobbering responses Eric Wong
` (3 more replies)
0 siblings, 4 replies; 5+ messages in thread
From: Eric Wong @ 2016-07-19 22:24 UTC (permalink / raw)
To: yahns-public
1/2 is a critical fix, 2/2 is just a cleanup to catch further
bugs. Ugh, this proxy_pass/wbuf thing is really a PITA within
the confines of the Rack 1.x API with hijack + unhijack...
So yeah, I'm not exactly thrilled at the prospect of documenting
and supporting it; but then there's no alternative to nginx for
preforking servers...
Eric Wong (2):
wbuf_lite: prevent clobbering responses
wbuf_lite: unify EOF error handling
lib/yahns/wbuf.rb | 1 +
lib/yahns/wbuf_lite.rb | 18 ++++++------------
2 files changed, 7 insertions(+), 12 deletions(-)
^ permalink raw reply [flat|nested] 5+ messages in thread
* [PATCH 1/2] wbuf_lite: prevent clobbering responses
2016-07-19 22:24 [PATCH] ANOTHER round of proxy_pass fixes Eric Wong
@ 2016-07-19 22:24 ` Eric Wong
2016-07-19 22:24 ` [PATCH 2/2] wbuf_lite: unify EOF error handling Eric Wong
` (2 subsequent siblings)
3 siblings, 0 replies; 5+ messages in thread
From: Eric Wong @ 2016-07-19 22:24 UTC (permalink / raw)
To: yahns-public
All of our wbuf code assumes we append to existing buffers
(files) since sendfile cannot deal otherwise. We also
follow this pattern for StringIO to avoid extra data copies.
---
lib/yahns/wbuf.rb | 1 +
lib/yahns/wbuf_lite.rb | 7 ++++---
2 files changed, 5 insertions(+), 3 deletions(-)
diff --git a/lib/yahns/wbuf.rb b/lib/yahns/wbuf.rb
index 583df10..3abc5f9 100644
--- a/lib/yahns/wbuf.rb
+++ b/lib/yahns/wbuf.rb
@@ -58,6 +58,7 @@ def wbuf_write(c, buf)
end until @busy
@tmpio ||= Yahns::TmpIO.new(c.class.output_buffer_tmpdir)
+ # n.b.: we rely on O_APPEND in TmpIO, here
@sf_count += String === buf ? @tmpio.write(buf) : wbuf_writev(buf)
# we spent some time copying to the FS, try to write to
diff --git a/lib/yahns/wbuf_lite.rb b/lib/yahns/wbuf_lite.rb
index 1902ce7..8a93ad1 100644
--- a/lib/yahns/wbuf_lite.rb
+++ b/lib/yahns/wbuf_lite.rb
@@ -33,6 +33,7 @@ def wbuf_write(c, buf)
end until @busy
@tmpio ||= StringIO.new(''.dup) # relies on encoding: binary above
+ @tmpio.seek(0, 2) # fake O_APPEND behavior
@sf_count += @tmpio.write(buf)
# we spent some time copying to the FS, try to write to
@@ -45,12 +46,12 @@ def wbuf_write(c, buf)
@busy = rv
return rv
else
- raise "BUG: #{rv.nil? ? "EOF" : rv.inspect} on tmpio " \
+ raise "BUG: #{rv.nil? ? 'EOF' : rv.inspect} on "
+ "tmpio.size=#{@tmpio.size} " \
"sf_offset=#@sf_offset sf_count=#@sf_count"
end while @sf_count > 0
- # we're all caught up, try to prevent dirty data from getting flushed
- # to disk if we can help it.
+ # we're all caught up, try to save some memory if we can help it.
wbuf_abort
@sf_offset = 0
@busy = false
--
EW
^ permalink raw reply related [flat|nested] 5+ messages in thread
* [PATCH 2/2] wbuf_lite: unify EOF error handling
2016-07-19 22:24 [PATCH] ANOTHER round of proxy_pass fixes Eric Wong
2016-07-19 22:24 ` [PATCH 1/2] wbuf_lite: prevent clobbering responses Eric Wong
@ 2016-07-19 22:24 ` Eric Wong
2016-07-20 22:40 ` [PATCH 3/2] wbuf_lite: reset sf_offset/sf_count consistently Eric Wong
2016-07-21 0:57 ` [PATCH 4/2] wbuf_lite: clear @busy flag when re-arming Eric Wong
3 siblings, 0 replies; 5+ messages in thread
From: Eric Wong @ 2016-07-19 22:24 UTC (permalink / raw)
To: yahns-public
StringIO can never be truncated outside our control, so it is
a bug if we see EOF on trysendio, here.
---
lib/yahns/wbuf_lite.rb | 11 ++---------
1 file changed, 2 insertions(+), 9 deletions(-)
diff --git a/lib/yahns/wbuf_lite.rb b/lib/yahns/wbuf_lite.rb
index 8a93ad1..577b4dc 100644
--- a/lib/yahns/wbuf_lite.rb
+++ b/lib/yahns/wbuf_lite.rb
@@ -68,16 +68,9 @@ def wbuf_flush(client)
@sf_offset += rv # keep going otherwise
when :wait_writable, :wait_readable
return rv
- when nil
- # response got truncated, drop the connection
- # this may happens when using Rack::File or similar, we can't
- # keep the connection alive because we already sent our Content-Length
- # header the client would be confused.
- @wbuf_persist = false
- return wbuf_close(client)
else
- raise "BUG: rv=#{rv.inspect} " \
- "on tmpio=#{@tmpio.inspect} " \
+ raise "BUG: #{rv.nil? ? 'EOF' : rv.inspect} on "
+ "tmpio.size=#{@tmpio.size} " \
"sf_offset=#@sf_offset sf_count=#@sf_count"
end while @sf_count > 0
wbuf_close(client)
--
EW
^ permalink raw reply related [flat|nested] 5+ messages in thread
* [PATCH 3/2] wbuf_lite: reset sf_offset/sf_count consistently
2016-07-19 22:24 [PATCH] ANOTHER round of proxy_pass fixes Eric Wong
2016-07-19 22:24 ` [PATCH 1/2] wbuf_lite: prevent clobbering responses Eric Wong
2016-07-19 22:24 ` [PATCH 2/2] wbuf_lite: unify EOF error handling Eric Wong
@ 2016-07-20 22:40 ` Eric Wong
2016-07-21 0:57 ` [PATCH 4/2] wbuf_lite: clear @busy flag when re-arming Eric Wong
3 siblings, 0 replies; 5+ messages in thread
From: Eric Wong @ 2016-07-20 22:40 UTC (permalink / raw)
To: yahns-public
Since we use wbuf_lite for long, streaming requests, we need to
reset the offset and counts when aborting the existing wbuf and
not assume the wbuf goes out-of-scope and expires when we
are done using it. Fix stupid bugs in BUG notices while
we're at it :x
---
lib/yahns/wbuf_lite.rb | 13 +++++--------
1 file changed, 5 insertions(+), 8 deletions(-)
diff --git a/lib/yahns/wbuf_lite.rb b/lib/yahns/wbuf_lite.rb
index 577b4dc..7d77da9 100644
--- a/lib/yahns/wbuf_lite.rb
+++ b/lib/yahns/wbuf_lite.rb
@@ -46,14 +46,13 @@ def wbuf_write(c, buf)
@busy = rv
return rv
else
- raise "BUG: #{rv.nil? ? 'EOF' : rv.inspect} on "
+ raise "BUG: #{rv.nil? ? 'EOF' : rv.inspect} on " \
"tmpio.size=#{@tmpio.size} " \
"sf_offset=#@sf_offset sf_count=#@sf_count"
end while @sf_count > 0
# we're all caught up, try to save some memory if we can help it.
wbuf_abort
- @sf_offset = 0
@busy = false
nil
rescue
@@ -69,7 +68,7 @@ def wbuf_flush(client)
when :wait_writable, :wait_readable
return rv
else
- raise "BUG: #{rv.nil? ? 'EOF' : rv.inspect} on "
+ raise "BUG: #{rv.nil? ? 'EOF' : rv.inspect} on " \
"tmpio.size=#{@tmpio.size} " \
"sf_offset=#@sf_offset sf_count=#@sf_count"
end while @sf_count > 0
@@ -83,7 +82,7 @@ def wbuf_flush(client)
# called by Yahns::HttpClient#step_write
def wbuf_close(client)
- wbuf_abort
+ wbuf_abort if @tmpio
# resume the event loop when @blocked is empty
# The actual Yahns::ReqRes#yahns_step is actually read/write-event
@@ -102,12 +101,10 @@ def wbuf_close(client)
end
def wbuf_abort
+ @sf_offset = @sf_count = 0
# we can safely truncate since this is a StringIO, we cannot do this
# with a real file because zero-copy with sendfile means truncating
# a while could clobber in-flight data
- if @tmpio
- @tmpio.truncate(0)
- @tmpio = @tmpio.close
- end
+ @tmpio.truncate(0)
end
end
--
EW
^ permalink raw reply related [flat|nested] 5+ messages in thread
* [PATCH 4/2] wbuf_lite: clear @busy flag when re-arming
2016-07-19 22:24 [PATCH] ANOTHER round of proxy_pass fixes Eric Wong
` (2 preceding siblings ...)
2016-07-20 22:40 ` [PATCH 3/2] wbuf_lite: reset sf_offset/sf_count consistently Eric Wong
@ 2016-07-21 0:57 ` Eric Wong
3 siblings, 0 replies; 5+ messages in thread
From: Eric Wong @ 2016-07-21 0:57 UTC (permalink / raw)
To: yahns-public
This allows us to speed up subsequent calls to wbuf_write when
the client socket buffers are cleared. This doesn't affect
functionality outside of performance.
---
lib/yahns/wbuf_lite.rb | 1 +
1 file changed, 1 insertion(+)
diff --git a/lib/yahns/wbuf_lite.rb b/lib/yahns/wbuf_lite.rb
index 7d77da9..46da501 100644
--- a/lib/yahns/wbuf_lite.rb
+++ b/lib/yahns/wbuf_lite.rb
@@ -90,6 +90,7 @@ def wbuf_close(client)
# the req_res socket itself could be completely drained of readable
# data and just waiting for another request (which we don't support, yet)
if @req_res
+ @busy = false
client.hijack_cleanup
Thread.current[:yahns_queue].queue_mod(@req_res, Yahns::Queue::QEV_WR)
return :ignore
--
EW
^ permalink raw reply related [flat|nested] 5+ messages in thread
end of thread, other threads:[~2016-07-21 0:57 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-07-19 22:24 [PATCH] ANOTHER round of proxy_pass fixes Eric Wong
2016-07-19 22:24 ` [PATCH 1/2] wbuf_lite: prevent clobbering responses Eric Wong
2016-07-19 22:24 ` [PATCH 2/2] wbuf_lite: unify EOF error handling Eric Wong
2016-07-20 22:40 ` [PATCH 3/2] wbuf_lite: reset sf_offset/sf_count consistently Eric Wong
2016-07-21 0:57 ` [PATCH 4/2] wbuf_lite: clear @busy flag when re-arming Eric Wong
Code repositories for project(s) associated with this public inbox
https://yhbt.net/yahns.git/
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).