[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>
---
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);
}
--
2.26.0.rc2