[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

[lvm-devel] [PATCH] Fix a double close() in clvmd (fixing another deadlock bug)



Hi,

while working on 561226, I ran into another deadlock in clvmd. Again, it
may or may not be the same as reported in the aforementioned bug.

Nevertheless, this is what was going on: the management threads
(main_loop, the socket thread) could close a single fd twice in a row
sometimes. This wouldn't be such a tragedy in a single-threaded
application, the latter close would normally just fail (although there
is still plenty of scenarios where even in a single-threaded program,
this code could lead to a spectacular failure).

Anyway, at least one other thread can be running at the same time as the
threads doing the double close. That one running thread also happens to
do some IO (namely, open /proc/devices, read from it, close it). If
there was enough "demand" on the socket, this could happen:

- a connection to clvmd is about to finish, let's say the fd is 13 (it
  often happens to be in my test script, don't ask why)
- the local_sock thread calls close(13)
- the lvm thread calls open("/proc/devices"...) and gets 13
- the main_loop thread calls close(13) [OOPS!]
- new connection arrives, and is accept'd by a (new) local_sock thread
- the accept gives an fd of 13 (since it's the lowest free fd at this point)
- the lvm thread gets around to read from it's /proc/devices
  handle... 13, again
- the lvm thread hangs forever trying to read from the socket instead
  from /proc/devices

The attached patch should fix the problem.

diff -rN -u -p old-clvmd-hang/daemons/clvmd/clvmd.c new-clvmd-hang/daemons/clvmd/clvmd.c
--- old-clvmd-hang/daemons/clvmd/clvmd.c	2010-10-27 01:30:29.000000000 +0200
+++ new-clvmd-hang/daemons/clvmd/clvmd.c	2010-10-27 01:30:29.000000000 +0200
@@ -838,7 +838,10 @@ static void main_loop(int local_sock, in
 						lastfd->next = thisfd->next;
 						free_fd = thisfd;
 						thisfd = lastfd;
-						close(free_fd->fd);
+						if (free_fd->fd >= 0) {
+							close(free_fd->fd);
+							free_fd->fd = -1;
+						}
 
 						/* Queue cleanup, this also frees the client struct */
 						add_to_lvmqueue(free_fd, NULL, 0, NULL);
@@ -1088,7 +1091,10 @@ static int read_from_local_sock(struct l
 			thisfd->bits.localsock.pipe_client->bits.pipe.client =
 			    NULL;
 
-		close(thisfd->fd);
+		if (thisfd->fd >= 0) {
+			close(thisfd->fd);
+			thisfd->fd = -1;
+		}
 		return 0;
 	} else {
 		int comms_pipe[2];
Yours,
   Petr.

[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]