Re: --enable-thread-safety broken + patch regressions - Mailing list pgsql-hackers

Bruce Momjian writes:> Lee Kindness wrote:> > Bruce, the changes you made yesterday to configure for> >
--enable-thread-safetyhave broken the build, at least for Linux on> > Redhat 9.> OK, how did I break things?  Can you
showme the failure.
 

After a:
 ./configure --prefix=/var/lib/pgsql/74b --enable-thread-safety

a compile of port/threads.c fails with:
gcc -O2 -Wall -Wmissing-prototypes -Wmissing-declarations -I../../src/include -c -o threads.o threads.cthreads.c: In
function`pqGetpwuid':threads.c:49: too few arguments to function `getpwuid_r'threads.c:49: warning: assignment makes
pointerfrom integer without a castthreads.c: In function `pqGethostbyname':threads.c:74: warning: passing arg 5 of
`gethostbyname_r'from incompatible pointer typethreads.c:74: too few arguments to function
`gethostbyname_r'threads.c:74:warning: assignment makes pointer from integer without a cast
 

And this is what brought me to the issue below... The POSIX version
are getting picked up but handled like broken versions...

What info would help here? config.log?
> > Also, I took the opportunity to look at port/threads.c. It is missing> > important functionality compaired to the
patchI originally> > submitted. For getpwuid_r, gethostbyname_r and strerror_r there are> > three possible scenarios:>
>> > 1. The OS doesn't have it (but the non _r function can still be thread> > safe (i.e. HPUX 11)).> > > > 2. The OS
hasit, but the implmentation doesn't match the POSIX spec.> > > > 3. The OS has it, and the implmentation matches the
POSIXspec.> > > > Case 3 is not being considered. In my original patch this was handled> > by the pqGetpwuid etc
functionssimply being defined to getpwuid_r> > (except for pqStrerror).> > I believe what we did was that there was no
wayto test for #3 (at the> time), so we just went with the normal function and the POSIX one, and> were going to see
whathappened to see if anyone needed the non-POSIX> one.  Do we have any platforms that need it?
 

Well the code in thread.c will only work if the _r function is the
broken non-POSIX version.
> > I remember discussing with you that the implementation of pqStrerror> > didn't really need the distinction between
thetwo _r> > versions. However I think the others do, and the native/correct _r> > calls should be #defined in if they
matchthe POSIX spec.> > > > It's also worth considering that when the _r function is available AND> > the normal
functionis also thread-safe then the _r version should> > still be used since it has a clean API which removes unneeded
locking>> within the old function.> > We have that already. Have you looked in the template files.  There you> control
whetheryou should use _r functions.> > Also, I doubt that the locking really has any performance hit to> it.
 

As do I, but people are using this as an argument for the dumb libpq_r
library idea!
> > I've still got the latest (and earlier with some configure work)> > patches I submitted up at:> I just looked at
this--- I have not seem them before.
 

Everything on that page has been posted/linked to hackers and patches.
> Seems theading requires four things, potentially:> >     compile flags>     link flags>     link libraries>
specialfunctions> > While your configure checks can detect the existance of the last one,> they don't tell us what to
doif they don't exist --- are the normal> ones thread-safe.> > So, the big question is whether we gain by having
detectionof non-posix> functions or whether it is better to just have template control it.
 

We want to define & implement wrapper functions with the same API as
the POSIX versions of the _r functions we need. If we have the POSIX
versions then the replacement simply needs to be a #define to
it. Otherwise a stub function is implemented to wrap around either the
broken/old _r function or the legacy function (which may be thread
safe).

It's getting to the stage I think this isn't going to be done
correctly in time for 7.4...

L.