You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@httpd.apache.org by Jeff Trawick <tr...@bellsouth.net> on 2000/11/10 21:15:57 UTC
[?PATCH?] using getpwnam_r in mod_userdir
(Sorry about tossing the commit message prematurely.)
In practice, _POSIX_THREAD_SAFE_FUNCTIONS doesn't imply the existence
of getpwnam_r(). Example: FreeBSD 3.4
We need to check if we actually have the function...
This change works on FreeBSD 3.4 and Mandrake 7.0, but I'm concerned
that it may not work everywhere... If getpwnam_r() is in a special
library (e.g., libpthread), we won't know to link against the library
while we're doing module configuration. APR figures out that stuff
for us, and we aren't that far yet.
some possibilities:
. punt this section of code to APR by creating an apr_get_home_dir()
function somewhere (but maybe not appropriate on Win32?)
. use the code below until it breaks, then figure out what to do
. have APR export APR_HAVE_GETPWNAM_R to the app via apr.h
cvs server: Diffing modules/standard
Index: modules/standard/config.m4
===================================================================
RCS file: /cvs/apache/apache-2.0/src/modules/standard/config.m4,v
retrieving revision 1.30
diff -u -r1.30 config.m4
--- config.m4 2000/10/24 11:45:41 1.30
+++ config.m4 2000/11/10 20:02:33
@@ -25,7 +25,9 @@
APACHE_CHECK_STANDARD_MODULE(imap, internal imagemaps, , yes)
APACHE_CHECK_STANDARD_MODULE(actions, Action triggering on requests, action, yes)
APACHE_CHECK_STANDARD_MODULE(speling, correct common URL misspellings, , no)
-APACHE_CHECK_STANDARD_MODULE(userdir, mapping of user requests, , yes)
+APACHE_CHECK_STANDARD_MODULE(userdir, mapping of user requests, , yes, [
+ AC_CHECK_FUNCS(getpwnam_r)
+])
APACHE_CHECK_STANDARD_MODULE(suexec, set uid and gid for spawned processes, , no)
APACHE_CHECK_STANDARD_MODULE(alias, translation of requests, , yes)
Index: modules/standard/mod_userdir.c
===================================================================
RCS file: /cvs/apache/apache-2.0/src/modules/standard/mod_userdir.c,v
retrieving revision 1.25
diff -u -r1.25 mod_userdir.c
--- mod_userdir.c 2000/11/10 18:18:57 1.25
+++ mod_userdir.c 2000/11/10 20:02:33
@@ -326,7 +326,7 @@
#else /* WIN32 */
struct passwd *pw;
-#if APR_HAS_THREADS && defined(_POSIX_THREAD_SAFE_FUNCTIONS)
+#if APR_HAS_THREADS && defined(HAVE_GETPWNAM_R)
struct passwd pwd;
size_t buflen = sysconf(_SC_GETPW_R_SIZE_MAX);
char *buf = apr_pcalloc(r->pool, buflen);
--
Jeff Trawick | trawick@ibm.net | PGP public key at web site:
http://www.geocities.com/SiliconValley/Park/9289/
Born in Roswell... married an alien...
Re: [?PATCH?] using getpwnam_r in mod_userdir
Posted by rb...@covalent.net.
> > Before anybody tells me we can work around these issues, remember that we
> > really have no idea what functions are called internal to the C Run-time,
> > so we can't fix these problems 100%. If a platform doesn't provide a
> > known thread-safe implementation, then we really can't hack a valid
> > long-term solution. Imagine the case where both getpwnam and getpwent
> > make a call to the same function within the C run-time, and we have two
> > modules one that calls getpwent and one that calls getpwnam. We can't
> > protect ourselves from threading issues in that case. It isn't
> > possible.
>
> There are certainly theoretical difficulties... I almost suggested
> something similar+ but I don't know how many folks will find it cool
> to throw out the ability to thread on FreeBSD.
These aren't actual theoretical, they are real issues. The nice thing
about FreeBSD is that it is open source. :-) If people are upset that
the server can't be run in threaded mode, then people can fix the server.
> Are we sure that this won't be an issue for other APR apps too?
It will be an issue. Even worse, is that other APR apps are potentially
MORE likely to hit these problems, because they are more likely to try to
call thread-unsafe functions when they are threaded. Because of the
nature of httpd, we are just unlikely to hit these issues too often. Of
course, anytime you say that, we are garaunteed to hit them. :-)
> +have APR refuse to configure --with-threads if
> _POSIX_THREAD_SAFE_FUNCTIONS was defined but getpwnam_r() wasn't
> available, and document that getpwnam() better be thread-safe if we
> have threads but no _POSIX_THREAD_SAFE_FUNCTIONS
That's my current opinion, but I am willing to be convinced.
Ryan
_______________________________________________________________________________
Ryan Bloom rbb@apache.org
406 29th St.
San Francisco, CA 94131
-------------------------------------------------------------------------------
Re: [?PATCH?] using getpwnam_r in mod_userdir
Posted by Jeff Trawick <tr...@bellsouth.net>.
rbb@covalent.net writes:
> On 10 Nov 2000, Jeff Trawick wrote:
>
> This whole problem is bogus. I am very much leaning towards saying that
> since this problem is really unsolvable we should just not support thread
> Apache on these platforms.
>
> Before anybody tells me we can work around these issues, remember that we
> really have no idea what functions are called internal to the C Run-time,
> so we can't fix these problems 100%. If a platform doesn't provide a
> known thread-safe implementation, then we really can't hack a valid
> long-term solution. Imagine the case where both getpwnam and getpwent
> make a call to the same function within the C run-time, and we have two
> modules one that calls getpwent and one that calls getpwnam. We can't
> protect ourselves from threading issues in that case. It isn't
> possible.
There are certainly theoretical difficulties... I almost suggested
something similar+ but I don't know how many folks will find it cool
to throw out the ability to thread on FreeBSD.
Are we sure that this won't be an issue for other APR apps too?
+have APR refuse to configure --with-threads if
_POSIX_THREAD_SAFE_FUNCTIONS was defined but getpwnam_r() wasn't
available, and document that getpwnam() better be thread-safe if we
have threads but no _POSIX_THREAD_SAFE_FUNCTIONS
--
Jeff Trawick | trawick@ibm.net | PGP public key at web site:
http://www.geocities.com/SiliconValley/Park/9289/
Born in Roswell... married an alien...
Re: [?PATCH?] using getpwnam_r in mod_userdir
Posted by rb...@covalent.net.
> > You can't do that either. In many cases, the _r functions are much less
> > efficient than the non-_r functions. So, if you always use _r you lose
> > efficiency. Our wrappers would actually need to use _r or non-_r based on
> > how it is compiled.
>
> I think you are confusing something now..
>
> An _r wrapper is only used, if the platform does not have the
> corresponding _r function.
>
> Thus, an _r wrapper always uses the non-_r function.
>
> Do you mean that native _r functions are much less efficient?
On some platforms, the _r function is actually quite a bit less
efficient. I am looking right now to see how easy it would be for us to
implement a thread safe version of this stuff on a per-platform basis to
get around this problem. I am not motivated to do this, but oh well.
> I agree that fixing the platforms is the right mid-term to
> long-term approach. It is a bogus problem which will
> hopefully go away soon. In the mean-time, we should try to
> address the problem though. That is all a Portable Runtime is
> about.
I know, I'm just not in a great mood today. I may post something later to
solve this problem, or I may give up for the night.
Ryan
_______________________________________________________________________________
Ryan Bloom rbb@apache.org
406 29th St.
San Francisco, CA 94131
-------------------------------------------------------------------------------
Re: [?PATCH?] using getpwnam_r in mod_userdir
Posted by Sascha Schumann <sa...@schumann.cx>.
> > Remember that we need to have a wrapper anyway for all of
> > those _r functions, because the source code will use _r
> > functions even in non-threaded mode. (Unless you choose to
> > duplicate code for both modes all over the place.)
>
> You can't do that either. In many cases, the _r functions are much less
> efficient than the non-_r functions. So, if you always use _r you lose
> efficiency. Our wrappers would actually need to use _r or non-_r based on
> how it is compiled.
I think you are confusing something now..
An _r wrapper is only used, if the platform does not have the
corresponding _r function.
Thus, an _r wrapper always uses the non-_r function.
Do you mean that native _r functions are much less efficient?
> > I think we should use a single lock for protecting one
> > sub-system. We need to define `sub-system' here. FreeBSD will
> > need one lock for getpw functions.
> >
> > If a certain platform shows signs of hidden inter-function
> > dependencies, I'd be in favor of disabling threading on that
> > platform. Experience will tell.
>
> My own personal opinion is that this is a bogus problem because some
> platforms aren't following the specs. We shouldn't cater to broken
> platforms, especially not open source broken platforms. Those platforms
> should be fixed correctly.
I agree that fixing the platforms is the right mid-term to
long-term approach. It is a bogus problem which will
hopefully go away soon. In the mean-time, we should try to
address the problem though. That is all a Portable Runtime is
about.
- Sascha
Re: [?PATCH?] using getpwnam_r in mod_userdir
Posted by rb...@covalent.net.
> > Before anybody tells me we can work around these issues, remember that we
> > really have no idea what functions are called internal to the C Run-time,
> > so we can't fix these problems 100%. If a platform doesn't provide a
> > known thread-safe implementation, then we really can't hack a valid
> > long-term solution. Imagine the case where both getpwnam and getpwent
> > make a call to the same function within the C run-time, and we have two
> > modules one that calls getpwent and one that calls getpwnam. We can't
> > protect ourselves from threading issues in that case. It isn't possible.
>
> That's a problem indeed.
>
> Remember that we need to have a wrapper anyway for all of
> those _r functions, because the source code will use _r
> functions even in non-threaded mode. (Unless you choose to
> duplicate code for both modes all over the place.)
You can't do that either. In many cases, the _r functions are much less
efficient than the non-_r functions. So, if you always use _r you lose
efficiency. Our wrappers would actually need to use _r or non-_r based on
how it is compiled.
> I think we should use a single lock for protecting one
> sub-system. We need to define `sub-system' here. FreeBSD will
> need one lock for getpw functions.
>
> If a certain platform shows signs of hidden inter-function
> dependencies, I'd be in favor of disabling threading on that
> platform. Experience will tell.
My own personal opinion is that this is a bogus problem because some
platforms aren't following the specs. We shouldn't cater to broken
platforms, especially not open source broken platforms. Those platforms
should be fixed correctly.
Ryan
_______________________________________________________________________________
Ryan Bloom rbb@apache.org
406 29th St.
San Francisco, CA 94131
-------------------------------------------------------------------------------
Re: [?PATCH?] using getpwnam_r in mod_userdir
Posted by Sascha Schumann <sa...@schumann.cx>.
> This whole problem is bogus. I am very much leaning towards saying that
> since this problem is really unsolvable we should just not support thread
> Apache on these platforms.
Well, I think we can refine the 'throwing-out-the-child-with-
the-bathwater-approach'.
> Before anybody tells me we can work around these issues, remember that we
> really have no idea what functions are called internal to the C Run-time,
> so we can't fix these problems 100%. If a platform doesn't provide a
> known thread-safe implementation, then we really can't hack a valid
> long-term solution. Imagine the case where both getpwnam and getpwent
> make a call to the same function within the C run-time, and we have two
> modules one that calls getpwent and one that calls getpwnam. We can't
> protect ourselves from threading issues in that case. It isn't possible.
That's a problem indeed.
Remember that we need to have a wrapper anyway for all of
those _r functions, because the source code will use _r
functions even in non-threaded mode. (Unless you choose to
duplicate code for both modes all over the place.)
I think we should use a single lock for protecting one
sub-system. We need to define `sub-system' here. FreeBSD will
need one lock for getpw functions.
If a certain platform shows signs of hidden inter-function
dependencies, I'd be in favor of disabling threading on that
platform. Experience will tell.
- Sascha
Re: [?PATCH?] using getpwnam_r in mod_userdir
Posted by rb...@covalent.net.
On 10 Nov 2000, Jeff Trawick wrote:
> > If it is thread-safe (i.e. it uses thread-specific storage
> > instead of static storage), the vendor should provide
> > getpwnam_r. In fact, getpwnam should then only be a wrapper
> > around getpwnam_r.
>
> This isn't about *should* :(
>
> > Are there any known systems which provide a thread-safe
> > getpwnam and don't have getpwnam_r?
>
> OS/390: no _r functions, standard ones use thread-specific storage,
> _POSIX_THREAD_SAFE_FUNCTIONS is not defined
That is fine. The OS/390 case is a non-issue, because it is saying that
we should be using the non _r functions by not defining
_POSIX_THREAD_SAFE_FUNCTIONS.
This whole problem is bogus. I am very much leaning towards saying that
since this problem is really unsolvable we should just not support thread
Apache on these platforms.
Before anybody tells me we can work around these issues, remember that we
really have no idea what functions are called internal to the C Run-time,
so we can't fix these problems 100%. If a platform doesn't provide a
known thread-safe implementation, then we really can't hack a valid
long-term solution. Imagine the case where both getpwnam and getpwent
make a call to the same function within the C run-time, and we have two
modules one that calls getpwent and one that calls getpwnam. We can't
protect ourselves from threading issues in that case. It isn't possible.
Ryan
_______________________________________________________________________________
Ryan Bloom rbb@apache.org
406 29th St.
San Francisco, CA 94131
-------------------------------------------------------------------------------
Re: [?PATCH?] using getpwnam_r in mod_userdir
Posted by Jeff Trawick <tr...@bellsouth.net>.
Sascha Schumann <sa...@schumann.cx> writes:
> > Another issue (perhaps not affecting more than one system): it
> > assumes that getpwnam() isn't thread-safe. If getpwnam() is
> > thread-safe, we don't need the lock and we don't need to copy.
>
> If it is thread-safe (i.e. it uses thread-specific storage
> instead of static storage), the vendor should provide
> getpwnam_r. In fact, getpwnam should then only be a wrapper
> around getpwnam_r.
This isn't about *should* :(
> Are there any known systems which provide a thread-safe
> getpwnam and don't have getpwnam_r?
OS/390: no _r functions, standard ones use thread-specific storage,
_POSIX_THREAD_SAFE_FUNCTIONS is not defined
> > How many of these reentrant function issues have come up with PHP?
>
> Well, quite a lot. We try hard to avoid compatibility layers
> in normal source code, so we use the reentrant interfaces
> everywhere. [text deleted]
Maybe we should just put the relevant PHP code in APR (changes
required, of course, but not too many as I think your example showed)?
--
Jeff Trawick | trawick@ibm.net | PGP public key at web site:
http://www.geocities.com/SiliconValley/Park/9289/
Born in Roswell... married an alien...
Re: [?PATCH?] using getpwnam_r in mod_userdir
Posted by Sascha Schumann <sa...@schumann.cx>.
> Another issue (perhaps not affecting more than one system): it
> assumes that getpwnam() isn't thread-safe. If getpwnam() is
> thread-safe, we don't need the lock and we don't need to copy.
If it is thread-safe (i.e. it uses thread-specific storage
instead of static storage), the vendor should provide
getpwnam_r. In fact, getpwnam should then only be a wrapper
around getpwnam_r.
Are there any known systems which provide a thread-safe
getpwnam and don't have getpwnam_r?
> How many of these reentrant function issues have come up with PHP?
Well, quite a lot. We try hard to avoid compatibility layers
in normal source code, so we use the reentrant interfaces
everywhere. If the target platform has
- old, but reentrant interfaces, we try to support them
(HPUX time-related functions, Solaris(/AIX?) readdir_r());
or if it has
- only non-reentrant interfaces, we supply wrappers.
Currently, we cover these functions:
asctime_r
ctime_r
gmtime_r
localtime_r
rand_r*
readdir_r
strtok_r*
* completely reimplemented for performance reasons
That is almost the complete set of _r functions, considering
the 1995 Amendment to POSIX. The get*_r functions are missing
though as we currently don't have any need for them.
- Sascha
Re: [?PATCH?] using getpwnam_r in mod_userdir
Posted by Jeff Trawick <tr...@bellsouth.net>.
Sascha Schumann <sa...@schumann.cx> writes:
> Our "user" code like mod_userdir.c should just assume that
> getpwnam_r exists. If it does not exist, we can provide a
> fallback which serializes access to the OS-function.
> Otherwise, we start duplicating work-arounds and checks all
> over the place.
>
> I'm attaching such a fallback for getpwnam_r. There are two
> issues with this:
>
> - ACQUIRE_LOCK/RELEASE_LOCK should be defined to mutex-like
> functions, if we are compiling thread-safe (otherwise,
> they are nops).
> - The getpwnam_r function should be prefixed properly to
> avoid conflicts with system functions which have the same
> name, but another interface.
Another issue (perhaps not affecting more than one system): it
assumes that getpwnam() isn't thread-safe. If getpwnam() is
thread-safe, we don't need the lock and we don't need to copy.
How many of these reentrant function issues have come up with PHP?
--
Jeff Trawick | trawick@ibm.net | PGP public key at web site:
http://www.geocities.com/SiliconValley/Park/9289/
Born in Roswell... married an alien...
Re: [?PATCH?] using getpwnam_r in mod_userdir
Posted by Sascha Schumann <sa...@schumann.cx>.
On Fri, 10 Nov 2000 rbb@covalent.net wrote:
> > >
> > > Argh. :-( The general rule that we used in APR was that if
> > > _POSIX_THREAD_SAFE_FUNCTIONS is defined, then we can use the _r
> > > functions. The problem is that with the patch below, the code is broken
> > > when using threaded httpd on FreeBSD. How does 3.4 provide a thread-safe
> > > version of getpwnam?
> >
> > This is FreeBSD land where POSIX does not matter. =)
> >
> > Wes Peters wrote 1.5 years ago[1] that he might implement
> > getpwnam_r and related functions, but I don't see them in
> > the -current tree.
>
> Well that's bogus. Does that mean that FreeBSD doesn't do threadsafe
> getpwnam? I have no good answer for this. I guess we check if getpwnam_r
> is available, but this is just wrong.
Well, it is a standard problem of writing portable reentrant
code.
Our "user" code like mod_userdir.c should just assume that
getpwnam_r exists. If it does not exist, we can provide a
fallback which serializes access to the OS-function.
Otherwise, we start duplicating work-arounds and checks all
over the place.
I'm attaching such a fallback for getpwnam_r. There are two
issues with this:
- ACQUIRE_LOCK/RELEASE_LOCK should be defined to mutex-like
functions, if we are compiling thread-safe (otherwise,
they are nops).
- The getpwnam_r function should be prefixed properly to
avoid conflicts with system functions which have the same
name, but another interface.
The locks probably require a small framework for
initialization like we do here:
http://lxr.php.net/source/php4/main/reentrancy.c
Comments?
- Sascha
Re: [?PATCH?] using getpwnam_r in mod_userdir
Posted by rb...@covalent.net.
> >
> > Argh. :-( The general rule that we used in APR was that if
> > _POSIX_THREAD_SAFE_FUNCTIONS is defined, then we can use the _r
> > functions. The problem is that with the patch below, the code is broken
> > when using threaded httpd on FreeBSD. How does 3.4 provide a thread-safe
> > version of getpwnam?
>
> This is FreeBSD land where POSIX does not matter. =)
>
> Wes Peters wrote 1.5 years ago[1] that he might implement
> getpwnam_r and related functions, but I don't see them in
> the -current tree.
Well that's bogus. Does that mean that FreeBSD doesn't do threadsafe
getpwnam? I have no good answer for this. I guess we check if getpwnam_r
is available, but this is just wrong.
Ryan
_______________________________________________________________________________
Ryan Bloom rbb@apache.org
406 29th St.
San Francisco, CA 94131
-------------------------------------------------------------------------------
Re: [?PATCH?] using getpwnam_r in mod_userdir
Posted by Sascha Schumann <sa...@schumann.cx>.
On Fri, 10 Nov 2000 rbb@covalent.net wrote:
>
> Argh. :-( The general rule that we used in APR was that if
> _POSIX_THREAD_SAFE_FUNCTIONS is defined, then we can use the _r
> functions. The problem is that with the patch below, the code is broken
> when using threaded httpd on FreeBSD. How does 3.4 provide a thread-safe
> version of getpwnam?
This is FreeBSD land where POSIX does not matter. =)
Wes Peters wrote 1.5 years ago[1] that he might implement
getpwnam_r and related functions, but I don't see them in
the -current tree.
[1] http://docs.freebsd.org/cgi/getmsg.cgi?fetch=482572+0+archive/1999/freebsd-hackers/19990214.freebsd-hackers
- Sascha
Re: [?PATCH?] using getpwnam_r in mod_userdir
Posted by rb...@covalent.net.
Argh. :-( The general rule that we used in APR was that if
_POSIX_THREAD_SAFE_FUNCTIONS is defined, then we can use the _r
functions. The problem is that with the patch below, the code is broken
when using threaded httpd on FreeBSD. How does 3.4 provide a thread-safe
version of getpwnam?
Ryan
On Fri, 10 Nov 2000, Jeff Trawick wrote:
> (Sorry about tossing the commit message prematurely.)
>
> In practice, _POSIX_THREAD_SAFE_FUNCTIONS doesn't imply the existence
> of getpwnam_r(). Example: FreeBSD 3.4
>
> We need to check if we actually have the function...
>
> This change works on FreeBSD 3.4 and Mandrake 7.0, but I'm concerned
> that it may not work everywhere... If getpwnam_r() is in a special
> library (e.g., libpthread), we won't know to link against the library
> while we're doing module configuration. APR figures out that stuff
> for us, and we aren't that far yet.
>
> some possibilities:
>
> . punt this section of code to APR by creating an apr_get_home_dir()
> function somewhere (but maybe not appropriate on Win32?)
>
> . use the code below until it breaks, then figure out what to do
>
> . have APR export APR_HAVE_GETPWNAM_R to the app via apr.h
>
> cvs server: Diffing modules/standard
> Index: modules/standard/config.m4
> ===================================================================
> RCS file: /cvs/apache/apache-2.0/src/modules/standard/config.m4,v
> retrieving revision 1.30
> diff -u -r1.30 config.m4
> --- config.m4 2000/10/24 11:45:41 1.30
> +++ config.m4 2000/11/10 20:02:33
> @@ -25,7 +25,9 @@
> APACHE_CHECK_STANDARD_MODULE(imap, internal imagemaps, , yes)
> APACHE_CHECK_STANDARD_MODULE(actions, Action triggering on requests, action, yes)
> APACHE_CHECK_STANDARD_MODULE(speling, correct common URL misspellings, , no)
> -APACHE_CHECK_STANDARD_MODULE(userdir, mapping of user requests, , yes)
> +APACHE_CHECK_STANDARD_MODULE(userdir, mapping of user requests, , yes, [
> + AC_CHECK_FUNCS(getpwnam_r)
> +])
> APACHE_CHECK_STANDARD_MODULE(suexec, set uid and gid for spawned processes, , no)
> APACHE_CHECK_STANDARD_MODULE(alias, translation of requests, , yes)
>
> Index: modules/standard/mod_userdir.c
> ===================================================================
> RCS file: /cvs/apache/apache-2.0/src/modules/standard/mod_userdir.c,v
> retrieving revision 1.25
> diff -u -r1.25 mod_userdir.c
> --- mod_userdir.c 2000/11/10 18:18:57 1.25
> +++ mod_userdir.c 2000/11/10 20:02:33
> @@ -326,7 +326,7 @@
> #else /* WIN32 */
> struct passwd *pw;
>
> -#if APR_HAS_THREADS && defined(_POSIX_THREAD_SAFE_FUNCTIONS)
> +#if APR_HAS_THREADS && defined(HAVE_GETPWNAM_R)
> struct passwd pwd;
> size_t buflen = sysconf(_SC_GETPW_R_SIZE_MAX);
> char *buf = apr_pcalloc(r->pool, buflen);
>
> --
> Jeff Trawick | trawick@ibm.net | PGP public key at web site:
> http://www.geocities.com/SiliconValley/Park/9289/
> Born in Roswell... married an alien...
>
>
_______________________________________________________________________________
Ryan Bloom rbb@apache.org
406 29th St.
San Francisco, CA 94131
-------------------------------------------------------------------------------