Re: SSL toolkit detection in acinclude.m4
-Ursprüngliche Nachricht- Von: Plüm, Rüdiger, VF-Group Gesendet: Dienstag, 4. November 2008 13:24 An: dev@httpd.apache.org Betreff: Re: SSL toolkit detection in acinclude.m4 -Ursprüngliche Nachricht- Von: Ruediger Pluem Gesendet: Samstag, 25. Oktober 2008 15:18 An: dev@httpd.apache.org Betreff: Re: SSL toolkit detection in acinclude.m4 On 10/16/2008 12:04 PM, Rainer Jung wrote: Ruediger Pluem wrote: While investigating PR46018 I came across the following in acinclude.m4 dnl Run header and version checks saved_CPPFLAGS=$CPPFLAGS saved_LIBS=$LIBS if test x$ap_ssltk_base != x; then APR_ADDTO(CPPFLAGS, [-I$ap_ssltk_base/include]) APR_ADDTO(INCLUDES, [-I$ap_ssltk_base/include]) APR_ADDTO(LDFLAGS, [-L$ap_ssltk_base/lib]) Is there any reason why we only save / restore CPPFLAGS and LIBS and not INCLUDES and LDFLAGS which are also modified? The next thing I don't get is why we do not add `$apr_config --libs` to LIBS any longer (since r669924). This causes -ldl to miss and thus the checks to fail (I guess only if there is no pkgconfig file). Yes, I ran into a similar issue, when compiling 2.2.10 against static openssl libs on Solaris, having no pkgconfig. Then you need to add -ldl, -lnsl, -lsocket during the configure tests, because otherwise tehy fail. That was not necessary until 2.2.9. Any further comments, ideas how to fix this? I added a possible fix to the PR (https://issues.apache.org/bugzilla/attachment.cgi?id=22826). Does somebody mind checking if this is the correct thing to do? Rainer does the patch fix your issues on Solaris? Regards Rüdiger
Re: SSL toolkit detection in acinclude.m4
Plüm, Rüdiger, VF-Group schrieb: -Ursprüngliche Nachricht- Von: Plüm, Rüdiger, VF-Group Gesendet: Dienstag, 4. November 2008 13:24 An: dev@httpd.apache.org Betreff: Re: SSL toolkit detection in acinclude.m4 -Ursprüngliche Nachricht- Von: Ruediger Pluem Gesendet: Samstag, 25. Oktober 2008 15:18 An: dev@httpd.apache.org Betreff: Re: SSL toolkit detection in acinclude.m4 On 10/16/2008 12:04 PM, Rainer Jung wrote: Ruediger Pluem wrote: While investigating PR46018 I came across the following in acinclude.m4 dnl Run header and version checks saved_CPPFLAGS=$CPPFLAGS saved_LIBS=$LIBS if test x$ap_ssltk_base != x; then APR_ADDTO(CPPFLAGS, [-I$ap_ssltk_base/include]) APR_ADDTO(INCLUDES, [-I$ap_ssltk_base/include]) APR_ADDTO(LDFLAGS, [-L$ap_ssltk_base/lib]) Is there any reason why we only save / restore CPPFLAGS and LIBS and not INCLUDES and LDFLAGS which are also modified? The next thing I don't get is why we do not add `$apr_config --libs` to LIBS any longer (since r669924). This causes -ldl to miss and thus the checks to fail (I guess only if there is no pkgconfig file). Yes, I ran into a similar issue, when compiling 2.2.10 against static openssl libs on Solaris, having no pkgconfig. Then you need to add -ldl, -lnsl, -lsocket during the configure tests, because otherwise tehy fail. That was not necessary until 2.2.9. Any further comments, ideas how to fix this? I added a possible fix to the PR (https://issues.apache.org/bugzilla/attachment.cgi?id=22826). Does somebody mind checking if this is the correct thing to do? Rainer does the patch fix your issues on Solaris? I'm in New Orleans. Will be able to test remotely but only later. Will report back in 24 hours. Regards, Rainer
Re: SSL toolkit detection in acinclude.m4
Plüm, Rüdiger, VF-Group schrieb: On 10/16/2008 12:04 PM, Rainer Jung wrote: Ruediger Pluem wrote: While investigating PR46018 I came across the following in acinclude.m4 dnl Run header and version checks saved_CPPFLAGS=$CPPFLAGS saved_LIBS=$LIBS if test x$ap_ssltk_base != x; then APR_ADDTO(CPPFLAGS, [-I$ap_ssltk_base/include]) APR_ADDTO(INCLUDES, [-I$ap_ssltk_base/include]) APR_ADDTO(LDFLAGS, [-L$ap_ssltk_base/lib]) Is there any reason why we only save / restore CPPFLAGS and LIBS and not INCLUDES and LDFLAGS which are also modified? The next thing I don't get is why we do not add `$apr_config --libs` to LIBS any longer (since r669924). This causes -ldl to miss and thus the checks to fail (I guess only if there is no pkgconfig file). Yes, I ran into a similar issue, when compiling 2.2.10 against static openssl libs on Solaris, having no pkgconfig. Then you need to add -ldl, -lnsl, -lsocket during the configure tests, because otherwise tehy fail. That was not necessary until 2.2.9. Any further comments, ideas how to fix this? I added a possible fix to the PR (https://issues.apache.org/bugzilla/attachment.cgi?id=22826). Does somebody mind checking if this is the correct thing to do? Rainer does the patch fix your issues on Solaris? The additional `$apr_config --libs` seems fine to fix the issue during configure itself. Saving INCLUDES and LDFLAGS break the build. Without saving, entries are added to EXTRA_INCLUDES and EXTRA_LDFLAGS in build/config_vars.mk. Those entries are needed to build ab and mod_ssl and to link them. For the linking issue one could instead of adding the SSL-specifix -L... to EXTRA_LDFLAGS add them to ab_LDADD and to MOD_SSL_LDADD: it seems not to be needed for anything else. Since both of those vars include SSL_LIBS, it could also go in there. The include flag is harder, since at the moment the build system does ot include a component specific include variable (like ab_INCADD or MOD_SSL_INCADD). So now we know, why INCLUDES and LDFLAGS where not saved in configure ;) I'll test once more without the save-related parts of the patch and let you know. Regards, Rainer
Re: SSL toolkit detection in acinclude.m4
Rainer Jung schrieb: Plüm, Rüdiger, VF-Group schrieb: Rainer does the patch fix your issues on Solaris? I'll test once more without the save-related parts of the patch and let you know. I now removed the changes for INCLUDES and LDFLAGS and only kept the additional `$apr_config --libs`. That works fine. Regards, Rainer
Re: SSL toolkit detection in acinclude.m4
On 11/06/2008 05:50 PM, Rainer Jung wrote: Rainer Jung schrieb: Plüm, Rüdiger, VF-Group schrieb: Rainer does the patch fix your issues on Solaris? I'll test once more without the save-related parts of the patch and let you know. I now removed the changes for INCLUDES and LDFLAGS and only kept the additional `$apr_config --libs`. That works fine. Thanks for testing Rainer. Just for clarification: In fact you used the following patch in your successful test. Index: acinclude.m4 === --- acinclude.m4(Revision 711951) +++ acinclude.m4(Arbeitskopie) @@ -447,10 +447,10 @@ pkglookup=`$PKGCONFIG --libs-only-L --libs-only-other openssl` APR_ADDTO(LDFLAGS, [$pkglookup]) else -ap_ssltk_libs=-lssl -lcrypto +ap_ssltk_libs=-lssl -lcrypto `$apr_config --libs` fi else - ap_ssltk_libs=-lssl -lcrypto + ap_ssltk_libs=-lssl -lcrypto `$apr_config --libs` fi fi APR_SETVAR(SSL_LIBS, [$ap_ssltk_libs]) Correct? Regards Rüdiger
Re: SSL toolkit detection in acinclude.m4
Ruediger Pluem schrieb: On 11/06/2008 05:50 PM, Rainer Jung wrote: Rainer Jung schrieb: Plüm, Rüdiger, VF-Group schrieb: Rainer does the patch fix your issues on Solaris? I'll test once more without the save-related parts of the patch and let you know. I now removed the changes for INCLUDES and LDFLAGS and only kept the additional `$apr_config --libs`. That works fine. Thanks for testing Rainer. Just for clarification: In fact you used the following patch in your successful test. Index: acinclude.m4 === --- acinclude.m4(Revision 711951) +++ acinclude.m4(Arbeitskopie) @@ -447,10 +447,10 @@ pkglookup=`$PKGCONFIG --libs-only-L --libs-only-other openssl` APR_ADDTO(LDFLAGS, [$pkglookup]) else -ap_ssltk_libs=-lssl -lcrypto +ap_ssltk_libs=-lssl -lcrypto `$apr_config --libs` fi else - ap_ssltk_libs=-lssl -lcrypto + ap_ssltk_libs=-lssl -lcrypto `$apr_config --libs` fi fi APR_SETVAR(SSL_LIBS, [$ap_ssltk_libs]) Correct? Exactly, should have included it in my response directly. Sorry.
Re: SSL toolkit detection in acinclude.m4
On 11/06/2008 08:57 PM, Rainer Jung wrote: Ruediger Pluem schrieb: On 11/06/2008 05:50 PM, Rainer Jung wrote: Rainer Jung schrieb: Plüm, Rüdiger, VF-Group schrieb: Rainer does the patch fix your issues on Solaris? I'll test once more without the save-related parts of the patch and let you know. I now removed the changes for INCLUDES and LDFLAGS and only kept the additional `$apr_config --libs`. That works fine. Thanks for testing Rainer. Just for clarification: In fact you used the following patch in your successful test. Index: acinclude.m4 === --- acinclude.m4(Revision 711951) +++ acinclude.m4(Arbeitskopie) @@ -447,10 +447,10 @@ pkglookup=`$PKGCONFIG --libs-only-L --libs-only-other openssl` APR_ADDTO(LDFLAGS, [$pkglookup]) else -ap_ssltk_libs=-lssl -lcrypto +ap_ssltk_libs=-lssl -lcrypto `$apr_config --libs` fi else - ap_ssltk_libs=-lssl -lcrypto + ap_ssltk_libs=-lssl -lcrypto `$apr_config --libs` fi fi APR_SETVAR(SSL_LIBS, [$ap_ssltk_libs]) Correct? Exactly, should have included it in my response directly. Sorry. No problem. I updated PR46018 (https://issues.apache.org/bugzilla/show_bug.cgi?id=46018) and will wait some time for feedback. After that I will commit. Regards Rüdiger
Re: svn commit: r711886 - /httpd/httpd/branches/2.2.x/STATUS
On 11/06/2008 05:07 PM, [EMAIL PROTECTED] wrote:
Author: jim
Date: Thu Nov 6 08:06:54 2008
New Revision: 711886
URL: http://svn.apache.org/viewvc?rev=711886view=rev
Log:
Hold off until I learn more from Ruediger
Modified:
httpd/httpd/branches/2.2.x/STATUS
Modified: httpd/httpd/branches/2.2.x/STATUS
URL:
http://svn.apache.org/viewvc/httpd/httpd/branches/2.2.x/STATUS?rev=711886r1=711885r2=711886view=diff
==
--- httpd/httpd/branches/2.2.x/STATUS (original)
+++ httpd/httpd/branches/2.2.x/STATUS Thu Nov 6 08:06:54 2008
@@ -135,7 +135,7 @@
http://svn.apache.org/viewvc?rev=706921view=rev
Backport version for 2.2.x of patch:
Trunk version of patch works
- +1: jim
+ +1:
rpluem: This code still has bad edge cases that lead to segfaults. I
have
a better version in the pipe that currently undergoes testing and will
be in trunk soon.
After analysing some crashes on one of my systems I came to the conclusion that
the
current approach to solve the problem still has open edge cases of which some
IMHO
could not be fixed this way. Furthermore the already complex flushing code
would become
even more complex and harder to understand. So I chose a new approach to solve
the problem.
What is the problem at all?
mod_proxy_http uses a a conn_rec to communicate with the backend. It somehow
reverses
the meaning of input and output filters and uses them to send the request and
receive
the response. In order to use persistent SSL connections to the backend it is
needed
to keep this conn_rec (and thus its bucket allocator) alive and tied to the
backend
connection. Thus it is no longer destroyed at the end of a client request /
connection.
So the conn_rec and especially the bucket allocator for the backend have a
lifecycle
that is disjoint from the one to the client. Especially it can happen that due
to e.g.
a faulty backend connection the conn_rec to the backend *lives* shorter than
buckets that are still buffered somewhere in the output filter chain to the
client
(see comments in removed code below). This causes segfaults when these buckets
are
accessed then or if brigades that contain them get cleaned up later on due to a
parent
pool cleanup.
The new approach to solve the problem is to transform the lifetime of the
buckets
by shifting them to a different allocator. The basic approach is to iterate over
the brigade fetched from the backend connection and
for metabuckets recreate them
for data buckets read them and put the read data in a transient bucket.
Currently only HEAP, TRANSIENT, POOL and IMMORTAL data buckets are expected
from the backend.
In this case apr_bucket_read is a trivial operation that only returns a pointer
to the
data stored in the bucket. Once we pass the buckets down the chain filters are
responsible
to set the buckets aside if they do not consume them in this filter pass.
Setting aside
a transient bucket causes the data to be moved into a freshly allocated memory
area that
is allocated from the bucket allocator, in this case the correct one as it is
the one from
the connection to the client. This way effort intensive memory copying only
happens, when
really needed. A big plus of this approach is that it opens the door again for
returning
the backend connection back to the connection pool as soon as we have received
all data
from the backend instead of the need to wait until all data is flushed to the
client.
I guess this will make some people happy here :-).
Well here we go:
Index: modules/proxy/proxy_util.c
===
--- modules/proxy/proxy_util.c (revision 711224)
+++ modules/proxy/proxy_util.c (working copy)
@@ -1628,9 +1628,6 @@
{
proxy_conn_rec *conn = (proxy_conn_rec *)theconn;
proxy_worker *worker = conn-worker;
-apr_bucket_brigade *bb;
-conn_rec *c;
-request_rec *r;
/*
* If the connection pool is NULL the worker
@@ -1651,112 +1648,6 @@
}
#endif
-r = conn-r;
-if (conn-need_flush r) {
-request_rec *main_request;
-
-/*
- * We need to get to the main request as subrequests do not count any
- * sent bytes.
- */
-main_request = r;
-while (main_request-main) {
-main_request = main_request-main;
-}
-if (main_request-bytes_sent || r-eos_sent) {
-ap_filter_t *flush_filter;
-
-/*
- * We need to ensure that buckets that may have been buffered in
the
- * network filters get flushed to the network. This is needed since
- * these buckets have been created with the bucket allocator of the
- * backend connection. This allocator either gets destroyed if
- * conn-close is set or the worker address is not reusable which
- * causes the
