Stephen Smoogen writes:
I am trying to figure out the logic of this section:
```
static char * lastUname = NULL; // So lastUname is NULL
static uid_t lastUid;
if (!thisUname) {
lastUname = rfree(lastUname); // lastUname should still be NULL and
we are freeing NULL and setting itself back to NULL.
return -1;
```
I expect this is where I am not understanding something basic in C from too
many years in non-pointer land. I looked at the change of these lines and
they date back to this commit.
This is a fairly common kind of simple caching to avoid expensive
username/userid and groupname/groupid lookups by caching the last one. This
code is expecting that it will be called, repeatedly, to look up the same
user/group, so it caches the results of the last lookup, and returns it.
Most of the files in a binary rpm would typically have the same uid gid
owner, so these functions are expected to be called with the same values
each time.
Which now get cached in static variables. This worked great while everything
was single-threaded.
Now, if you have two execution threads going step by step right here, and
both of them passed in a null pointer, both of them will run this, and both
of them will `free` the same pointer. Fail.
This `static` usage pattern is inherently thread unsafe.
The entries for __thread I found come in around 2019. Did you have a
bugzilla
or a report
on <URL:https://github.com/rpm-software-
management/rpm/>https://github.com/rpm-software-management/rpm/ which I can
add anything I find?
and most date back from 2013. --
https://github.com/rpm-software-management/rpm/issues/2826