Hello, Thanks for your comments. How about second version:
# HG changeset patch # User ykirpic...@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 <mdou...@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@nginx.org > http://mailman.nginx.org/mailman/listinfo/nginx-devel >
_______________________________________________ nginx-devel mailing list nginx-devel@nginx.org http://mailman.nginx.org/mailman/listinfo/nginx-devel