udev: prevent interruption by thread cancellation

[wuguanghao3]

Using mutexes only guarantees no concurrency during normal runtime. However, if a thread is cancelled mid-execution, a libudev API call might be interrupted halfway (partially executed), leaving memory values in an inconsistent or abnormal state.

Furthermore, this change aligns with the requirement that libudev interfaces are not thread-safe and must not be called from multiple threads.

fixed #152

[mwilck]

Good catch! I would prefer to move the implementation into mt-libudev.c, though.

[mwilck]

We don't do direct merges into master, except for new releases. Please create a PR against the "queue" branch in github.com/openSUSE/multipath-tools, which is our staging area.

[mwilck]

Can we move this into mt-libudev.c and use it as wrapper for the actual libudev calls?

[wuguanghao3]

Like this?

#define MT_NO_CANCEL(call) ({						\
		int __oldstate;						\
		pthread_setcancelstate(PTHREAD_CANCEL_DISABLE, &__oldstate); \
		__typeof__(call) __ret = (call);				\
		pthread_setcancelstate(__oldstate, NULL);		\
		pthread_testcancel();					\
		__ret;							\
	})

struct udev *mt_udev_new(void)
{
	struct udev *ret;

	pthread_mutex_lock(&libudev_mutex);
	pthread_cleanup_push(cleanup_mutex, &libudev_mutex);

	ret = MT_NO_CANCEL((udev_new)());

	pthread_cleanup_pop(1);
	return ret;
}

[mwilck]

Yes. Actually, in order to get rid of repetitive code, I'd go one step further than that, like this:

#define LU_WRAP_1(rtype, func, type1)					\
	rtype mt_##func(type1 __x) {					\
		rtype __r;						\
		pthread_mutex_lock(&libudev_mutex);			\
		pthread_cleanup_push(cleanup_mutex, &libudev_mutex);	\
		__r = MT_NO_CANCEL(func(__x));				\
		pthread_cleanup_pop(1);					\
		return __r;						\
	}

LU_WRAP_1(struct udev *, udev_ref, struct udev *);

But we can do this on top of your patch later.

This requires separate macros for different number of macro arguments (LU_WRAP_2 etc). Let me know if you can think of something more elegant.

[wuguanghao3]

Got it. I'll make the changes and submit a PR to github.com/openSUSE/multipath-tools

[mwilck]

Actually, this was quite straightforward to implement. I've pushed a commit to my tip branch giving you credit, see here.

Let me know what you think.

[wuguanghao3]

openSUSE#24

[mwilck]

Thanks, see there!

[wuguanghao3]

We don't do direct merges into master, except for new releases. Please create a PR against the "queue" branch in github.com/openSUSE/multipath-tools, which is our staging area.

OK, I will submit a PR against the queue branch in the next version