[PATCH] SPDY: fix nopush cleanup

[Yury Kirpichev]

Hello,

Thanks for your comments.
How about second version:

# HG changeset patch
# User ykirpichev at gmail.com
# Date 1370964275 -14400
# Branch nopush_fix_2
# Node ID 14de55787a48327019d549d48abf2631e294b4d8
# Parent  725fb71ab1a60bd48b0afb8b001b5349f5054cb1
SPDY: fix nopush cleanup

diff -r 725fb71ab1a6 -r 14de55787a48 src/http/ngx_http_spdy.c
--- a/src/http/ngx_http_spdy.c  Fri Jun 07 13:16:00 2013 -0700
+++ b/src/http/ngx_http_spdy.c  Tue Jun 11 19:24:35 2013 +0400
@@ -504,6 +504,51 @@
     clcf = ngx_http_get_module_loc_conf(sc->http_connection->conf_ctx,
                                         ngx_http_core_module);

+    // all data is sent, can clean nopush if necessary
+    if (wev->ready)
+    {
+        int tcp_nodelay;
+
+        if (c->tcp_nopush == NGX_TCP_NOPUSH_SET) {
+            if (ngx_tcp_push(c->fd) == -1) {
+                ngx_connection_error(c, ngx_socket_errno, ngx_tcp_push_n "
failed");
+                return NGX_ERROR;
+            }
+
+            c->tcp_nopush = NGX_TCP_NOPUSH_UNSET;
+            tcp_nodelay = ngx_tcp_nodelay_and_tcp_nopush ? 1 : 0;
+
+        } else {
+            tcp_nodelay = 1;
+        }
+
+        if (tcp_nodelay
+            && clcf->tcp_nodelay
+            && c->tcp_nodelay == NGX_TCP_NODELAY_UNSET)
+        {
+            ngx_log_debug0(NGX_LOG_DEBUG_HTTP, c->log, 0, "tcp_nodelay");
+
+            if (setsockopt(c->fd, IPPROTO_TCP, TCP_NODELAY,
+                           (const void *) &tcp_nodelay, sizeof(int))
+                == -1)
+            {
+#if (NGX_SOLARIS)
+                /* Solaris returns EINVAL if a socket has been shut down */
+                c->log_error = NGX_ERROR_IGNORE_EINVAL;
+#endif
+
+                ngx_connection_error(c, ngx_socket_errno,
+                                     "setsockopt(TCP_NODELAY) failed");
+
+                c->log_error = NGX_ERROR_INFO;
+                return NGX_ERROR;
+            }
+
+            c->tcp_nodelay = NGX_TCP_NODELAY_SET;
+        }
+
+    }
+
     if (ngx_handle_write_event(wev, clcf->send_lowat) != NGX_OK) {
         return NGX_ERROR; /* FIXME */
     }


2013/6/11 Maxim Dounin <mdounin at mdounin.ru>

> Hello!
>
> On Tue, Jun 11, 2013 at 01:00:42PM +0400, Yury Kirpichev wrote:
>
> > Could you please take a look at patch below.
> > I've tried to fix problem that TCP_CORK option is not cleaned in SPDY
> case.
>
> The patch looks wrong.
>
> - It introduces layering violation and build failure --without-http
>   as a result.  You may have better luck focusing on a problem you
>   want to fix, and avoiding unrelated changes as much as possible.
>
> - It tries to restore nopush after each c->send_chain() call,
>   which looks suboptimal.  It probably should be done if there are no
>   pending data to send.
>
> --
> Maxim Dounin
> http://nginx.org/en/donation.html
>
> _______________________________________________
> nginx-devel mailing list
> nginx-devel at nginx.org
> http://mailman.nginx.org/mailman/listinfo/nginx-devel
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mailman.nginx.org/pipermail/nginx-devel/attachments/20130611/09c57a77/attachment-0001.html>