Re: [HACKERS] thread safety tests
Jan Wieck wrote:
I looked over the code and the only place getpwuid_r (through
pqGetpwuid) is used is in libpq to look up the default username based on
the euid for the connection to the backend. Unfortunately, I can't find
any other way to do such a lookup in a thread-safe manner unless we do a
system() or lock/read /etc/passwd ourselves, both of which are ugly.
I can't imagine how some OS's cannot give us a thread-safe way to do
this.
When FreeBSD can't enable threads in 7.5, folks are going to be upset.
In 7.4 we allowed it by having our own C code lock/copy the passwd
structure, but someone pointed out that calls to getpwuid() in other
places in the client code don't have such locking, so it would not work,
so it was removed for 7.5.
I disagree that all or nothing is a good strategy. What you have changed
this to is to deny using PostgreSQL from multithreaded applications on
platforms that have no getpwuid_r() altogether, if their platform
happens to require any thread special compiler options for libpq to work
in general.
Take Slony as an example. It is multithreaded, and we aren't happy that
we have to guard the pg_connect() call with a mutex against multiple
concurrent calls. But since our connections are of long living nature
this is no problem. And nowhere else in the entire code is any call to
getpwuid() or anything else. So we have the situation under control. But
I really don't want to tell people in the build instructions that they
have to edit libpq's Makefile because PostgreSQL's ./configure script is
too restrictive.
OK, I have added a configure option --enable-thread-safety-force that
compiles with thread safety even if the OS tests fail. I have not
mentioned this in the SGML docs but mention the option in configure
--help and when you get a thread test failure with
--enable-thread-safety.
Patch attached and applied.
And just to make your day, your tests for thread safety are incomplete.
The reason why we use a mutex now on all platforms, thread safe or not,
is because in the event you have a kerberos lib available (which is not
thread safe), pg_connect() can crash wether you use kerberos or not. So
I think when compiling for --enable-thread-safe we should disable
kerberos in libpq, right?
We have libpq locking for kerberos in CVS.
--
Bruce Momjian| http://candle.pha.pa.us
[EMAIL PROTECTED] | (610) 359-1001
+ If your life is a hard drive, | 13 Roberts Road
+ Christ can be your backup.| Newtown Square, Pennsylvania 19073
Index: configure
===
RCS file: /cvsroot/pgsql-server/configure,v
retrieving revision 1.376
diff -c -c -r1.376 configure
*** configure 24 Jun 2004 18:55:17 - 1.376
--- configure 10 Jul 2004 01:08:16 -
***
*** 846,851
--- 846,852
--enable-depend turn on automatic dependency tracking
--enable-cassertenable assertion checks (for debugging)
--enable-thread-safety make client libraries thread-safe
+ --enable-thread-safety-force force thread-safety in spite of thread test failure
--disable-largefile omit support for large files
Optional Packages:
***
*** 2937,2947
case $enableval in
yes)
!
! cat confdefs.h \_ACEOF
! #define ENABLE_THREAD_SAFETY 1
! _ACEOF
!
;;
no)
:
--- 2938,2944
case $enableval in
yes)
! :
;;
no)
:
***
*** 2958,2963
--- 2955,2994
fi;
+
+
+ # Check whether --enable-thread-safety-force or --disable-thread-safety-force was
given.
+ if test ${enable_thread_safety_force+set} = set; then
+ enableval=$enable_thread_safety_force
+
+ case $enableval in
+ yes)
+ :
+ ;;
+ no)
+ :
+ ;;
+ *)
+ { { echo $as_me:$LINENO: error: no argument expected for
--enable-thread-safety-force option 5
+ echo $as_me: error: no argument expected for --enable-thread-safety-force option
2;}
+{ (exit 1); exit 1; }; }
+ ;;
+ esac
+
+ else
+ enable_thread_safety_force=no
+
+ fi;
+
+ if test $enable_thread_safety = yes -o
+test $enable_thread_safety_force = yes; then
+ enable_thread_safety=yes # for 'force'
+
+ cat confdefs.h \_ACEOF
+ #define ENABLE_THREAD_SAFETY 1
+ _ACEOF
+
+ fi
echo $as_me:$LINENO: result: $enable_thread_safety 5
echo ${ECHO_T}$enable_thread_safety 6
***
*** 17941,17947
# We have to run the thread test near the end so we have all our symbols
# defined. Cross compiling throws a warning.
#
! if test $enable_thread_safety = yes; then
echo $as_me:$LINENO: checking thread safety of required library functions 5
echo $ECHO_N checking thread safety of required library functions... $ECHO_C 6
--- 17972,17991
# We have to run the thread test near the
Re: [HACKERS] thread safety tests
Jan Wieck wrote: On 6/10/2004 2:11 AM, Tom Lane wrote: Bruce Momjian [EMAIL PROTECTED] writes: Are people OK with requiring PGUSER, $USER, $LOGNAME, or the username to be supplied by the connection string in libpq on platforms that want threads and don't have getpwuid_r() (Solaris, FreeBSD, etc.)? AFAICS that was not what Jan was suggesting at all. I don't like it either --- changing the user-visible behavior based on whether we think the platform is thread-safe or not is horrid. What I understood Jan to be saying is that we should be willing to build the most thread-safe approximation we can when --enable-thread-safety is requested. Don't bomb out if you don't have getpwuid_r, just give a warning and then use getpwuid. Make it so that --enable-thread-safety bombs out, but make another --enable-thread-safey-anyway work the way Tom descibed it. Done as --enable-thread-safety-force. -- Bruce Momjian| http://candle.pha.pa.us [EMAIL PROTECTED] | (610) 359-1001 + If your life is a hard drive, | 13 Roberts Road + Christ can be your backup.| Newtown Square, Pennsylvania 19073 ---(end of broadcast)--- TIP 2: you can get off all lists at once with the unregister command (send unregister YourEmailAddressHere to [EMAIL PROTECTED])
Re: [HACKERS] thread safety tests
Bruce Momjian [EMAIL PROTECTED] writes: Are people OK with requiring PGUSER, $USER, $LOGNAME, or the username to be supplied by the connection string in libpq on platforms that want threads and don't have getpwuid_r() (Solaris, FreeBSD, etc.)? AFAICS that was not what Jan was suggesting at all. I don't like it either --- changing the user-visible behavior based on whether we think the platform is thread-safe or not is horrid. What I understood Jan to be saying is that we should be willing to build the most thread-safe approximation we can when --enable-thread-safety is requested. Don't bomb out if you don't have getpwuid_r, just give a warning and then use getpwuid. regards, tom lane ---(end of broadcast)--- TIP 6: Have you searched our list archives? http://archives.postgresql.org
Re: [HACKERS] thread safety tests
On 6/10/2004 2:11 AM, Tom Lane wrote: Bruce Momjian [EMAIL PROTECTED] writes: Are people OK with requiring PGUSER, $USER, $LOGNAME, or the username to be supplied by the connection string in libpq on platforms that want threads and don't have getpwuid_r() (Solaris, FreeBSD, etc.)? AFAICS that was not what Jan was suggesting at all. I don't like it either --- changing the user-visible behavior based on whether we think the platform is thread-safe or not is horrid. What I understood Jan to be saying is that we should be willing to build the most thread-safe approximation we can when --enable-thread-safety is requested. Don't bomb out if you don't have getpwuid_r, just give a warning and then use getpwuid. Make it so that --enable-thread-safety bombs out, but make another --enable-thread-safey-anyway work the way Tom descibed it. Jan -- #==# # It's easier to get forgiveness for being wrong than for being right. # # Let's break this rule - forgive me. # #== [EMAIL PROTECTED] # ---(end of broadcast)--- TIP 7: don't forget to increase your free space map settings
Re: [HACKERS] thread safety tests
Jan Wieck wrote: On 6/10/2004 2:11 AM, Tom Lane wrote: Bruce Momjian [EMAIL PROTECTED] writes: Are people OK with requiring PGUSER, $USER, $LOGNAME, or the username to be supplied by the connection string in libpq on platforms that want threads and don't have getpwuid_r() (Solaris, FreeBSD, etc.)? AFAICS that was not what Jan was suggesting at all. I don't like it either --- changing the user-visible behavior based on whether we think the platform is thread-safe or not is horrid. What I understood Jan to be saying is that we should be willing to build the most thread-safe approximation we can when --enable-thread-safety is requested. Don't bomb out if you don't have getpwuid_r, just give a warning and then use getpwuid. Make it so that --enable-thread-safety bombs out, but make another --enable-thread-safey-anyway work the way Tom descibed it. Sure, we can do that by just not running the thread_test program. In fact a cross-compile already skips running the test. -- Bruce Momjian| http://candle.pha.pa.us [EMAIL PROTECTED] | (610) 359-1001 + If your life is a hard drive, | 13 Roberts Road + Christ can be your backup.| Newtown Square, Pennsylvania 19073 ---(end of broadcast)--- TIP 4: Don't 'kill -9' the postmaster
Re: [HACKERS] thread safety tests
On 6/10/2004 8:49 AM, Bruce Momjian wrote: Jan Wieck wrote: Make it so that --enable-thread-safety bombs out, but make another --enable-thread-safey-anyway work the way Tom descibed it. Sure, we can do that by just not running the thread_test program. In fact a cross-compile already skips running the test. That sounds good to me. Jan -- #==# # It's easier to get forgiveness for being wrong than for being right. # # Let's break this rule - forgive me. # #== [EMAIL PROTECTED] # ---(end of broadcast)--- TIP 9: the planner will ignore your desire to choose an index scan if your joining column's datatypes do not match
Re: [HACKERS] thread safety tests
Jan Wieck wrote: I am wondering why thread_test.c is checking for mktemp()? That function is nowhere used in the libpq. Uh, it isn't checking for mktemp, it is using it, and it is using it because someone didn't like hard-coded paths I was using in the past. Is there something wrong with using mktemp? I have heard of no portability problems, except some need six X's, and we updated that. Also, I would suggest that we allow to build the libpq with thread specific compiler options, even if it is not entirely thread safe. It would require that a really multithreaded application has to have mutexes around certain operations, but being entirely unable to configure in a way that adds thread compile options to the CFLAGS makes libpq completely useless for multithreaded programs on some platforms (for example Solaris). I am confused what you are suggesting. Are you saying to use thread flags but not the other things that make is thread-safe? There isn't much else other than the flags actually. Now that more OS's are thread-safe by default, we could consider using threading if it is available by default. We would need some way of reporting that to the user, perhaps via an installed readme file or a pg_config output option. -- Bruce Momjian| http://candle.pha.pa.us [EMAIL PROTECTED] | (610) 359-1001 + If your life is a hard drive, | 13 Roberts Road + Christ can be your backup.| Newtown Square, Pennsylvania 19073 ---(end of broadcast)--- TIP 7: don't forget to increase your free space map settings
Re: [HACKERS] thread safety tests
On 6/9/2004 9:36 AM, Bruce Momjian wrote: Also, I would suggest that we allow to build the libpq with thread specific compiler options, even if it is not entirely thread safe. It would require that a really multithreaded application has to have mutexes around certain operations, but being entirely unable to configure in a way that adds thread compile options to the CFLAGS makes libpq completely useless for multithreaded programs on some platforms (for example Solaris). I am confused what you are suggesting. Are you saying to use thread flags but not the other things that make is thread-safe? There isn't much else other than the flags actually. Now that more OS's are thread-safe by default, we could consider using threading if it is available by default. We would need some way of reporting that to the user, perhaps via an installed readme file or a pg_config output option. The problem is that if your thread-safety tests fail, there is no way to build libpq with -pthread and -DREENTRANT or whatever is required on that platform. On Solaris this results in errno being defined as extern int errno; as supposed to #define errno *(errno()) which makes libpq on Solaris completely useless for every program that uses threads for something. There is still value in compiling it with thread support compiler flags, even if it will not result in a thread safe libpq. Jan -- #==# # It's easier to get forgiveness for being wrong than for being right. # # Let's break this rule - forgive me. # #== [EMAIL PROTECTED] # ---(end of broadcast)--- TIP 7: don't forget to increase your free space map settings
Re: [HACKERS] thread safety tests
Jan Wieck wrote:
On 6/9/2004 9:36 AM, Bruce Momjian wrote:
Jan Wieck wrote:
I am wondering why thread_test.c is checking for mktemp()? That function
is nowhere used in the libpq.
Uh, it isn't checking for mktemp, it is using it, and it is using it
because someone didn't like hard-coded paths I was using in the past.
Is there something wrong with using mktemp? I have heard of no
portability problems, except some need six X's, and we updated that.
There seems to be a portability issue here. Stefan Kaltenbrunner
reported a configure failure on sparc64-unknown-openbsd3.5 and the
config.log says:
/tmp//ccx22029.o: In function `main':
/tmp//ccx22029.o(.text+0x8c): warning: mktemp() possibly used unsafely;
consider
using mkstemp()
Yes, I was wondering how mktemp was going to guard against concurrent
access. I have applied the following patch to use mkstemp().
Which is only a warning at this time, it fails later on getpwuid().
Oh, I will need to hear more about that failure.
--
Bruce Momjian| http://candle.pha.pa.us
[EMAIL PROTECTED] | (610) 359-1001
+ If your life is a hard drive, | 13 Roberts Road
+ Christ can be your backup.| Newtown Square, Pennsylvania 19073
Index: src/tools/thread/thread_test.c
===
RCS file: /cvsroot/pgsql-server/src/tools/thread/thread_test.c,v
retrieving revision 1.30
diff -c -c -r1.30 thread_test.c
*** src/tools/thread/thread_test.c 28 May 2004 18:37:10 - 1.30
--- src/tools/thread/thread_test.c 9 Jun 2004 15:03:29 -
***
*** 104,110
{
pthread_t thread1,
thread2;
!
if (argc 1)
{
fprintf(stderr, Usage: %s\n, argv[0]);
--- 104,111
{
pthread_t thread1,
thread2;
! int fd;
!
if (argc 1)
{
fprintf(stderr, Usage: %s\n, argv[0]);
***
*** 120,130
/* Make temp filenames, might not have strdup() */
temp_filename_1 = malloc(strlen(TEMP_FILENAME_1) + 1);
strcpy(temp_filename_1, TEMP_FILENAME_1);
! mktemp(temp_filename_1);
temp_filename_2 = malloc(strlen(TEMP_FILENAME_2) + 1);
strcpy(temp_filename_2, TEMP_FILENAME_2);
! mktemp(temp_filename_2);
#if !defined(HAVE_GETADDRINFO) !defined(HAVE_GETHOSTBYNAME_R)
if (gethostname(myhostname, MAXHOSTNAMELEN) != 0)
--- 121,133
/* Make temp filenames, might not have strdup() */
temp_filename_1 = malloc(strlen(TEMP_FILENAME_1) + 1);
strcpy(temp_filename_1, TEMP_FILENAME_1);
! fd = mkstemp(temp_filename_1);
! close(fd);
temp_filename_2 = malloc(strlen(TEMP_FILENAME_2) + 1);
strcpy(temp_filename_2, TEMP_FILENAME_2);
! fd = mkstemp(temp_filename_2);
! close(fd);
#if !defined(HAVE_GETADDRINFO) !defined(HAVE_GETHOSTBYNAME_R)
if (gethostname(myhostname, MAXHOSTNAMELEN) != 0)
---(end of broadcast)---
TIP 5: Have you checked our extensive FAQ?
http://www.postgresql.org/docs/faqs/FAQ.html
Re: [HACKERS] thread safety tests
On 6/9/2004 9:36 AM, Bruce Momjian wrote: Jan Wieck wrote: I am wondering why thread_test.c is checking for mktemp()? That function is nowhere used in the libpq. Uh, it isn't checking for mktemp, it is using it, and it is using it because someone didn't like hard-coded paths I was using in the past. Is there something wrong with using mktemp? I have heard of no portability problems, except some need six X's, and we updated that. There seems to be a portability issue here. Stefan Kaltenbrunner reported a configure failure on sparc64-unknown-openbsd3.5 and the config.log says: /tmp//ccx22029.o: In function `main': /tmp//ccx22029.o(.text+0x8c): warning: mktemp() possibly used unsafely; consider using mkstemp() Which is only a warning at this time, it fails later on getpwuid(). Jan Also, I would suggest that we allow to build the libpq with thread specific compiler options, even if it is not entirely thread safe. It would require that a really multithreaded application has to have mutexes around certain operations, but being entirely unable to configure in a way that adds thread compile options to the CFLAGS makes libpq completely useless for multithreaded programs on some platforms (for example Solaris). I am confused what you are suggesting. Are you saying to use thread flags but not the other things that make is thread-safe? There isn't much else other than the flags actually. Now that more OS's are thread-safe by default, we could consider using threading if it is available by default. We would need some way of reporting that to the user, perhaps via an installed readme file or a pg_config output option. -- #==# # It's easier to get forgiveness for being wrong than for being right. # # Let's break this rule - forgive me. # #== [EMAIL PROTECTED] # ---(end of broadcast)--- TIP 5: Have you checked our extensive FAQ? http://www.postgresql.org/docs/faqs/FAQ.html
Re: [HACKERS] thread safety tests
On 6/9/2004 11:45 AM, Bruce Momjian wrote: Jan Wieck wrote: The problem is that if your thread-safety tests fail, there is no way to build libpq with -pthread and -DREENTRANT or whatever is required on that platform. On Solaris this results in errno being defined as extern int errno; as supposed to #define errno *(errno()) which makes libpq on Solaris completely useless for every program that uses threads for something. There is still value in compiling it with thread support compiler flags, even if it will not result in a thread safe libpq. Well, first we should find out how to get the thread test to pass for that patform, but for cases where we can't (FreeBSD doesn't have getpwuid_r(), we are stuck. (That might be your Solaris problem as well.) There is no problem with thread safety on Solaris. The configure script for 7.4.2 is broken for it, leading to a broken libpq when using --enable-thread-safey. I looked over the code and the only place getpwuid_r (through pqGetpwuid) is used is in libpq to look up the default username based on the euid for the connection to the backend. Unfortunately, I can't find any other way to do such a lookup in a thread-safe manner unless we do a system() or lock/read /etc/passwd ourselves, both of which are ugly. I can't imagine how some OS's cannot give us a thread-safe way to do this. When FreeBSD can't enable threads in 7.5, folks are going to be upset. In 7.4 we allowed it by having our own C code lock/copy the passwd structure, but someone pointed out that calls to getpwuid() in other places in the client code don't have such locking, so it would not work, so it was removed for 7.5. I disagree that all or nothing is a good strategy. What you have changed this to is to deny using PostgreSQL from multithreaded applications on platforms that have no getpwuid_r() altogether, if their platform happens to require any thread special compiler options for libpq to work in general. Take Slony as an example. It is multithreaded, and we aren't happy that we have to guard the pg_connect() call with a mutex against multiple concurrent calls. But since our connections are of long living nature this is no problem. And nowhere else in the entire code is any call to getpwuid() or anything else. So we have the situation under control. But I really don't want to tell people in the build instructions that they have to edit libpq's Makefile because PostgreSQL's ./configure script is too restrictive. And just to make your day, your tests for thread safety are incomplete. The reason why we use a mutex now on all platforms, thread safe or not, is because in the event you have a kerberos lib available (which is not thread safe), pg_connect() can crash wether you use kerberos or not. So I think when compiling for --enable-thread-safe we should disable kerberos in libpq, right? Jan -- #==# # It's easier to get forgiveness for being wrong than for being right. # # Let's break this rule - forgive me. # #== [EMAIL PROTECTED] # ---(end of broadcast)--- TIP 7: don't forget to increase your free space map settings
Re: [HACKERS] thread safety tests
On 6/9/2004 11:16 AM, Bruce Momjian wrote:
Jan Wieck wrote:
On 6/9/2004 9:36 AM, Bruce Momjian wrote:
Jan Wieck wrote:
I am wondering why thread_test.c is checking for mktemp()? That function
is nowhere used in the libpq.
Uh, it isn't checking for mktemp, it is using it, and it is using it
because someone didn't like hard-coded paths I was using in the past.
Is there something wrong with using mktemp? I have heard of no
portability problems, except some need six X's, and we updated that.
There seems to be a portability issue here. Stefan Kaltenbrunner
reported a configure failure on sparc64-unknown-openbsd3.5 and the
config.log says:
/tmp//ccx22029.o: In function `main':
/tmp//ccx22029.o(.text+0x8c): warning: mktemp() possibly used unsafely;
consider
using mkstemp()
Yes, I was wondering how mktemp was going to guard against concurrent
access. I have applied the following patch to use mkstemp().
Which is only a warning at this time, it fails later on getpwuid().
Oh, I will need to hear more about that failure.
The relevant part of the config.log is:
configure:17942: checking thread safety of required library functions
configure:17967: gcc -o conftest -O2 -fno-strict-aliasing -pthread
-D_REENTRANT -D_THREAD_SAFE -D_POSIX_PTHREAD_SEMANTICS -DIN_CONFIGURE
conftest.c -lz -lreadline -lcurses -lresolv -lcompat -lm
-lutil 5
/tmp//ccx22029.o: In function `main':
/tmp//ccx22029.o(.text+0x8c): warning: mktemp() possibly used unsafely;
consider using mkstemp()
configure:17970: $? = 0
configure:17972: ./conftest
Your errno is thread-safe.
Your system has sterror_r(); it does not need strerror().
Your system uses getpwuid() which is not thread-safe. **
Your system has getaddrinfo(); it does not need gethostbyname()
or gethostbyname_r().
** YOUR PLATFORM IS NOT THREAD-SAFE. **
configure:17975: $? = 1
configure: program exited with status 1
configure: failed program was:
#line 17961 configure
#include confdefs.h
#include ./src/tools/thread/thread_test.c
configure:17984: result: no
configure:17986: error:
*** Thread test program failed. Your platform is not thread-safe.
*** Check the file 'config.log'for the exact reason.
Index: src/tools/thread/thread_test.c
===
RCS file: /cvsroot/pgsql-server/src/tools/thread/thread_test.c,v
retrieving revision 1.30
diff -c -c -r1.30 thread_test.c
*** src/tools/thread/thread_test.c 28 May 2004 18:37:10 - 1.30
--- src/tools/thread/thread_test.c 9 Jun 2004 15:03:29 -
***
*** 104,110
{
pthread_t thread1,
thread2;
!
if (argc 1)
{
fprintf(stderr, Usage: %s\n, argv[0]);
--- 104,111
{
pthread_t thread1,
thread2;
! int fd;
!
if (argc 1)
{
fprintf(stderr, Usage: %s\n, argv[0]);
***
*** 120,130
/* Make temp filenames, might not have strdup() */
temp_filename_1 = malloc(strlen(TEMP_FILENAME_1) + 1);
strcpy(temp_filename_1, TEMP_FILENAME_1);
! mktemp(temp_filename_1);
temp_filename_2 = malloc(strlen(TEMP_FILENAME_2) + 1);
strcpy(temp_filename_2, TEMP_FILENAME_2);
! mktemp(temp_filename_2);
#if !defined(HAVE_GETADDRINFO) !defined(HAVE_GETHOSTBYNAME_R)
if (gethostname(myhostname, MAXHOSTNAMELEN) != 0)
--- 121,133
/* Make temp filenames, might not have strdup() */
temp_filename_1 = malloc(strlen(TEMP_FILENAME_1) + 1);
strcpy(temp_filename_1, TEMP_FILENAME_1);
! fd = mkstemp(temp_filename_1);
! close(fd);
temp_filename_2 = malloc(strlen(TEMP_FILENAME_2) + 1);
strcpy(temp_filename_2, TEMP_FILENAME_2);
! fd = mkstemp(temp_filename_2);
! close(fd);
#if !defined(HAVE_GETADDRINFO) !defined(HAVE_GETHOSTBYNAME_R)
if (gethostname(myhostname, MAXHOSTNAMELEN) != 0)
--
#==#
# It's easier to get forgiveness for being wrong than for being right. #
# Let's break this rule - forgive me. #
#== [EMAIL PROTECTED] #
---(end of broadcast)---
TIP 5: Have you checked our extensive FAQ?
http://www.postgresql.org/docs/faqs/FAQ.html
Re: [HACKERS] thread safety tests
Jan Wieck wrote: On 6/9/2004 11:45 AM, Bruce Momjian wrote: Jan Wieck wrote: The problem is that if your thread-safety tests fail, there is no way to build libpq with -pthread and -DREENTRANT or whatever is required on that platform. On Solaris this results in errno being defined as extern int errno; as supposed to #define errno *(errno()) which makes libpq on Solaris completely useless for every program that uses threads for something. There is still value in compiling it with thread support compiler flags, even if it will not result in a thread safe libpq. Well, first we should find out how to get the thread test to pass for that patform, but for cases where we can't (FreeBSD doesn't have getpwuid_r(), we are stuck. (That might be your Solaris problem as well.) There is no problem with thread safety on Solaris. The configure script for 7.4.2 is broken for it, leading to a broken libpq when using --enable-thread-safey. Oh, yes, to be fixed in 7.4.3. I looked over the code and the only place getpwuid_r (through pqGetpwuid) is used is in libpq to look up the default username based on the euid for the connection to the backend. Unfortunately, I can't find any other way to do such a lookup in a thread-safe manner unless we do a system() or lock/read /etc/passwd ourselves, both of which are ugly. I can't imagine how some OS's cannot give us a thread-safe way to do this. When FreeBSD can't enable threads in 7.5, folks are going to be upset. In 7.4 we allowed it by having our own C code lock/copy the passwd structure, but someone pointed out that calls to getpwuid() in other places in the client code don't have such locking, so it would not work, so it was removed for 7.5. I disagree that all or nothing is a good strategy. What you have changed this to is to deny using PostgreSQL from multithreaded applications on platforms that have no getpwuid_r() altogether, if their platform happens to require any thread special compiler options for libpq to work in general. Yes, this was the agreed approach. It wasn't all my idea. Take Slony as an example. It is multithreaded, and we aren't happy that we have to guard the pg_connect() call with a mutex against multiple concurrent calls. But since our connections are of long living nature this is no problem. And nowhere else in the entire code is any call to getpwuid() or anything else. So we have the situation under control. But I really don't want to tell people in the build instructions that they have to edit libpq's Makefile because PostgreSQL's ./configure script is too restrictive. Yep, a problem. However, when we create libpq we can't know that your app isn't going to call getpwuid itself, can we; and even if we did, we couldn't know if other libraries you are calling are using it. I bet you don't even know that. And just to make your day, your tests for thread safety are incomplete. Why incomplete? Because it doesn't handle kerberos? See below. The reason why we use a mutex now on all platforms, thread safe or not, is because in the event you have a kerberos lib available (which is not thread safe), pg_connect() can crash wether you use kerberos or not. So I think when compiling for --enable-thread-safe we should disable kerberos in libpq, right? I thought we added kerberos locking to our current CVS. As for your general issue, I don't think we want to head down the road of having a libpq that is safe only if certain qualifications are made of clients calling it. If folks know enough to deal with those qualifications, they can use CFLAGS to pass compiler flags to their libpq build or hack up the code to bypass the thread checks. What we really need is a way to do the uid-username mapping in a thread-safe way. Could we check the environment for $USER or $LOGNAME? Could we require them to be set for thread builds on OS's without getpwuid_r and in cases where the username is not specified in the connection string? If FreeBSD and Solaris both have this issue, it is important for us to solve it. -- Bruce Momjian| http://candle.pha.pa.us [EMAIL PROTECTED] | (610) 359-1001 + If your life is a hard drive, | 13 Roberts Road + Christ can be your backup.| Newtown Square, Pennsylvania 19073 ---(end of broadcast)--- TIP 2: you can get off all lists at once with the unregister command (send unregister YourEmailAddressHere to [EMAIL PROTECTED])
Re: [HACKERS] thread safety tests
Well, looks like you are missing getpwuid_r():
Your system uses getpwuid() which is not thread-safe. **
and we don't have any workaround for this.
---
Jan Wieck wrote:
On 6/9/2004 11:16 AM, Bruce Momjian wrote:
Jan Wieck wrote:
On 6/9/2004 9:36 AM, Bruce Momjian wrote:
Jan Wieck wrote:
I am wondering why thread_test.c is checking for mktemp()? That function
is nowhere used in the libpq.
Uh, it isn't checking for mktemp, it is using it, and it is using it
because someone didn't like hard-coded paths I was using in the past.
Is there something wrong with using mktemp? I have heard of no
portability problems, except some need six X's, and we updated that.
There seems to be a portability issue here. Stefan Kaltenbrunner
reported a configure failure on sparc64-unknown-openbsd3.5 and the
config.log says:
/tmp//ccx22029.o: In function `main':
/tmp//ccx22029.o(.text+0x8c): warning: mktemp() possibly used unsafely;
consider
using mkstemp()
Yes, I was wondering how mktemp was going to guard against concurrent
access. I have applied the following patch to use mkstemp().
Which is only a warning at this time, it fails later on getpwuid().
Oh, I will need to hear more about that failure.
The relevant part of the config.log is:
configure:17942: checking thread safety of required library functions
configure:17967: gcc -o conftest -O2 -fno-strict-aliasing -pthread
-D_REENTRANT -D_THREAD_SAFE -D_POSIX_PTHREAD_SEMANTICS -DIN_CONFIGURE
conftest.c -lz -lreadline -lcurses -lresolv -lcompat -lm
-lutil 5
/tmp//ccx22029.o: In function `main':
/tmp//ccx22029.o(.text+0x8c): warning: mktemp() possibly used unsafely;
consider using mkstemp()
configure:17970: $? = 0
configure:17972: ./conftest
Your errno is thread-safe.
Your system has sterror_r(); it does not need strerror().
Your system uses getpwuid() which is not thread-safe. **
Your system has getaddrinfo(); it does not need gethostbyname()
or gethostbyname_r().
** YOUR PLATFORM IS NOT THREAD-SAFE. **
configure:17975: $? = 1
configure: program exited with status 1
configure: failed program was:
#line 17961 configure
#include confdefs.h
#include ./src/tools/thread/thread_test.c
configure:17984: result: no
configure:17986: error:
*** Thread test program failed. Your platform is not thread-safe.
*** Check the file 'config.log'for the exact reason.
Index: src/tools/thread/thread_test.c
===
RCS file: /cvsroot/pgsql-server/src/tools/thread/thread_test.c,v
retrieving revision 1.30
diff -c -c -r1.30 thread_test.c
*** src/tools/thread/thread_test.c 28 May 2004 18:37:10 - 1.30
--- src/tools/thread/thread_test.c 9 Jun 2004 15:03:29 -
***
*** 104,110
{
pthread_t thread1,
thread2;
!
if (argc 1)
{
fprintf(stderr, Usage: %s\n, argv[0]);
--- 104,111
{
pthread_t thread1,
thread2;
! int fd;
!
if (argc 1)
{
fprintf(stderr, Usage: %s\n, argv[0]);
***
*** 120,130
/* Make temp filenames, might not have strdup() */
temp_filename_1 = malloc(strlen(TEMP_FILENAME_1) + 1);
strcpy(temp_filename_1, TEMP_FILENAME_1);
! mktemp(temp_filename_1);
temp_filename_2 = malloc(strlen(TEMP_FILENAME_2) + 1);
strcpy(temp_filename_2, TEMP_FILENAME_2);
! mktemp(temp_filename_2);
#if !defined(HAVE_GETADDRINFO) !defined(HAVE_GETHOSTBYNAME_R)
if (gethostname(myhostname, MAXHOSTNAMELEN) != 0)
--- 121,133
/* Make temp filenames, might not have strdup() */
temp_filename_1 = malloc(strlen(TEMP_FILENAME_1) + 1);
strcpy(temp_filename_1, TEMP_FILENAME_1);
! fd = mkstemp(temp_filename_1);
! close(fd);
temp_filename_2 = malloc(strlen(TEMP_FILENAME_2) + 1);
strcpy(temp_filename_2, TEMP_FILENAME_2);
! fd = mkstemp(temp_filename_2);
! close(fd);
#if !defined(HAVE_GETADDRINFO) !defined(HAVE_GETHOSTBYNAME_R)
if (gethostname(myhostname, MAXHOSTNAMELEN) != 0)
--
#==#
# It's easier to get forgiveness for being wrong than for being right. #
# Let's break this rule - forgive me. #
#== [EMAIL PROTECTED] #
--
Bruce Momjian| http://candle.pha.pa.us
[EMAIL PROTECTED] | (610) 359-1001
+ If your life is a hard drive, | 13 Roberts Road
+ Christ can be your backup.| Newtown Square,
Re: [HACKERS] thread safety tests
On 6/9/2004 1:04 PM, Bruce Momjian wrote: What we really need is a way to do the uid-username mapping in a thread-safe way. Could we check the environment for $USER or $LOGNAME? Could we require them to be set for thread builds on OS's without getpwuid_r and in cases where the username is not specified in the connection string? Maybe not as popular, but what about breaking backward compatibility and require the DB name to be specified, no username fallback? How many applications really rely on that feature? And people who are used to it from the commandline can set PGDATABASE in their .profile to get it back. Jan -- #==# # It's easier to get forgiveness for being wrong than for being right. # # Let's break this rule - forgive me. # #== [EMAIL PROTECTED] # ---(end of broadcast)--- TIP 8: explain analyze is your friend
Re: [HACKERS] thread safety tests
Jan Wieck wrote: On 6/9/2004 1:04 PM, Bruce Momjian wrote: What we really need is a way to do the uid-username mapping in a thread-safe way. Could we check the environment for $USER or $LOGNAME? Could we require them to be set for thread builds on OS's without getpwuid_r and in cases where the username is not specified in the connection string? Maybe not as popular, but what about breaking backward compatibility and require the DB name to be specified, no username fallback? How many applications really rely on that feature? And people who are used to it from the commandline can set PGDATABASE in their .profile to get it back. That is only part of where the username is used. I assume it is also used for connections when the username isn't supplied, not just as the default for the database name. Basically on those platforms, either the username would have to be in the environment, or supplied as part of the connection string. -- Bruce Momjian| http://candle.pha.pa.us [EMAIL PROTECTED] | (610) 359-1001 + If your life is a hard drive, | 13 Roberts Road + Christ can be your backup.| Newtown Square, Pennsylvania 19073 ---(end of broadcast)--- TIP 9: the planner will ignore your desire to choose an index scan if your joining column's datatypes do not match
Re: [HACKERS] thread safety tests
On 6/9/2004 1:44 PM, Bruce Momjian wrote: Jan Wieck wrote: On 6/9/2004 1:04 PM, Bruce Momjian wrote: What we really need is a way to do the uid-username mapping in a thread-safe way. Could we check the environment for $USER or $LOGNAME? Could we require them to be set for thread builds on OS's without getpwuid_r and in cases where the username is not specified in the connection string? Maybe not as popular, but what about breaking backward compatibility and require the DB name to be specified, no username fallback? How many applications really rely on that feature? And people who are used to it from the commandline can set PGDATABASE in their .profile to get it back. That is only part of where the username is used. I assume it is also used for connections when the username isn't supplied, not just as the default for the database name. Basically on those platforms, either the username would have to be in the environment, or supplied as part of the connection string. We have PGUSER, PGHOST, PGPORT, PGDATABASE, all of them you can set in your .profile, why do we need to lookup the uid at all? Jan -- #==# # It's easier to get forgiveness for being wrong than for being right. # # Let's break this rule - forgive me. # #== [EMAIL PROTECTED] # ---(end of broadcast)--- TIP 2: you can get off all lists at once with the unregister command (send unregister YourEmailAddressHere to [EMAIL PROTECTED])
Re: [HACKERS] thread safety tests
Jan Wieck wrote: On 6/9/2004 1:44 PM, Bruce Momjian wrote: Jan Wieck wrote: On 6/9/2004 1:04 PM, Bruce Momjian wrote: What we really need is a way to do the uid-username mapping in a thread-safe way. Could we check the environment for $USER or $LOGNAME? Could we require them to be set for thread builds on OS's without getpwuid_r and in cases where the username is not specified in the connection string? Maybe not as popular, but what about breaking backward compatibility and require the DB name to be specified, no username fallback? How many applications really rely on that feature? And people who are used to it from the commandline can set PGDATABASE in their .profile to get it back. That is only part of where the username is used. I assume it is also used for connections when the username isn't supplied, not just as the default for the database name. Basically on those platforms, either the username would have to be in the environment, or supplied as part of the connection string. We have PGUSER, PGHOST, PGPORT, PGDATABASE, all of them you can set in your .profile, why do we need to lookup the uid at all? In case they don't set it, which is very common, I am sure. -- Bruce Momjian| http://candle.pha.pa.us [EMAIL PROTECTED] | (610) 359-1001 + If your life is a hard drive, | 13 Roberts Road + Christ can be your backup.| Newtown Square, Pennsylvania 19073 ---(end of broadcast)--- TIP 5: Have you checked our extensive FAQ? http://www.postgresql.org/docs/faqs/FAQ.html
Re: [HACKERS] thread safety tests
Are people OK with requiring PGUSER, $USER, $LOGNAME, or the username to be supplied by the connection string in libpq on platforms that want threads and don't have getpwuid_r() (Solaris, FreeBSD, etc.)? If so, I can easily create a patch and apply it. --- Jan Wieck wrote: On 6/9/2004 1:44 PM, Bruce Momjian wrote: Jan Wieck wrote: On 6/9/2004 1:04 PM, Bruce Momjian wrote: What we really need is a way to do the uid-username mapping in a thread-safe way. Could we check the environment for $USER or $LOGNAME? Could we require them to be set for thread builds on OS's without getpwuid_r and in cases where the username is not specified in the connection string? Maybe not as popular, but what about breaking backward compatibility and require the DB name to be specified, no username fallback? How many applications really rely on that feature? And people who are used to it from the commandline can set PGDATABASE in their .profile to get it back. That is only part of where the username is used. I assume it is also used for connections when the username isn't supplied, not just as the default for the database name. Basically on those platforms, either the username would have to be in the environment, or supplied as part of the connection string. We have PGUSER, PGHOST, PGPORT, PGDATABASE, all of them you can set in your .profile, why do we need to lookup the uid at all? Jan -- #==# # It's easier to get forgiveness for being wrong than for being right. # # Let's break this rule - forgive me. # #== [EMAIL PROTECTED] # -- Bruce Momjian| http://candle.pha.pa.us [EMAIL PROTECTED] | (610) 359-1001 + If your life is a hard drive, | 13 Roberts Road + Christ can be your backup.| Newtown Square, Pennsylvania 19073 ---(end of broadcast)--- TIP 9: the planner will ignore your desire to choose an index scan if your joining column's datatypes do not match
