[PATCH] Nullify pc->connection in case of failure

Piotr Sikora piotr.sikora at frickle.com
Sun Jan 22 09:40:34 UTC 2012


Hi Maxim,

> I tend to think this patch isn't enough, as it might not del
> events already added as a result.

You're correct, as usual.

I back-ported this from the module I'm working on (with single 
ngx_add_event() call) and apparently I didn't pay enough attention.

> Probably better solution would be to just return NGX_ERROR if
> we've already set pc->connection, and let real
> ngx_close_connection() to do the work.

Yes, this would indeed work better, but to be honest, I don't see much point 
in deferring ngx_close_connection() call. Personally, I prefer when failing 
function cleans-up after itself on failure if possible, so I would propose 
this patch instead:

--- src/event/ngx_event_connect.c.orig  Wed Nov 25 18:03:59 2009
+++ src/event/ngx_event_connect.c       Sun Jan 22 09:22:31 2012
@@ -159,6 +159,9 @@ ngx_event_connect_peer(ngx_peer_connection_t *pc)
             ngx_log_error(level, c->log, err, "connect() to %V failed",
                           pc->name);

+            ngx_close_connection(c);
+            pc->connection = NULL;
+
             return NGX_DECLINED;
         }
     }
@@ -240,12 +243,8 @@ ngx_event_connect_peer(ngx_peer_connection_t *pc)

failed:

-    ngx_free_connection(c);
-
-    if (ngx_close_socket(s) == -1) {
-        ngx_log_error(NGX_LOG_ALERT, pc->log, ngx_socket_errno,
-                      ngx_close_socket_n " failed");
-    }
+    ngx_close_connection(c);
+    pc->connection = NULL;

     return NGX_ERROR;
}

Best regards,
Piotr Sikora < piotr.sikora at frickle.com >



More information about the nginx-devel mailing list