nss client: make innetgr() thread safe #5541
Conversation
|
Hi, I have send two different implementations, the second is #5542. Each has its pros and cons. This one requires more memory since all data is read in a single run and kept in the client process between the calls. This is more or less the same as libnss_files.so does, the related data from /etc/netgroup is read by setnetgrent() and processed later. The second one uses a new connection for each request sequences. Since the socket has to be opened and the connection established this will require a bit more time. My main concern here is that the file descriptor might get lost and the connection stays open if the caller does not implement the setnetgrent(), getentgrent(), endgrent() sequence correctly or calls them in threads because the sequence itself is not thread-safe, only the implementation with innetgr(). bye, |
|
In general, I prefer this solution as more clear and simple. See #5542 (comment) |
|
Hi, thank you for the review. I set bye, |
|
Sorry I missed test code last time (I have a couple of minor comments in its regard.) |
|
Hi, the latest version fixes the comments for the test program as well and adds a GPLv3 header to the file. bye, |
|
Hi, a couple of other remarks. |
The innetgr() call is expected to be thread safe but SSSD's the current implementation isn't. In glibc innetgr() is implementend by calling the setnetgrent(), getnetgrent(), endgrent() sequence with a private context (struct __netgrent) with provides a member where NSS modules can store data between the calls. With this patch setnetgrent() will read all required data from the NSS responder and store it in the data member of the __netgrent struct. Upcoming getnetgrent() calls will only operate on the stored data and not connect to the NSS responder anymore. endgrent() will free the data. Since the netgroup data is read in a single request to the NSS responder protected by a mutex and stored in private context of innetgr() this call is now thread-safe. Resolves: SSSD#5540
This integration test adds 2 large netgroups in LDAP and runs a program with 2 threads looking up those netgroups in parallel. Resolves: SSSD#5540
|
I'm fine with this. Ack. |
The innetgr() call is expected to be thread safe but SSSD's the current
implementation isn't. In glibc innetgr() is implementend by calling the
setnetgrent(), getnetgrent(), endgrent() sequence with a private context
(struct __netgrent) with provides a member where NSS modules can store data
between the calls.
With this patch setnetgrent() will read all required data from the NSS
responder and store it in the data member of the __netgrent struct.
Upcoming getnetgrent() calls will only operate on the stored data and not
connect to the NSS responder anymore. endgrent() will free the data. Since
the netgroup data is read in a single request to the NSS responder
protected by a mutex and stored in private context of innetgr() this call
is now thread-safe.
Resolves: #5540