Re: [Libguestfs] [nbdkit PATCH 2/2] nbd: Reorder cleanup to avoid getting stuck in poll()

We have been seeing sporadic hangs on test-nbd-tls-psk.sh, where even though the client to the 'nbdkit nbd' process has cleanly exited, things are stalled in .close where nbd is trying to pthread_join() the reader thread, while the reader thread is itself blocked on a poll() that will never make additional progress. Tracing the race is difficult: nbd_shutdown() sends NBD_CMD_DISC to the server, and the NBD protocol does not require the server to send a response but does not forbid the server from using gnutls_bye to at least gracefully end the TLS session. So where a plaintext server just closes the socket (and the resulting EOF exits our poll()), it appears that with TLS, we can sometimes hit the situation where the server is waiting for a response to gnutls_bye() (note that qemu does not use this function, but nbdkit does), while at the same time our client is waiting to see EOF on the socket, resulting in deadlock. In the past (see commit 430f8141), we added shutdown(SHUT_WR) for plaintext clients to prod the server into closing the socket faster, but libnbd does not yet have that counterpart action. But I can at least reuse the same mechanism we have for waking up the poll() loop when first sending a command to the server. That is, since we already have a pipe-to-self in addition to reading from the server, it's trivial to argue that closing the pipe-to-self will guarantee that the reader thread sees something interesting to break out of its poll() loop, regardless of whether it also sees something interesting from the server after having sent NBD_CMD_DISC, and regardless of whether I need to add in more gnutls_bye() calls to either nbdkit or libnbd. Fixes: ab7760fc Signed-off-by: Eric Blake <eblake(a)redhat.com&gt; --- May be incomplete: I might also need to break out of the reader loop when read() returns 0. plugins/nbd/nbd.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/plugins/nbd/nbd.c b/plugins/nbd/nbd.c index e5b8f338..23a7da06 100644 --- a/plugins/nbd/nbd.c +++ b/plugins/nbd/nbd.c @@ -537,10 +537,10 @@ nbdplug_close_handle (struct handle *h) { if (nbd_shutdown (h->nbd, 0) == -1) nbdkit_debug ("failed to clean up handle: %s", nbd_get_error ()); + close (h->fds[1]); if ((errno = pthread_join (h->reader, NULL))) nbdkit_debug ("failed to join reader thread: %m"); close (h->fds[0]); - close (h->fds[1]); nbd_close (h->nbd); free (h); }