Skip to content

Commit

Permalink
Fix a bug in uv_pipe_bind where the lock file was unlinked prematurely.
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
bacchusrx committed Apr 13, 2012
1 parent 1c066b0 commit bdd0b9b
Show file tree
Hide file tree
Showing 2 changed files with 23 additions and 8 deletions.
22 changes: 14 additions & 8 deletions src/unix/pipe.c
Expand Up @@ -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;
Expand Down Expand Up @@ -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;
}
Expand Down
9 changes: 9 additions & 0 deletions test/test-pipe-bind-error.c
Expand Up @@ -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);
Expand Down

0 comments on commit bdd0b9b

Please sign in to comment.