wandenberg / nginx-push-stream-module

A pure stream http push technology for your Nginx setup. Comet made easy and really scalable.
Other
2.22k stars 295 forks source link

Any subsequent request hangs after DELETE channel in a keep-alive connection #116

Closed 5lava closed 10 years ago

5lava commented 10 years ago

After I perform a DELETE method to delete a channel (existing or non-existing) within a persistent (keep-alive) connection, any subsequent request hangs (whether it goes to a location where push-stream-module responsible or not). This is a bug of 0.4.0 only, 0.3.5 works fine. Tried on nginx 1.4.4, 1.5.7. How to reproduce:

nginx.conf:

push_stream_shared_memory_size 10M;
location /pub {
    push_stream_publisher admin;
    push_stream_channels_path $arg_id;
}

Do telnet localhost 80, enter following (including trailing empty line):

DELETE /pub?id=1 HTTP/1.1
Host: localhost

You will get a response. Then put following in the same telnet connection (including trailing empty line):

GET / HTTP/1.1
Host: localhost

No response, the connection hangs.

I tried exactly the same on 0.3.5, everything works fine. nginx.conf for 0.3.5:

location /pub {
    push_stream_publisher admin;
    set $push_stream_channel_id $arg_id;
    push_stream_keepalive on;
}

P.S. I suppose it can be somehow related to #115.

5lava commented 10 years ago

Ok, I discovered that any request that ends with ngx_http_push_stream_send_only_header_response() blocks the connection. And that's the reason of #115 too.

5lava commented 10 years ago

That happens because of lack of ngx_http_finalize_request(). And yes, that's also the reason of #115. Furthermore, not only DELETE, but at least one more case is affected, when POST/PUT ends with ngx_http_push_stream_send_only_header_response() and doesn't return a body. Following patch fixes both cases.

diff --git a/src/ngx_http_push_stream_module_publisher.c b/src/ngx_http_push_stream_module_publisher.c
index 6b13466..bd1f6d7 100644
--- a/src/ngx_http_push_stream_module_publisher.c
+++ b/src/ngx_http_push_stream_module_publisher.c
@@ -224,6 +224,8 @@ ngx_http_push_stream_publisher_delete_handler(ngx_http_request_t *r)
     } else {
         ngx_http_push_stream_send_only_header_response(r, NGX_HTTP_OK, &NGX_HTTP_PUSH_STREAM_CHANNEL_DELETED);
     }
+
+    ngx_http_finalize_request(r, NGX_OK);
 }

 static void
@@ -242,6 +244,7 @@ ngx_http_push_stream_publisher_body_handler(ngx_http_request_t *r)
     if (r->headers_in.content_length_n <= 0) {
         ngx_log_error(NGX_LOG_ERR, r->connection->log, 0, "push stream module: Post request was sent with no message");
         ngx_http_push_stream_send_only_header_response(r, NGX_HTTP_BAD_REQUEST, &NGX_HTTP_PUSH_STREAM_EMPTY_POST_REQUEST_MESSAGE);
+        ngx_http_finalize_request(r, NGX_OK);
         return;
     }

P.S. I don't create a pull request because I'm not sure if I've covered all cases (I'm neither nginx guru nor advanced C developer)

wandenberg commented 10 years ago

Fixed on branch 0.4.1-dev at 582bb73fb9ad8c7a4214a42949cb333427d9bdf6