[dm-devel] [PATCH 17/19] libmultipath: add udev and logsink symbols

[Martin Wilck]

On Mon, 2020-09-21 at 15:10 -0500, Benjamin Marzinski wrote:
> 
> After calling libmultipath_exit(), you can never reinitialized the
> udev
> device.  That seems fine, but it should probably set udev to null, so
> that future calls to libmultipath_init() don't return success. Either
> that or multipath_init() should use a mutex instead of pthread_once()
> to
> avoid races, so that you can reinitialize udev after a call to
> libmultipath_exit().

I've been thinking about this some more. It makes a lot of sense to
move more cleanup code into libmultipath_exit() in the future; thus
this function will do more than just clean up "udev". I believe calling
libmultipath_exit() will become the only supported way to clean up
libmultipath, and basically mandatory, rather sooner than later. 

The handling of "udev" is the main cause of headache, because we don't
know whether the application wants to continue to use the variable
after libmultipath_exit(). In libmultipath_exit(), we can't determine
if "udev" is the symbol from libmultipath or from some other object
file. It's also impossible to tell by the return value of udev_unref()
whether or not it destroyed the variable. Setting udev to NULL is
dangerous if the uevent listener thread is still running, or if the
application needs to use the variable further.

So this is my current idea for a "robust" design:

1. libmultipath_init() initializes udev if it's NULL; otherwise, it
   simply takes an additional ref. IOW, applications may (but
   don't have to) initialize and use udev before calling
   libmultipath_init().
2. libmultipath_exit() calls udev_unref(udev), without nullifying it.
   Thus applications may continue using udev afterwards, if they own an
   additional reference.
3. libmultipath_init() always fails after calling libmultipath_exit().
4. Other libmultipath calls after libmultipath_exit() may cause
   undefined behavior.
5. Normal usage would be to call libmultipath_exit() at program exit.
6. Calling libmultipath_init() is currently only mandatory for
   programs that don't initialize "udev" themselves. This may change
   in the future.
7. Calling libmultipath_exit() will be mandatory for programs that
   wish to avoid memory leaks.

The only downside that I see is that the application can't test whether
"udev" is valid by checking if it's NULL. But that's by design of
udev_unref(). The application needs to track its own references.

There is no rigorous reason for (3.). In principle, we could just
handle re-initialization like (1.). But I don't think it's worth the
effort of figuring out all possible ways in which re-initialization
could go wrong, in particular if we want to initialize more stuff
later.

Does this make sense? 
Martin