search for: 430f8141

...been satisfied I'm also fairly confident that I'll need to patch libnbd to call shutdown(SHUT_WR) on plaintext and gnutls_bye(GNUTLS_SHUT_WR) on TLS connections after sending NBD_CMD_DISC, as that is something we lowt when the nbd plugin switched to libnbd, even after documenting in commit 430f8141 why it is important for preventing shutdown deadlocks. Eric Blake (2): nbd: Don't reference stale errno in reader loop nbd: Reorder cleanup to avoid getting stuck in poll() plugins/nbd/nbd.c | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) -- 2.26.0.rc2

...closing the connection anyway); but if another connection happens to start in that window, we could end up read()ing from the fd opened by the new connection, with disastrous results on the second connection. [1] ./configure CXFLAGS=-fsanitize=thread LDFLAGS=-fsanitize=thread Commits c70616f8 and 430f8141 tried to clean up deadlock during shutdown, but missed that without some sort of locking, a close-before-read was still possible. Swap lines so that pthread_join() now serves as the locking to ensure close is not attempted while another thread may be about to use the fd. Thanks: Richard W.M. Jones...

.... And that's where the shutdown(2) API comes in handy: it forces the kernel to realize that one or both directions of traffic are no longer needed, making this asynchronous handling in close() much faster. At least nbdkit is known to react favorably to a shutdown() request - see nbdkit commit 430f8141. We may want to add h->sock->ops->shutdown() (especially if we want to handle SHUT_WR through adding more states for more graceful shutdown paths, including when TLS is involved), but even in the short-term, something as simple as adding shutdown(h->fd, SHUT_RDWR) immediately before th...

...as shutdown(SHUT_WR) on plaintext, or gnutls_bye() with TLS), but waits for us to close the socket first, we end up with two processes deadlocked on each other. (nbdkit as server has used gnutls_bye since commit bd9a52e0; qemu however does not use it.) Other commits such as 14ba4154, c70616f8, and 430f8141 show that getting the shutdown sequence race-free has not been trivial. Fixes: ab7760fc Signed-off-by: Eric Blake <eblake@redhat.com> --- 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 ba1e7188..0ea2a4a2 100...

...t 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...

This is a simple patch which stops nbd_close from waiting too long for a server subprocess to shut down. I wanted to send SIGHUP because the server will be able to catch it and do a clean shutdown if that is required. Is another signal better? Is it right to send a signal here? Rich.

...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...