On Wed, Feb 18, 2009 at 09:39:31PM +0100, Ruediger Pluem wrote:
> On 02/18/2009 11:16 AM, Joe Orton wrote:
> There is still a nasty issue with the trunk code that can cause you to
> run out of FD's as the new non blocking core output filter has some trouble
> setting aside the file buckets to the correct pool (it puts them in the
> connection pool which is too long living and bad).
> 
> See thread starting at
> 
> http://mail-archives.apache.org/mod_mbox/httpd-dev/200901.mbox/%3c495ff1b9.9070...@kippdata.de%3e
> 
> I admit that I lost momentum on this, but this definitely needs to be fixed.

Yes, that looks broken.

Something like the patch below should fix it, though I'm not sure it's 
really a good idea to be allowing a pathological case of 16 fds consumed 
per client.  It might present some interesting tuning challenges; 
hitting fd limits before hitting num(client) limits.

Index: core_filters.c
===================================================================
--- core_filters.c      (revision 734690)
+++ core_filters.c      (working copy)
@@ -367,6 +367,7 @@
 
 #define THRESHOLD_MIN_WRITE 4096
 #define THRESHOLD_MAX_BUFFER 65536
+#define THRESHOLD_MAX_FILES 16
 
 /* Optional function coming from mod_logio, used for logging of output
  * traffic
@@ -380,7 +381,7 @@
     core_output_filter_ctx_t *ctx = net->out_ctx;
     apr_bucket_brigade *bb;
     apr_bucket *bucket, *next;
-    apr_size_t bytes_in_brigade, non_file_bytes_in_brigade;
+    apr_size_t bytes_in_brigade, non_file_bytes_in_brigade, 
num_files_in_brigade;
 
     /* Fail quickly if the connection has already been aborted. */
     if (c->aborted) {
@@ -466,6 +467,8 @@
 
     bytes_in_brigade = 0;
     non_file_bytes_in_brigade = 0;
+    num_files_in_brigade = 0;
+
     for (bucket = APR_BRIGADE_FIRST(bb); bucket != APR_BRIGADE_SENTINEL(bb);
          bucket = next) {
         next = APR_BUCKET_NEXT(bucket);
@@ -497,13 +500,17 @@
                 next = APR_BUCKET_NEXT(bucket);
             }
             bytes_in_brigade += bucket->length;
-            if (!APR_BUCKET_IS_FILE(bucket)) {
+            if (APR_BUCKET_IS_FILE(bucket)) {
+                num_files_in_brigade++;
+            } 
+            else {                
                 non_file_bytes_in_brigade += bucket->length;
             }
         }
     }
 
-    if (non_file_bytes_in_brigade >= THRESHOLD_MAX_BUFFER) {
+    if (non_file_bytes_in_brigade >= THRESHOLD_MAX_BUFFER
+        || num_files_in_brigade >= THRESHOLD_MAX_FILES) {
         /* ### Writing the entire brigade may be excessive; we really just
          * ### need to send enough data to be under THRESHOLD_MAX_BUFFER.
          */

Reply via email to