perl-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Steve Hay" <Steve...@planit.com>
Subject [PATCH] Stop httpd crashing when user presses Stop in browser
Date Tue, 17 May 2011 15:27:29 GMT
I've long been having problems reported from several customers of their
Apache httpd.exe process aborting with exit code 2 from time to time,
with a suspicion that the cause is users pressing Stop in their web
browsers, and I think the patch below (also attached) may fix the
problem.

The software is running as a perl-script handler via a
ModPerl::RegistryCooker subclass with error_check() overridden to simply
log any connection aborted/reset errors and otherwise print out a short
HTML page describing  the error. The HTML obviously can't be output if
the connection is lost, which was the purpose of SVN rev. 1052232. That
change has been working well, with various "Apache2 IO write" errors
written to the error logs from time to time.

However, the error logs also contain some "Apache2 IO flush" errors,
always immediately before the occasional crash that is being seen, and I
*think* what is happening is that an I/O flush is happening late in the
game, after the protecting evals have been exited, and therefore causes
httpd to crash if the flush operation happens to croak.

This patch uses a macro introduced long ago in SVN rev. 202146 to
prevent filter flushing from croaking on a lost connection to similarly
inspect the error that has occurred and simply log it if it is a lost
connection, and only throw an exception otherwise.

The software that I'm running is using simple CORE::print() calls, and
hence goes through PerlIOApache_flush() so I've changed the wrapper on
the modperl_wbucket_flush() call in there, and I've also changed the
wrapper on another call in mpxs_Apache2__RequestRec_rflush() for the
benefit of code using $r->print().

Any thoughts on whether this change is good to commit?

It seems like a sensible change to me, given that the Perl code is
unable to catch the exception if it does get thrown (unlike that thrown
from the write() function), although it would also be nice to fix
whatever is the real problem.

Unfortunately, I have not been able to reproduce the problems reported
from our customers (although I have seen the errors in their log files,
so it really is happening). If I break in PerlIOApache_flush() then I
find that rcfg->wbucket->outcnt is 0 every time it calls
modperl_wbucket_flush(), which therefore never fails since the
add_flush_bucket parameter is passed as FALSE. This implies that
rcfg->wbucket->outcnt must be non-zero sometimes on customers' sites,
but I'm unable to reproduce that, even pressing Stop while it is in the
midst of print()ing stuff from a CGI script.

Index: src/modules/perl/modperl_filter.c
===================================================================
--- src/modules/perl/modperl_filter.c	(revision 1104242)
+++ src/modules/perl/modperl_filter.c	(working copy)
@@ -472,24 +472,6 @@
     return status;
 }
 
-
-#define MP_RUN_CROAK_RESET_OK(func)
\
-    {
\
-        apr_status_t rc = func(filter);
\
-        if (rc != APR_SUCCESS) {
\
-            if (APR_STATUS_IS_ECONNRESET(rc) ||
\
-                APR_STATUS_IS_ECONNABORTED(rc)) {
\
-                ap_log_error(APLOG_MARK, APLOG_INFO, 0, s,
\
-                             "Apache2::Filter internal flush got: %s",
\
-                             modperl_error_strerror(aTHX_ rc));
\
-            }
\
-            else {
\
-                modperl_croak(aTHX_ rc,
\
-                              "Apache2::Filter internal flush");
\
-            }
\
-        }
\
-    }
-
 int modperl_run_filter(modperl_filter_t *filter)
 {
     AV *args = Nullav;
@@ -563,10 +545,12 @@
             apr_brigade_destroy(filter->bb_in);
             filter->bb_in = NULL;
         }
-        MP_RUN_CROAK_RESET_OK(modperl_input_filter_flush);
+        MP_RUN_CROAK_RESET_OK(s, modperl_input_filter_flush(filter),
+                              "Apache2::Filter internal flush");
     }
     else {
-        MP_RUN_CROAK_RESET_OK(modperl_output_filter_flush);
+        MP_RUN_CROAK_RESET_OK(s, modperl_output_filter_flush(filter),
+                              "Apache2::Filter internal flush");
     }
 
     MP_FILTER_RESTORE_ERRSV(errsv);
Index: src/modules/perl/modperl_error.h
===================================================================
--- src/modules/perl/modperl_error.h	(revision 1104242)
+++ src/modules/perl/modperl_error.h	(working copy)
@@ -45,4 +45,20 @@
         }                                                    \
     } STMT_END
 
+#define MP_RUN_CROAK_RESET_OK(s, rc_run, func) STMT_START
\
+    {
\
+        apr_status_t rc = rc_run;
\
+        if (rc != APR_SUCCESS) {
\
+            if (APR_STATUS_IS_ECONNRESET(rc) ||
\
+                APR_STATUS_IS_ECONNABORTED(rc)) {
\
+                ap_log_error(APLOG_MARK, APLOG_INFO, 0, s,
\
+                             "%s got: %s", func,
\
+                             modperl_error_strerror(aTHX_ rc));
\
+            }
\
+            else {
\
+                modperl_croak(aTHX_ rc, func);
\
+            }
\
+        }
\
+    } STMT_END
+
 #endif /* MODPERL_ERROR_H */
Index: src/modules/perl/modperl_io_apache.c
===================================================================
--- src/modules/perl/modperl_io_apache.c	(revision 1104242)
+++ src/modules/perl/modperl_io_apache.c	(working copy)
@@ -169,8 +169,9 @@
                                   rcfg->wbucket->outbuf,
                                   rcfg->wbucket->outcnt));
 
-    MP_RUN_CROAK(modperl_wbucket_flush(rcfg->wbucket, FALSE),
-                 ":Apache2 IO flush");
+    MP_RUN_CROAK_RESET_OK(st->r->server,
+                          modperl_wbucket_flush(rcfg->wbucket, FALSE),
+                          ":Apache2 IO flush");
 
     return 0;
 }
Index: xs/Apache2/RequestIO/Apache2__RequestIO.h
===================================================================
--- xs/Apache2/RequestIO/Apache2__RequestIO.h	(revision 1104242)
+++ xs/Apache2/RequestIO/Apache2__RequestIO.h	(working copy)
@@ -179,8 +179,9 @@
                rcfg->wbucket->outcnt,
                apr_pstrmemdup(rcfg->wbucket->pool,
rcfg->wbucket->outbuf,
                               rcfg->wbucket->outcnt));
-    MP_RUN_CROAK(modperl_wbucket_flush(rcfg->wbucket, TRUE),
-                 "Apache2::RequestIO::rflush");
+    MP_RUN_CROAK_RESET_OK(r->server,
+                          modperl_wbucket_flush(rcfg->wbucket, TRUE),
+                          "Apache2::RequestIO::rflush");
 }
 
 static MP_INLINE long mpxs_ap_get_client_block(pTHX_ request_rec *r,


Mime
View raw message