On Wed, Sep 23, 2020 at 10:16:48AM +0200, Martin Wilck wrote: > 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? Yeah. After I sent off my message, I realized that there's no way to know if you dropped the last reference, which means that NULLing out udev doesn't make sense. -Ben > Martin > -- dm-devel mailing list dm-devel@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/dm-devel
