tatsuhiro-t / spdylay

The experimental SPDY protocol version 2, 3 and 3.1 implementation in C
http://tatsuhiro-t.github.io/spdylay/
MIT License
604 stars 102 forks source link

Shrpx often stuck if the client disconnect/stop receiving in the middle of the transfer #107

Closed fcicq closed 10 years ago

fcicq commented 10 years ago

I use shrpx to transform a remote spdy proxy to a local http one.

reproduce: open a youtube video, when it is buffering, switch to a different play position (to stop the current request and start another one). shrpx will no longer send requests to upstream & it still accept downstream request.

expected result: shrpx send RST_STREAM to upstream, and normally process requests.

fcicq commented 10 years ago

use curl --proxy and press ^C quickly can also reproduce.

fcicq commented 10 years ago
(first curl --proxy, Ctrl+C pressed)
[INFO] [CLIENT_HANDLER:0x9dd2d20] Network error: Broken pipe
       (shrpx_client_handler.cc:89)
[INFO] [CLIENT_HANDLER:0x9dd2d20] Deleting
       (shrpx_client_handler.cc:188)
[INFO] [DOWNSTREAM:0x9dd3068] Deleting
       (shrpx_downstream.cc:67)
[INFO] [DCONN:0x9dd3248] Deleting
       (shrpx_spdy_downstream_connection.cc:59)
[INFO] [DCONN:0x9dd3248] Submit RST_STREAM for DOWNSTREAM:0x9dd3068, stream_id=1
       (shrpx_spdy_downstream_connection.cc:140)
[INFO] [DCONN:0x9dd3248] Deleted
       (shrpx_spdy_downstream_connection.cc:76)
[INFO] [DOWNSTREAM:0x9dd3068] Deleted
       (shrpx_downstream.cc:77)
[INFO] [CLIENT_HANDLER:0x9dd2d20] Deleted
       (shrpx_client_handler.cc:213)
[INFO] [DSPDY:0x9dd2588] Stream stream_id=1 is being closed
       (shrpx_spdy_session.cc:683)

(second curl --proxy)
[INFO] [LISTEN:0x9dd1570] Accepted connection. fd=9
       (shrpx_listen_handler.cc:97)
[INFO] [UPSTREAM:0x9dd3190] HTTP request started
       (shrpx_https_upstream.cc:78)
[INFO] [UPSTREAM:0x9dd3190] HTTP request headers completed
       (shrpx_https_upstream.cc:135)
[INFO] [UPSTREAM:0x9dd3190] HTTP request headers
       (shrpx_https_upstream.cc:156)
(omitted)
[INFO] [CLIENT_HANDLER:0x9dedd28] Downstream connection pool is empty. Create new one  
       (shrpx_client_handler.cc:320)
[INFO] [DCONN:0x9dee630] Attaching to DOWNSTREAM:0x9dd3068
       (shrpx_spdy_downstream_connection.cc:101)
[INFO] [DCONN:0x9dee630] HTTP request headers
(omitted)
       (shrpx_spdy_downstream_connection.cc:360)
[INFO] [UPSTREAM:0x9dd3190] HTTP request completed
       (shrpx_https_upstream.cc:227)
(and waiting ...)
fcicq commented 10 years ago

slightly better with this, but I dont think this is the final fix.

diff --git a/src/shrpx_spdy_downstream_connection.cc b/src/shrpx_spdy_downstream_connection.cc
index 92c36c8..306c5e6 100644
--- a/src/shrpx_spdy_downstream_connection.cc
+++ b/src/shrpx_spdy_downstream_connection.cc
@@ -60,6 +60,7 @@ SpdyDownstreamConnection::~SpdyDownstreamConnection()
   }
   if(request_body_buf_) {
     evbuffer_free(request_body_buf_);
+    request_body_buf_ = 0;
   }
   if(downstream_) {
     if(submit_rst_stream(downstream_) == 0) {
tatsuhiro-t commented 10 years ago

I did pause and seek in youtube and curl + ctrl-C, but have not reproduced the issue so far.

I don't think the patch does something meaningful because it is a destructor and after evbuffer_free(), request_bodybuf is not used (if used, it leads to crash).

fcicq commented 10 years ago

the first request always works fine. but one abnormal interrupted request will stop the spdy processing :(interrupt just after header is sent may help to reproduce...I did pause and seek in youtube and curl + ctrl-C, but have not reproduced the issue so far.I don't think the patch does something meaningful because it is a destructor and after evbuffer_free(), request_bodybuf is not used (if used, it leads to crash).—Reply to this email directly or view it on GitHub.

fcicq commented 10 years ago

hmm... seems remote have indeed received the request, so another issue should be opened.