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