You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@apr.apache.org by Nick Kew <ni...@webthing.com> on 2005/08/20 13:29:56 UTC
Reducing #ifdef burden on APR applications
When writing APR applications, we sometimes need to make heavy
use of APR #ifdefs.
A case in point is mod_dbd, which uses either apr_reslist or a
single persistent connection according to whether APR_HAS_THREADS.
As an APR application, this would be greatly simplified if
apr_reslist fell back to a trivial implementation of a single
resource in the non-threaded case, sparing the application
the burden of dealing with it.
There are a number of APR modules that could do this, to the
benefit of applications. A trivial case to illustrate is
apr_thread_mutex, where the change is merely:
Index: include/apr_thread_mutex.h
===================================================================
--- include/apr_thread_mutex.h (revision 233161)
+++ include/apr_thread_mutex.h (working copy)
@@ -99,6 +99,13 @@
*/
APR_POOL_DECLARE_ACCESSOR(thread_mutex);
+#else /* APR_HAS_THREADS */
+/* Stubs for unthreaded APR, so applications don't have to use #ifdef */
+#define apr_thread_mutex_create(mutex,flags,pool)
+#define apr_thread_mutex_lock(mutex)
+#define apr_thread_mutex_trylock(mutex)
+#define apr_thread_mutex_unlock(mutex)
+#define apr_thread_mutex_destroy(mutex)
#endif /* APR_HAS_THREADS */
/** @} */
Anyone object to this kind of update?
--
Nick Kew
Re: Reducing #ifdef burden on APR applications
Posted by Philip Martin <ph...@codematters.co.uk>.
Jeff Trawick <tr...@gmail.com> writes:
> Solaris will link in APR_SUCCESS-style no-op pthread functions if your
> application isn't built thread-capable.
Glibc on Linux does the same for things like pthread_mutex_lock:
$ cat z.c
#include <pthread.h>
int main()
{
if (pthread_mutex_lock(NULL))
return 1;
return 0;
}
$ gcc z.c # build without threads
$ ./a.out
$ echo $?
0
$ gcc -pthread z.c # with threads
$ ./a.out
Segmentation fault
> If APR always provides such APIs and acts like they work, what is to
> signal to a threaded APR app that they are picking up a non-threaded
> libapr?
Glibc doesn't provide no-op functions for things like
pthread_mutex_create, those are only available when the full threading
library replaces all the no-op functions.
--
Philip Martin
Re: Reducing #ifdef burden on APR applications
Posted by Nick Kew <ni...@webthing.com>.
On Monday 22 August 2005 12:09, Jeff Trawick wrote:
> maybe do the APR_SUCCESS no-op flavor and give the app a way to query
> if certain features are enabled; so an app could fail at startup if
> there is no threads support but it doesn't want to run with such a
> build of APR; querying feature set is independently useful
Indeed, I had contemplated that, and it's what I'd favour if this were the
design phase of a new product. I wasn't proposing it now because it's
not clear how to do it without some risk of breaking existing apps -
if any would break now on unconditional-SUCCESS.
--
Nick Kew
Re: Reducing #ifdef burden on APR applications
Posted by "William A. Rowe, Jr." <wr...@rowe-clan.net>.
At 02:30 PM 8/23/2005, Justin Erenkrantz wrote:
>... The application can then decide what to do in the presence of the APR_ENOTIMPL case - in this case, we can document that apr_thread_* foo can return APR_ENOTIMPL if the underlying OS doesn't support it. As Jeff said, we let the app (or library) decide what to do from there. -- justin
Yes, as long as we all understand this is an apr version
major bump (e.g., 2.0). A behavior change is equivalent
to breaking the ABI.
Bill
Re: Reducing #ifdef burden on APR applications
Posted by Nick Kew <ni...@webthing.com>.
On Wednesday 24 August 2005 11:49, Joe Orton wrote:
> > No, I still disagree. The fundamental point is that it leaves us using
> > ENOTIMPL in a manner that is both success (it's a no-op) and failure
> > (it's not implemented because noone got round to it yet). I want my
> > applications to know when they're dealing with an error!
>
> Nick, I think the disconnect here is that you are talking specifically
> about making thread mutex locking functions (and maybe condvars, rwlocks
> etc?) return APR_SUCCESS on error, right?
Indeed, it's only a small subset of ops for which NOOP is in fact
equivalent to success. Areas like thread create or run, reslist, etc
are either fully implemented or it's an error for which ENOTIMPL
is entirely appropriate.
> The argument being that in absence of threads, the locking across
> threads can legitimately do nothing and return SUCCESS since there are
> no threads to lock between. This seems quite tempting, but I do find
> Jeff's horror story pretty scary w.r.t. the Solaris equivalent.
I remain to be convinced about that, but I accept we might like a distinction
- hence my suggestion of APR_NOOP for such cases.
But we now also have Philip Martin's point that returning SUCCESS is in
line with glibc on Linux. Evidently this is common practice in real life!
> But for the general case, e.g. apr_thread_create() and really
> apr_thread_* in apr_thread_proc.h, it would just be a gross API
> violation to have the functions return SUCCESS for a !threads build, do
> you agree?
Of course! No argument about that.
--
Nick Kew
Re: Reducing #ifdef burden on APR applications
Posted by Joe Orton <jo...@redhat.com>.
On Wed, Aug 24, 2005 at 11:26:11AM +0100, Nick Kew wrote:
> Justin Erenkrantz wrote:
> >--On August 22, 2005 1:35:48 PM +0100 Nick Kew <ni...@webthing.com> wrote:
> >
> >>I disagree. For example, APR_EOF is an end-condition, not an error.
> >>apr_errno even defines different ranges of codes for different things.
> >>
> >>In practical terms, suppose I implement APR for Platform X, but leave
> >>some parts (which my application doesn't happen to need)
> >>unimplemented, returning APR_ENOTIMPL. Now if we've used APR_ENOTIMPL
> >>for a no-op success, *all apps* that use the feature have lost the
> >>distinction between success and a failure that can't be ignored.
> >
> >
> >Not true. The application can then decide what to do in the presence of
> >the APR_ENOTIMPL case - in this case, we can document that apr_thread_*
> >foo can return APR_ENOTIMPL if the underlying OS doesn't support it.
>
> No, I still disagree. The fundamental point is that it leaves us using
> ENOTIMPL in a manner that is both success (it's a no-op) and failure
> (it's not implemented because noone got round to it yet). I want my
> applications to know when they're dealing with an error!
Nick, I think the disconnect here is that you are talking specifically
about making thread mutex locking functions (and maybe condvars, rwlocks
etc?) return APR_SUCCESS on error, right?
The argument being that in absence of threads, the locking across
threads can legitimately do nothing and return SUCCESS since there are
no threads to lock between. This seems quite tempting, but I do find
Jeff's horror story pretty scary w.r.t. the Solaris equivalent.
But for the general case, e.g. apr_thread_create() and really
apr_thread_* in apr_thread_proc.h, it would just be a gross API
violation to have the functions return SUCCESS for a !threads build, do
you agree?
Regards,
joe
Re: Reducing #ifdef burden on APR applications
Posted by Nick Kew <ni...@webthing.com>.
Justin Erenkrantz wrote:
> --On August 22, 2005 1:35:48 PM +0100 Nick Kew <ni...@webthing.com> wrote:
>
>> I disagree. For example, APR_EOF is an end-condition, not an error.
>> apr_errno even defines different ranges of codes for different things.
>>
>> In practical terms, suppose I implement APR for Platform X, but leave
>> some parts (which my application doesn't happen to need)
>> unimplemented, returning APR_ENOTIMPL. Now if we've used APR_ENOTIMPL
>> for a no-op success, *all apps* that use the feature have lost the
>> distinction between success and a failure that can't be ignored.
>
>
> Not true. The application can then decide what to do in the presence of
> the APR_ENOTIMPL case - in this case, we can document that apr_thread_*
> foo can return APR_ENOTIMPL if the underlying OS doesn't support it.
No, I still disagree. The fundamental point is that it leaves us using
ENOTIMPL in a manner that is both success (it's a no-op) and failure
(it's not implemented because noone got round to it yet). I want my
applications to know when they're dealing with an error!
--
Nick Kew
Re: Reducing #ifdef burden on APR applications
Posted by Justin Erenkrantz <ju...@erenkrantz.com>.
--On August 22, 2005 1:35:48 PM +0100 Nick Kew <ni...@webthing.com> wrote:
> I disagree. For example, APR_EOF is an end-condition, not an error.
> apr_errno even defines different ranges of codes for different things.
>
> In practical terms, suppose I implement APR for Platform X, but leave
> some parts (which my application doesn't happen to need)
> unimplemented, returning APR_ENOTIMPL. Now if we've used APR_ENOTIMPL
> for a no-op success, *all apps* that use the feature have lost the
> distinction between success and a failure that can't be ignored.
Not true. The application can then decide what to do in the presence of
the APR_ENOTIMPL case - in this case, we can document that apr_thread_* foo
can return APR_ENOTIMPL if the underlying OS doesn't support it. As Jeff
said, we let the app (or library) decide what to do from there. -- justin
Re: Reducing #ifdef burden on APR applications
Posted by Nick Kew <ni...@webthing.com>.
On Monday 22 August 2005 13:12, Joe Orton wrote:
> > Erm, because ENOTIMPL is an error, whereas this noop isn't?
>
> Now I'm confused; all apr_status_t values other than 0 are "errors" by
> definition. If you mean, "because the caller can be taught to treat
> APR_NOOP as non-fatal", well, sure; the caller can equally be taught to
> treat APR_ENOTIMPL as non-fatal.
I disagree. For example, APR_EOF is an end-condition, not an error.
apr_errno even defines different ranges of codes for different things.
In practical terms, suppose I implement APR for Platform X, but leave
some parts (which my application doesn't happen to need)
unimplemented, returning APR_ENOTIMPL. Now if we've used APR_ENOTIMPL
for a no-op success, *all apps* that use the feature have lost the distinction
between success and a failure that can't be ignored.
--
Nick Kew
Re: Reducing #ifdef burden on APR applications
Posted by "William A. Rowe, Jr." <wr...@rowe-clan.net>.
At 07:00 AM 8/22/2005, Nick Kew wrote:
>On Monday 22 August 2005 12:13, Joe Orton wrote:
>>
>> What's really needed is a feature-detection API along with the ENOTIMPL
>> stubs.
>>
>> if (apr_has_feature(APR_FEATURE_THREADS))
>> /* create mutex */
FYI that only tells you that APR SUPPORTS threads(!) It's almost
always the wrong decision, e.g. in httpd - we have ap_mpm_query()
which tells us if the MPM is actually threading.
Bill
Re: Reducing #ifdef burden on APR applications
Posted by Joe Orton <jo...@redhat.com>.
On Mon, Aug 22, 2005 at 01:00:13PM +0100, Nick Kew wrote:
> On Monday 22 August 2005 12:13, Joe Orton wrote:
>
> > > OK, I guess what we really want here is an additional return code
> > > APR_NOOP. NOTIMPL is inappropriate, and you've raised an objection to
> > > SUCCESS.
> >
> > Why is ENOTIMPL inappropriate?
>
> Erm, because ENOTIMPL is an error, whereas this noop isn't?
Now I'm confused; all apr_status_t values other than 0 are "errors" by
definition. If you mean, "because the caller can be taught to treat
APR_NOOP as non-fatal", well, sure; the caller can equally be taught to
treat APR_ENOTIMPL as non-fatal.
joe
Re: Reducing #ifdef burden on APR applications
Posted by Nick Kew <ni...@webthing.com>.
On Monday 22 August 2005 12:13, Joe Orton wrote:
> > OK, I guess what we really want here is an additional return code
> > APR_NOOP. NOTIMPL is inappropriate, and you've raised an objection to
> > SUCCESS.
>
> Why is ENOTIMPL inappropriate?
Erm, because ENOTIMPL is an error, whereas this noop isn't?
> The function is not implemented for the
> non-threaded build. If you want to be able to use these functions
> unconditionally then you have to check for SUCCESS or (something) and
> (something) may as well be APR_ENOTIMPL as some new code.
>
> What's really needed is a feature-detection API along with the ENOTIMPL
> stubs.
>
> if (apr_has_feature(APR_FEATURE_THREADS))
> /* create mutex */
That would also work, but it's more complexity, and looks a bit like
just shifting the #ifdefs away from the precompiler. I prefer Jeff's
suggestion of tying that kind of thing into startup. Or APR_NOOP.
--
Nick Kew
Re: Reducing #ifdef burden on APR applications
Posted by Joe Orton <jo...@redhat.com>.
On Mon, Aug 22, 2005 at 11:43:12AM +0100, Nick Kew wrote:
> On Saturday 20 August 2005 23:33, Jeff Trawick wrote:
> > On 8/20/05, Nick Kew <ni...@webthing.com> wrote:
> > > Jeff Trawick wrote:
> > > > If APR always provides such APIs and acts like they work, what is to
> > > > signal to a threaded APR app that they are picking up a non-threaded
> > > > libapr?
> > >
> > > With reference to anything in particular? Surely you wouldn't use
> > > apr_thread_mutex unless you were using apr_threads? Doesn't that
> > > principle extend to other applications?
> >
> > perhaps I'm a library that needs to be thread-safe, and I'm called by
> > a threaded app but find a non-thread-capable apr at load time? (too
> > far fetched)
>
> OK, I guess what we really want here is an additional return code APR_NOOP.
> NOTIMPL is inappropriate, and you've raised an objection to SUCCESS.
Why is ENOTIMPL inappropriate? The function is not implemented for the
non-threaded build. If you want to be able to use these functions
unconditionally then you have to check for SUCCESS or (something) and
(something) may as well be APR_ENOTIMPL as some new code.
What's really needed is a feature-detection API along with the ENOTIMPL
stubs.
if (apr_has_feature(APR_FEATURE_THREADS))
/* create mutex */
though there will be some subset of the API for which run-time detection
is infeasible e.g. apr_portable.h, IPv6-ness.
joe
Re: Reducing #ifdef burden on APR applications
Posted by Jeff Trawick <tr...@gmail.com>.
On 8/22/05, Nick Kew <ni...@webthing.com> wrote:
> On Saturday 20 August 2005 23:33, Jeff Trawick wrote:
> > On 8/20/05, Nick Kew <ni...@webthing.com> wrote:
> > > Jeff Trawick wrote:
> > > > If APR always provides such APIs and acts like they work, what is to
> > > > signal to a threaded APR app that they are picking up a non-threaded
> > > > libapr?
> > >
> > > With reference to anything in particular? Surely you wouldn't use
> > > apr_thread_mutex unless you were using apr_threads? Doesn't that
> > > principle extend to other applications?
> >
> > perhaps I'm a library that needs to be thread-safe, and I'm called by
> > a threaded app but find a non-thread-capable apr at load time? (too
> > far fetched)
>
> OK, I guess what we really want here is an additional return code APR_NOOP.
> NOTIMPL is inappropriate, and you've raised an objection to SUCCESS.
>
> Of course that'll complicate the normal check-for-APR_SUCCESS code:-(
maybe do the APR_SUCCESS no-op flavor and give the app a way to query
if certain features are enabled; so an app could fail at startup if
there is no threads support but it doesn't want to run with such a
build of APR; querying feature set is independently useful
Re: Reducing #ifdef burden on APR applications
Posted by Nick Kew <ni...@webthing.com>.
On Saturday 20 August 2005 23:33, Jeff Trawick wrote:
> On 8/20/05, Nick Kew <ni...@webthing.com> wrote:
> > Jeff Trawick wrote:
> > > If APR always provides such APIs and acts like they work, what is to
> > > signal to a threaded APR app that they are picking up a non-threaded
> > > libapr?
> >
> > With reference to anything in particular? Surely you wouldn't use
> > apr_thread_mutex unless you were using apr_threads? Doesn't that
> > principle extend to other applications?
>
> perhaps I'm a library that needs to be thread-safe, and I'm called by
> a threaded app but find a non-thread-capable apr at load time? (too
> far fetched)
OK, I guess what we really want here is an additional return code APR_NOOP.
NOTIMPL is inappropriate, and you've raised an objection to SUCCESS.
Of course that'll complicate the normal check-for-APR_SUCCESS code:-(
--
Nick Kew
Re: Reducing #ifdef burden on APR applications
Posted by Jeff Trawick <tr...@gmail.com>.
On 8/20/05, Nick Kew <ni...@webthing.com> wrote:
> Jeff Trawick wrote:
> > If APR always provides such APIs and acts like they work, what is to
> > signal to a threaded APR app that they are picking up a non-threaded
> > libapr?
>
> With reference to anything in particular? Surely you wouldn't use
> apr_thread_mutex unless you were using apr_threads? Doesn't that
> principle extend to other applications?
perhaps I'm a library that needs to be thread-safe, and I'm called by
a threaded app but find a non-thread-capable apr at load time? (too
far fetched)
okay ;)
Re: Reducing #ifdef burden on APR applications
Posted by Nick Kew <ni...@webthing.com>.
Jeff Trawick wrote:
> apr_reslist seems like a good candidate to do the right thing when
> !APR_HAS_THREADS.
Indeed. Now where's my box of round tuits?
> If APR always provides such APIs and acts like they work, what is to
> signal to a threaded APR app that they are picking up a non-threaded
> libapr?
With reference to anything in particular? Surely you wouldn't use
apr_thread_mutex unless you were using apr_threads? Doesn't that
principle extend to other applications?
Your Solaris/Apache13 story is a different scenario where two
executables (apache and the module) with different and incompatible
assumptions were linked. In APR terms, it sounds more like linking
a threaded and an unthreaded APR into the same application.
--
Nick Kew
Re: Reducing #ifdef burden on APR applications
Posted by Jeff Trawick <tr...@gmail.com>.
On 8/20/05, Nick Kew <ni...@webthing.com> wrote:
> Joe Orton wrote:
> > On Sat, Aug 20, 2005 at 12:29:56PM +0100, Nick Kew wrote:
> >
> >>When writing APR applications, we sometimes need to make heavy
> >>use of APR #ifdefs.
> >>
> >>A case in point is mod_dbd, which uses either apr_reslist or a
> >>single persistent connection according to whether APR_HAS_THREADS.
> >>As an APR application, this would be greatly simplified if
> >>apr_reslist fell back to a trivial implementation of a single
> >>resource in the non-threaded case, sparing the application
> >>the burden of dealing with it.
apr_reslist seems like a good candidate to do the right thing when
!APR_HAS_THREADS.
> > and define
> > ENOTIMPL stubs for everything in the !THREADS case.
>
> I think that needs to be APR_SUCCESS stubs for a null operation.
> That's what apps test against, and there's no error condition!
FWIW, here's a situation which I shall remember as long as I can
remember my middle name:
Solaris will link in APR_SUCCESS-style no-op pthread functions if your
application isn't built thread-capable. Apache 1.3 was linked like
this for a while. Some module code which actually used pthread
functions was loaded and failed in bizarre ways. I couldn't believe
that Solaris would do such a hokey thing. After much discussion
between a security module vendor and a European bank and myself and a
fortuitous discovery by a dev@httpd lurker, it was resolved.
If APR always provides such APIs and acts like they work, what is to
signal to a threaded APR app that they are picking up a non-threaded
libapr?
Re: Reducing #ifdef burden on APR applications
Posted by Nick Kew <ni...@webthing.com>.
Joe Orton wrote:
> On Sat, Aug 20, 2005 at 12:29:56PM +0100, Nick Kew wrote:
>
>>When writing APR applications, we sometimes need to make heavy
>>use of APR #ifdefs.
>>
>>A case in point is mod_dbd, which uses either apr_reslist or a
>>single persistent connection according to whether APR_HAS_THREADS.
>>As an APR application, this would be greatly simplified if
>>apr_reslist fell back to a trivial implementation of a single
>>resource in the non-threaded case, sparing the application
>>the burden of dealing with it.
>
>
> I think it would be far far better to make APR ABI-compatible not just
> API-compatible across APR_HAS_THREADS/!APR_HAS_THREADS
Hmmm, fair point.
> and define
> ENOTIMPL stubs for everything in the !THREADS case.
I think that needs to be APR_SUCCESS stubs for a null operation.
That's what apps test against, and there's no error condition!
Anyway, having *public API* functions *conditionally* defined in the
APR headers is a booby trap for apps, and should be killed off.
--
Nick Kew
Re: Reducing #ifdef burden on APR applications
Posted by Joe Orton <jo...@redhat.com>.
On Sat, Aug 20, 2005 at 12:29:56PM +0100, Nick Kew wrote:
> When writing APR applications, we sometimes need to make heavy
> use of APR #ifdefs.
>
> A case in point is mod_dbd, which uses either apr_reslist or a
> single persistent connection according to whether APR_HAS_THREADS.
> As an APR application, this would be greatly simplified if
> apr_reslist fell back to a trivial implementation of a single
> resource in the non-threaded case, sparing the application
> the burden of dealing with it.
I think it would be far far better to make APR ABI-compatible not just
API-compatible across APR_HAS_THREADS/!APR_HAS_THREADS and define
ENOTIMPL stubs for everything in the !THREADS case.
joe