Where any response is sent, return OK from the handler. Where there

  is no response (but a status code) return the code.  This patch adds
  a great number of debugging emits for failed ap_pass_brigade calls,
  to help diagnose failure cases, and disambiguates OK from APR_SUCCESS.

PR: 40470
Submitted by: wrowe, Matt Eaton <asf divinehawk.com>

git-svn-id: https://svn.apache.org/repos/asf/httpd/httpd/trunk@492333 13f79535-47bb-0310-9956-ffa450edef68

diff --git a/modules/arch/win32/mod_isapi.c b/modules/arch/win32/mod_isapi.c
index 21a28947c5fb024d34140c6421905f393060ad2f..87ff7a5cb76352428b0dd422c5b5c86134459210 100644 (file)
--- a/modules/arch/win32/mod_isapi.c
+++ b/modules/arch/win32/mod_isapi.c
@@ -815,7 +815,7 @@ int APR_THREAD_FUNC WriteClient(isapi_cid    *cid,
     char *buf_data = (char*)buf_ptr;
     apr_bucket_brigade *bb;
     apr_bucket *b;
-    apr_status_t rv;
+    apr_status_t rv = APR_SUCCESS;
 
     if (!cid->headers_set) {
         /* It appears that the foxisapi module and other clients
@@ -841,10 +841,14 @@ int APR_THREAD_FUNC WriteClient(isapi_cid    *cid,
         APR_BRIGADE_INSERT_TAIL(bb, b);
         rv = ap_pass_brigade(r->output_filters, bb);
         cid->response_sent = 1;
+        if (rv != APR_SUCCESS)
+            ap_log_rerror(APLOG_MARK, APLOG_DEBUG, rv, r,
+                          "ISAPI: WriteClient ap_pass_brigade "
+                          "failed: %s", r->filename);
     }
 
     if ((flags & HSE_IO_ASYNC) && cid->completion) {
-        if (rv == OK) {
+        if (rv == APR_SUCCESS) {
             cid->completion(cid->ecb, cid->completion_arg,
                             *size_arg, ERROR_SUCCESS);
         }
@@ -928,6 +932,11 @@ int APR_THREAD_FUNC ServerSupportFunction(isapi_cid    *cid,
             APR_BRIGADE_INSERT_TAIL(bb, b);
             rv = ap_pass_brigade(cid->r->output_filters, bb);
             cid->response_sent = 1;
+            if (rv != APR_SUCCESS)
+                ap_log_rerror(APLOG_MARK, APLOG_DEBUG, rv, r,
+                              "ISAPI: ServerSupport function "
+                              "HSE_REQ_SEND_RESPONSE_HEADER "
+                              "ap_pass_brigade failed: %s", r->filename);
             return (rv == APR_SUCCESS);
         }
         /* Deliberately hold off sending 'just the headers' to begin to
@@ -1125,13 +1134,18 @@ int APR_THREAD_FUNC ServerSupportFunction(isapi_cid    *cid,
         APR_BRIGADE_INSERT_TAIL(bb, b);
         rv = ap_pass_brigade(r->output_filters, bb);
         cid->response_sent = 1;
+        if (rv != APR_SUCCESS)
+            ap_log_rerror(APLOG_MARK, APLOG_DEBUG, rv, r,
+                          "ISAPI: ServerSupport function "
+                          "HSE_REQ_TRANSMIT_FILE "
+                          "ap_pass_brigade failed: %s", r->filename);
 
         /* Use tf->pfnHseIO + tf->pContext, or if NULL, then use cid->fnIOComplete
          * pass pContect to the HseIO callback.
          */
         if (tf->dwFlags & HSE_IO_ASYNC) {
             if (tf->pfnHseIO) {
-                if (rv == OK) {
+                if (rv == APR_SUCCESS) {
                     tf->pfnHseIO(cid->ecb, tf->pContext,
                                  ERROR_SUCCESS, sent);
                 }
@@ -1141,7 +1155,7 @@ int APR_THREAD_FUNC ServerSupportFunction(isapi_cid    *cid,
                 }
             }
             else if (cid->completion) {
-                if (rv == OK) {
+                if (rv == APR_SUCCESS) {
                     cid->completion(cid->ecb, cid->completion_arg,
                                     sent, ERROR_SUCCESS);
                 }
@@ -1191,6 +1205,14 @@ int APR_THREAD_FUNC ServerSupportFunction(isapi_cid    *cid,
         }
 
         if ((*data_type & HSE_IO_ASYNC) && cid->completion) {
+            /* XXX: Many authors issue their next HSE_REQ_ASYNC_READ_CLIENT
+             * within the completion logic.  An example is MS's own PSDK
+             * sample web/iis/extensions/io/ASyncRead.  This potentially
+             * leads to stack exhaustion.  To refactor, the notification 
+             * logic needs to move to isapi_handler() - differentiating
+             * the cid->completed event with a new flag to indicate 
+             * an async-notice versus the async request completed.
+             */
             if (res >= 0) {
                 cid->completion(cid->ecb, cid->completion_arg,
                                 read, ERROR_SUCCESS);
@@ -1325,6 +1347,11 @@ int APR_THREAD_FUNC ServerSupportFunction(isapi_cid    *cid,
             APR_BRIGADE_INSERT_TAIL(bb, b);
             rv = ap_pass_brigade(cid->r->output_filters, bb);
             cid->response_sent = 1;
+            if (rv != APR_SUCCESS)
+                ap_log_rerror(APLOG_MARK, APLOG_DEBUG, rv, r,
+                              "ISAPI: ServerSupport function "
+                              "HSE_REQ_SEND_RESPONSE_HEADER_EX "
+                              "ap_pass_brigade failed: %s", r->filename);
             return (rv == APR_SUCCESS);
         }
         /* Deliberately hold off sending 'just the headers' to begin to
@@ -1600,21 +1627,23 @@ apr_status_t isapi_handler (request_rec *r)
         case HSE_STATUS_ERROR:
             /* end response if we have yet to do so.
              */
+            ap_log_rerror(APLOG_MARK, APLOG_WARNING, apr_get_os_error(), r,
+                          "ISAPI: HSE_STATUS_ERROR result from "
+                          "HttpExtensionProc(): %s", r->filename);
             r->status = HTTP_INTERNAL_SERVER_ERROR;
             break;
 
         default:
-            /* TODO: log unrecognized retval for debugging
-             */
-             ap_log_rerror(APLOG_MARK, APLOG_WARNING, 0, r,
-                           "ISAPI: return code %d from HttpExtensionProc() "
-                           "was not not recognized", rv);
+            ap_log_rerror(APLOG_MARK, APLOG_WARNING, apr_get_os_error(), r,
+                          "ISAPI: unrecognized result code %d "
+                          "from HttpExtensionProc(): %s ", 
+                          rv, r->filename);
             r->status = HTTP_INTERNAL_SERVER_ERROR;
             break;
     }
 
     /* Flush the response now, including headers-only responses */
-    if (cid->headers_set) {
+    if (cid->headers_set || cid->response_sent) {
         conn_rec *c = r->connection;
         apr_bucket_brigade *bb;
         apr_bucket *b;
@@ -1623,13 +1652,16 @@ apr_status_t isapi_handler (request_rec *r)
         bb = apr_brigade_create(r->pool, c->bucket_alloc);
         b = apr_bucket_eos_create(c->bucket_alloc);
         APR_BRIGADE_INSERT_TAIL(bb, b);
-        b = apr_bucket_flush_create(c->bucket_alloc);
-        APR_BRIGADE_INSERT_TAIL(bb, b);
         rv = ap_pass_brigade(r->output_filters, bb);
         cid->response_sent = 1;
 
-        return (rv == APR_SUCCESS);
-        /* NOT r->status or cid->r->status, even if it has changed. */
+        if (rv != APR_SUCCESS) {
+            ap_log_rerror(APLOG_MARK, APLOG_DEBUG, rv, r,
+                          "ISAPI: ap_pass_brigade failed to "
+                          "complete the response: %s ", r->filename);
+        }
+
+        return OK; /* NOT r->status, even if it has changed. */
     }
 
     /* As the client returned no error, and if we did not error out
@@ -1639,8 +1671,8 @@ apr_status_t isapi_handler (request_rec *r)
         r->status = cid->ecb->dwHttpStatusCode;
     }
 
-    /* For all missing-response situations simply return the status.
-     * and let the core deal respond to the client.
+    /* For all missing-response situations simply return the status,
+     * and let the core respond to the client.
      */
     return r->status;
 }