From bdd0b9b0b17bce312ec1563744751ff92bdd9204 Mon Sep 17 00:00:00 2001
From: bacchusrx <bacchusrx@eightstar.ca>
Date: Mon, 19 Dec 2011 20:52:51 -0500
Subject: [PATCH] Fix a bug in uv_pipe_bind where the lock file was unlinked
 prematurely.

Added a test to test-pipe-bind-error.c that covers the issue. Basically, since
uv_flock_destroy gets called whenever uv_pipe_bind fails, only the first
attempt to bind an already bound pipe would fail. Subsequent attempts would
succeed.

For now I've commented it out, but I think some cleanup still needs to happen
(just not the unlinking when another process already holds the lock).

I also moved some code out of uv_pipe_bind that was supposed to be in
uv_pipe_cleanup.
---
 src/unix/pipe.c             | 22 ++++++++++++++--------
 test/test-pipe-bind-error.c |  9 +++++++++
 2 files changed, 23 insertions(+), 8 deletions(-)

diff --git a/src/unix/pipe.c b/src/unix/pipe.c
index 16c5f3ac14..dc2cec4997 100644
--- a/src/unix/pipe.c
+++ b/src/unix/pipe.c
@@ -163,16 +163,17 @@ int uv_pipe_bind(uv_pipe_t* handle, const char* name) {
     }
     uv__close(sockfd);
 
-    if (pipe_flock) {
-      uv_flock_destroy(pipe_flock);
-    }
+    /* If you clear the lock here, then only the first attempt to bind a locked
+     * pipe fails. Presumably, some kind of cleanup still belongs here. */
+    
+    /*
+     * if (pipe_flock) {
+     *   uv_flock_destroy(pipe_flock);
+     * }
+     *
+     */
 
     free((void*)pipe_fname);
-
-    if (handle->pipe_flock) {
-      uv_flock_destroy((uv_flock_t*)handle->pipe_flock);
-      free(handle->pipe_flock);
-    }
   }
 
   errno = saved_errno;
@@ -228,6 +229,11 @@ int uv_pipe_cleanup(uv_pipe_t* handle) {
     free((void*)handle->pipe_fname);
   }
 
+  if (handle->pipe_flock) {
+    uv_flock_destroy((uv_flock_t*)handle->pipe_flock);
+    free(handle->pipe_flock);
+  }
+
   errno = saved_errno;
   return status;
 }
diff --git a/test/test-pipe-bind-error.c b/test/test-pipe-bind-error.c
index b84d20f1ea..3cb88f8c9d 100644
--- a/test/test-pipe-bind-error.c
+++ b/test/test-pipe-bind-error.c
@@ -57,6 +57,15 @@ TEST_IMPL(pipe_bind_error_addrinuse) {
 
   ASSERT(uv_last_error(uv_default_loop()).code == UV_EADDRINUSE);
 
+  /* This covers a bug in the original implementation whereby a only the first
+   * attempt to bind an already bound pipe would fail. Subsequent attempts
+   * would succeed because the lock file was cleared after the bind error. */
+
+  r = uv_pipe_bind(&server2, TEST_PIPENAME);
+  ASSERT(r == -1);
+
+  ASSERT(uv_last_error(uv_default_loop()).code == UV_EADDRINUSE);
+
   r = uv_listen((uv_stream_t*)&server1, SOMAXCONN, NULL);
   ASSERT(r == 0);
   r = uv_listen((uv_stream_t*)&server2, SOMAXCONN, NULL);