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 -- dm-devel mailing list dm-devel@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/dm-devel
