On Thu, Jul 25, 2019 at 08:31:03AM -0500, Eric Blake wrote:
On 7/25/19 3:56 AM, Martin Kletzander wrote:
On Thu, Jul 25, 2019 at 09:41:01AM +0100, Richard W.M. Jones wrote:
On Wed, Jul 24, 2019 at 03:14:28PM -0500, Eric Blake wrote:
Closing the handle in one thread while another thread is locked causes undefined behavior (as closing does not obtain a lock, and can cause use-after-free or other nasty problems to the thread that does own the lock). However, it is not sensible to try and obtain a lock in nbd_close, as POSIX says that it is also undefined for any other thread to wait on a mutex that has already been destroyed. Therefore, we don't need to change our code, but merely remind users that nbd_close is not safe until all other uses of the handle have ceased. ---
Yes it's not safe to call nbd_close until all other uses of the same handle from any other thread are over.
Would it be too much of a trouble to add reference counting and give users a way to "copy" of the handle? It wouldn't be a copy, but merely an increment on the reference counter.
So each time you hand an nbd handle to another thread, that new thread has to increment the counter, and then every thread must call nbd_close when done with their copy (but only the last thread to do so actually frees resources). It might help, but only if we are willing to go that route...
Or is it not worth doing that?
...and as Rich says, managing refcounting ourselves even with language bindings is harder than it looks (how do you guarantee the refcount is increased when another language has other ways to share the handle between threads?). The user can just as easily pthread_join before calling nbd_close, at which point the problem is equally solved, but with less burden on the library. I don't think we've locked ourselves in a corner - if there is a compelling reason to add thread-safe close via ref-counting later on, we can do it, and update the docs at that time, but for now I'm fine living with the doc patch.
Thanks for the explanation both of you, it makes perfect sense and I agree.
-- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3226 Virtualization: qemu.org | libvirt.org