[Libguestfs] [libnbd PATCH] docs: Mention that nbd_close is not thread-safe

[Martin Kletzander]

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
>



-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/libguestfs/attachments/20190725/d82bf362/attachment.sig>