Create a new UeventSender.sender object for every uevent call

[bobhenz-jabil]

The UeventSender.sender class uses libudev under the covers. libudev's documentation (referenced below) states:

Only a single specific thread may operate on a given object during its
entire lifetime. It's safe to allocate multiple independent objects
and use each from a specific thread in parallel. However, it's not
safe to allocate such an object in one thread, and operate or free it
from any other, even if locking is used to ensure these threads don't
operate on it at the very same time.

Therefore, if a test is multi-threaded and one thread calls to add mock devices the object might be created on that thread, then later another thread might remove the mock device and attempt to use the same libudev object.

For example, this can very easily happen in a ROS python-based environment where ROS spawns a new thread for each callback.

While I don't know what the failure might look like (as I was seeing a different crash at the time I stumbled upon this), the libudev documentation is rather concerning given how this was being used.

Reference: https://www.freedesktop.org/software/systemd/man/latest/libudev.html

[benzea]

Hmm, shouldn't the caller either push all requests through the same thread or do sufficient synchronisations using locks?

Otherwise we would begin to define umockdev to be threadsafe at least for some operations. I am just wondering whether or not that is desirable.

[bobhenz-jabil]

@benzea: Pushing all requests through the same thread would be difficult in ROS (specifically rospy which is what I'm using this in) since for every callback ROS spins up a new thread to run the callback. Therefore, I would have to have my code place the event on a queue and then have a thread setup only to make calls to umockdev.

Locking would not solve this as that only prevents umockdev from being accessed simultaneously by two separate threads not two separate threads accessing the umockdev structures within a mutex. (I already have a mutex preventing simultaneous access to umockdev and don't mind that restriction at all.)

I'm not asking umockdev to be multi-thread safe in general, but in this case, in my opinion, it is quite a burden to place on the client (and a surprising restriction) to require that all calls to umockdev be done from a single thread.

Also, such a restriction, if intentional, would need to be well-documented.

[martinpitt]

Hmm 🤔 My initial reaction was "let's make an UEventSender instance hash table per thread id". However, uevent_sender_open() is relatively cheap (allocation and sprintf), and udev_new() also doesn't do much, so performance wise it may even be more effort to do the hashtable. And usually tests don't do millions of uevents.

So I'm personally fine with this if it helps @bobhenz-jabil -- there is no written guarantee for umockdev to be threadsafe in all cases, and indeed I never bothered with this (such as avoiding globals). @benzea do you strongly object to this?

[martinpitt]

(sorry for late answer -- I'm back from vacation now)

[benzea]

Nah, fine with me. I just mostly feared it would set a precedence for thread safety in general.

[bobhenz-jabil]

Thank you!