Support thread-safe reading of the subordinate ID database

[blm768]

For an project I've been working on, I've found it would be handy if the read-only functions public functions in libsubid were thread-safe to call. I can protect my own calls to the library with a mutex, but that doesn't protect against issues if my code gets included as part of another application that calls into libsubid without using that mutex. I suppose that case is a bit contrived, so my mutex workaround seems workable for now, but I'd prefer for this to eventually be bulletproof.

Factors I've found so far that could affect thread safety:

• subordinate_uid_db and subordinate_gid_db (subordinateio.cpp) keep a fair amount of mutable state with no thread synchronization.

  Just making them thread-local might be enough to solve that issue, although that would technically be a breaking change. I'm not seeing any thread-locals elsewhere in the source, so I'm not sure if there would be compatibility issues with them on some platform that the project actively supports. It looks like the locking logic might also need some revamping if these were thread-local since it seems to be written with the assumption that there will be one lock per process.

  Putting a mutex around them might also be an option as long as all supported platforms have support for C11 mutexes. Alternatively, if the read-only functions used their own instances of commonio_db, we could skip some of the locking concerns.

• The static lock_count and nscd_need_reload variables in commonio.c aren't thread-safe.

• The various calls to getpwnam, etc. would have to be switched to use getpwnam_r.

If there's general interest in making the library thread-safe where feasible, I can dedicate some time to reading through the code more thoroughly and seeing if I can get some reasonable patches together.

[alejandro-colomar]

For an project I've been working on, I've found it would be handy if the read-only functions public functions in libsubid were thread-safe to call. I can protect my own calls to the library with a mutex, but that doesn't protect against issues if my code gets included as part of another application that calls into libsubid without using that mutex. I suppose that case is a bit contrived, so my mutex workaround seems workable for now, but I'd prefer for this to eventually be bulletproof.

Factors I've found so far that could affect thread safety:

* `subordinate_uid_db` and `subordinate_gid_db` (`subordinateio.cpp`) keep a fair amount of mutable state with no thread synchronization.
  Just making them thread-local might be enough to solve that issue, although that would _technically_ be a breaking change. I'm not seeing any thread-locals elsewhere in the source, so I'm not sure if there would be compatibility issues with them on some platform that the project actively supports. It looks like the locking logic might also need some revamping if these were thread-local since it seems to be written with the assumption that there will be one lock per process.
  Putting a mutex around them might also be an option as long as all supported platforms have support for C11 mutexes. Alternatively, if the read-only functions used their own instances of `commonio_db`, we could skip some of the locking concerns.

* The static `lock_count` and `nscd_need_reload` variables in `commonio.c` aren't thread-safe.

* The various calls to `getpwnam`, etc. would have to be switched to use `getpwnam_r`.

Hmm, I'd start with the latter two. Those are beneficial just on their own. I'd like to get rid of static variables for many reasons.

If there's general interest in making the library thread-safe where feasible, I can dedicate some time to reading through the code more thoroughly and seeing if I can get some reasonable patches together.

Sure.

If you start writing patches, it would be good to base them on this PR:

• Move shadow APIs to lib/shadow/ #1197

Since I've done quite some refactor of the APIs you'll be touching, so if you base on that, you'll have less rebasing work when we merge those patches.

[alejandro-colomar]

Actually, better base on this one:

• ls #1272

[alejandro-colomar]

Or better yet, wait some months (if you want). I've found some more large improvements I'd like to do that go in the same direction, and don't want to add conflicts with your patches. I think I can get rid of a lot of our code, and make another part thread-safe, or easily convertibey to being thread-safe.

[blm768]

I'm not in a particular rush, so I'd be fine with circling back to this later. In case it's useful, though, here's a preliminary patch series for better thread safety in commonio's thread_count.

[alejandro-colomar]

I'm not in a particular rush, so I'd be fine with circling back to this later. In case it's useful, though, here's a preliminary patch series for better thread safety in commonio's thread_count.

Uhhh, I hoped atomics and locks would come the last, after fixing the low-hanging fruits (implementing the _r variants, which get rid of a lot of static variables). :)

[blm768]

In this particular instance, I don't think we'll be able to easily avoid the locking and still get the necessary semantics, although if the lock count were broken out into some kind of "edit session" structure that's explicitly opened or closed before locking individual databases, that might ultimately be cleaner than just slapping a lock on some shared state.