httpd-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From c...@decus.org (Rodent of Unusual Size)
Subject [PATCH] Complete fix for mod_example memory bloat
Date Sat, 03 May 1997 02:43:36 GMT
    The attached patch primarily addresses the issue of child memory
    loss if mod_example is compiled into the server.  (The cause was
    ever-growing trace records for all directory walks.)  To sum it up,

     o trace information is now broken into calls made in "request
       context" and those that aren't; the former are traced in the
       request pool
     o the tracer keeps a record of routine/environment combinations for
       which it's been called, and won't trace them more than once
       (non-request-context calls only)

    One thing that's different is that I'm storing the per-request stuff
    in the r->request_config list, which appears to be only minimally
    supported.  I haven't found anything in the archives (yet) that
    indicates this is discouraged or deprecated.

    My tests indicate that the bloat seems to be cured by this.  Brian,
    maybe you could check me on this?

    #ken    :-P}

Index: mod_example.c
===================================================================
RCS file: /export/home/cvs/apache/src/modules/example/mod_example.c,v
retrieving revision 1.8
diff -c -r1.8 mod_example.c
*** mod_example.c	1997/04/22 15:52:06	1.8
--- mod_example.c	1997/05/03 02:39:59
***************
*** 99,107 ****
  /*
   * Let's set up a module-local static cell to point to the accreting callback
   * trace.  As each API callback is made to us, we'll tack on the particulars
!  * to whatever we've already recorded.
   */
  static char *trace = NULL;
  
  /*
   * To avoid leaking memory from pools other than the per-request one, we
--- 99,111 ----
  /*
   * Let's set up a module-local static cell to point to the accreting callback
   * trace.  As each API callback is made to us, we'll tack on the particulars
!  * to whatever we've already recorded.  To avoid massive memory bloat as
!  * directories are walked again and again, we record the routine/environment
!  * the first time (non-request context only), and ignore subsequent calls for
!  * the same routine/environment.
   */
  static char *trace = NULL;
+ static table *static_calls_made = NULL;
  
  /*
   * To avoid leaking memory from pools other than the per-request one, we
***************
*** 261,296 ****
  }
  
  /*
!  * This routine is used to add a trace of a callback to the list.  We're
!  * passed the server record (if available), an allocation pool, a pointer to
!  * our private configuration record (if available) for the environment to
!  * which the callback is supposed to apply, and some text.  We turn this into
!  * a textual representation and add it to the tail of the list.  The list can
!  * be displayed by the example_handler() routine.
   */
! static void trace_add
! 	(server_rec *s, example_config *mconfig, const char *note) {
  
!     char    *sofar;
!     char    *addon;
!     char    *where;
!     pool    *subpool;
  
      /*
!      * Make a new sub-pool and copy any existing trace to it.
       */
!     subpool = make_sub_pool (example_pool);
!     if (trace != NULL) {
! 	addon = pstrcat (subpool, trace, NULL);
!     }
      /*
!      * Now, if we have a sub-pool from before, nuke it and replace with the
!      * one we just allocated.
       */
!     if (example_subpool != NULL) {
! 	destroy_pool (example_subpool);
      }
-     example_subpool = subpool;
      /*
       * If we weren't passed a configuration record, we can't figure out to
       * what location this call applies.  This only happens for co-routines
--- 265,388 ----
  }
  
  /*
!  * Likewise for our configuration record for the specified request.
   */
! static example_config *our_rconfig
! 	(request_rec *r) {
  
!     return (example_config *) get_module_config
! 				(
! 				    r->request_config,
! 				    &example_module
! 				);
! }
  
+ /*
+  * This routine sets up some module-wide cells if they haven't been already.
+  */
+ static void setup_module_cells () {
      /*
!      * If we haven't already allocated our module-private pool, do so now.
       */
!     if (example_pool == NULL) {
! 	example_pool = make_sub_pool (NULL);
!     };
      /*
!      * Likewise for the table of routine/environment pairs we visit outside of
!      * request context.
       */
!     if (static_calls_made == NULL) {
! 	static_calls_made = make_table (example_pool, 16);
!     };
! }
! 
! /*
!  * This routine is used to add a trace of a callback to the list.  We're
!  * passed the server record (if available), the request record (if available),
!  * a pointer to our private configuration record (if available) for the
!  * environment to which the callback is supposed to apply, and some text.  We
!  * turn this into a textual representation and add it to the tail of the list.
!  * The list can be displayed by the example_handler() routine.
!  *
!  * If the call occurs within a request context (i.e., we're passed a request
!  * record), we put the trace into the request pool and attach it to the
!  * request via the request_config mechanism.  Otherwise, the trace gets added
!  * to the static (non-request-specific)  list.
!  */
! static void trace_add
! 	(server_rec *s, request_rec *r, example_config *mconfig,
! 	 const char *note) {
! 
!     char    *sofar;
!     char    *addon;
!     char    *where;
!     pool    *p;
!     char    **ctrace;
!     request_rec
! 	    *req = r;
!     example_config
! 	    *rconfig;
! 
!     /*
!      * Make sure our pools and tables are set up - we need 'em.
!      */
!     setup_module_cells ();
!     /*
!      * Now, if we're in request-context, we use the request pool.
!      */
!     if (r != NULL) {
! 	/*
! 	 * Make sure that we're storing our trace in the main request; we want
! 	 * to track all activities for subrequests as well.
! 	 */
! 	if (req->main != NULL) {
! 	    req = req->main;
! 	}
! 	p = req->pool;
! 	/*
! 	 * Find our per-request configuration record, which holds the trace
! 	 * of activities specific to the request  itself.
! 	 */
! 	rconfig = (example_config *) get_module_config
! 					(req->request_config, &example_module);
! 	/*
! 	 * If there isn't one, create one and add it.
! 	 */
! 	if (rconfig == NULL) {
! 	    rconfig = pcalloc (req->pool, sizeof(example_config));
! 	    set_module_config (req->request_config, &example_module, rconfig);
! 	    rconfig->trace = NULL;
! 	}
! 	/*
! 	 * Note that the trace data from this call go into the per-request
! 	 * list, not the static one.
! 	 */
! 	 ctrace = &rconfig->trace;
!     } else {
! 	/*
! 	 * We're not in request context, so the trace gets attached to our
! 	 * module-wide pool.  We do the create/destroy every time we're called
! 	 * in non-request context; this avoids leaking memory in some of
! 	 * the subsequent calls that allocate memory only once (such as the
! 	 * key formation below).
! 	 *
! 	 * Make a new sub-pool and copy any existing trace to it.  Point the
! 	 * trace cell at the copied value.
! 	 */
! 	p = make_sub_pool (example_pool);
! 	if (trace != NULL) {
! 	    trace = pstrdup (p, trace);
! 	}
! 	/*
! 	 * Now, if we have a sub-pool from before, nuke it and replace with
! 	 * the one we just allocated.
! 	 */
! 	if (example_subpool != NULL) {
! 	    destroy_pool (example_subpool);
! 	}
! 	example_subpool = p;
! 	ctrace = &trace;
      }
      /*
       * If we weren't passed a configuration record, we can't figure out to
       * what location this call applies.  This only happens for co-routines
***************
*** 300,308 ****
       */
      where = (mconfig != NULL) ? mconfig->loc : "nowhere";
      where = (where != NULL) ? where : "";
      addon = pstrcat 
  		(
! 		    subpool,
  		    "   <LI>\n",
  		    "    <DL>\n",
  		    "     <DT><SAMP>",
--- 392,422 ----
       */
      where = (mconfig != NULL) ? mconfig->loc : "nowhere";
      where = (where != NULL) ? where : "";
+     /*
+      * Now, if we're not in request context, see if we've been called with
+      * this particular combination before.  The table is allocated in the
+      * module's private pool, which doesn't get destroyed.
+      */
+     if (req == NULL) {
+ 	char	*key;
+ 
+ 	key = pstrcat (p, note, ":", where, NULL);
+ 	if (table_get (static_calls_made, key) != NULL) {
+ 	    /*
+ 	     * Been here, done this.
+ 	     */
+ 	    return;
+ 	} else {
+ 	    /*
+ 	     * First time for this combination of routine and environment -
+ 	     * log it so we don't do it again.
+ 	     */
+ 	    table_set (static_calls_made, key, "been here");
+ 	}
+     }
      addon = pstrcat 
  		(
! 		    p,
  		    "   <LI>\n",
  		    "    <DL>\n",
  		    "     <DT><SAMP>",
***************
*** 317,332 ****
  		    "   </LI>\n",
  		    NULL
  		);
!     sofar = (trace == NULL) ? "" : trace;
!     trace = pstrcat (subpool, sofar, addon, NULL);
!     /*
!      * Store a copy of the same information in the configuration record, if
!      * there is one.
!      */
!     if (mconfig != NULL) {
! 	sofar = (mconfig->trace == NULL) ? "" : mconfig->trace;
! 	mconfig->trace = pstrcat (subpool, sofar, addon, NULL);
!     }
      /*
       * You *could* uncomment the following if you wanted to see the calling
       * sequence reported in the server's error_log, but beware - almost all of
--- 431,438 ----
  		    "   </LI>\n",
  		    NULL
  		);
!     sofar = (*ctrace == NULL) ? "" : *ctrace;
!     *ctrace = pstrcat (p, sofar, addon, NULL);
      /*
       * You *could* uncomment the following if you wanted to see the calling
       * sequence reported in the server's error_log, but beware - almost all of
***************
*** 341,347 ****
  }
  
  /*--------------------------------------------------------------------------*/
- /*									    */
  /* We prototyped the various syntax for command handlers (routines that     */
  /* are called when the configuration parser detects a directive declared    */
  /* by our module) earlier.  Now we actually declare a "real" routine that   */
--- 447,452 ----
***************
*** 351,356 ****
--- 456,468 ----
  /* If a command handler encounters a problem processing the directive, it   */
  /* signals this fact by returning a non-NULL pointer to a string	    */
  /* describing the problem.						    */
+ /*									    */
+ /* The magic return value DECLINE_CMD is used to deal with directives	    */
+ /* that might be declared by multiple modules.  If the command handler	    */
+ /* returns NULL, the directive was processed; if it returns DECLINE_CMD,    */
+ /* the next module (if any) that declares the directive is given a chance   */
+ /* at it.  If it returns any other value, it's treated as the text of an    */
+ /* error message.							    */
  /*--------------------------------------------------------------------------*/
  /* 
   * Command handler for the NO_ARGS "Example" directive.  All we do is mark the
***************
*** 367,373 ****
       * "Example Wuz Here"
       */
      cfg->local = 1;
!     trace_add (cmd->server, cfg, "cmd_example()");
      return NULL;
  }
  
--- 479,485 ----
       * "Example Wuz Here"
       */
      cfg->local = 1;
!     trace_add (cmd->server, NULL, cfg, "cmd_example()");
      return NULL;
  }
  
***************
*** 399,408 ****
  	(request_rec *r) {
  
      example_config
! 	    *cfg;
  
!     cfg = our_dconfig (r);
!     trace_add (r->server, cfg, "example_handler()");
      /*
       * We're about to start sending content, so we need to force the HTTP
       * headers to be sent at this point.  Otherwise, no headers will be sent
--- 511,523 ----
  	(request_rec *r) {
  
      example_config
! 	    *dcfg;
!     example_config
! 	    *rcfg;
  
!     dcfg = our_dconfig (r);
!     rcfg = our_rconfig (r);
!     trace_add (r->server, r, dcfg, "example_handler()");
      /*
       * We're about to start sending content, so we need to force the HTTP
       * headers to be sent at this point.  Otherwise, no headers will be sent
***************
*** 461,481 ****
      rputs ("  indicates a location in the URL or filesystem\n", r);
      rputs ("  namespace.\n", r);
      rputs ("  </P>\n", r);
!     rprintf (r, "  <H2>Callbacks so far:</H2>\n  <OL>\n%s  </OL>\n",
trace);
      rputs ("  <H2>Environment for <EM>this</EM> call:</H2>\n", r);
      rputs ("  <UL>\n", r);
!     rprintf (r, "   <LI>Applies-to: <SAMP>%s</SAMP>\n   </LI>\n",
cfg->loc);
      rprintf
  	(
  	    r,
  	    "   <LI>\"Example\" directive declared here: %s\n   </LI>\n",
! 	    (cfg->local ? "YES" : "NO")
  	);
      rprintf
  	(
  	    r,
  	    "   <LI>\"Example\" inherited: %s\n   </LI>\n",
! 	    (cfg->congenital ? "YES" : "NO")
  	);
      rputs ("  </UL>\n", r);
      rputs (" </BODY>\n", r);
--- 576,607 ----
      rputs ("  indicates a location in the URL or filesystem\n", r);
      rputs ("  namespace.\n", r);
      rputs ("  </P>\n", r);
!     rprintf
! 	(
! 	    r,
! 	    "  <H2>Static callbacks so far:</H2>\n  <OL>\n%s  </OL>\n",
! 	    trace
! 	);
!     rprintf
! 	(
! 	    r,
! 	    "  <H2>Request-specific callbacks so far:</H2>\n  <OL>\n%s  </OL>\n",
! 	    rcfg->trace
! 	);
      rputs ("  <H2>Environment for <EM>this</EM> call:</H2>\n", r);
      rputs ("  <UL>\n", r);
!     rprintf (r, "   <LI>Applies-to: <SAMP>%s</SAMP>\n   </LI>\n",
dcfg->loc);
      rprintf
  	(
  	    r,
  	    "   <LI>\"Example\" directive declared here: %s\n   </LI>\n",
! 	    (dcfg->local ? "YES" : "NO")
  	);
      rprintf
  	(
  	    r,
  	    "   <LI>\"Example\" inherited: %s\n   </LI>\n",
! 	    (dcfg->congenital ? "YES" : "NO")
  	);
      rputs ("  </UL>\n", r);
      rputs (" </BODY>\n", r);
***************
*** 541,558 ****
      char    *sname = s->server_hostname;
  
      /*
!      * If we haven't already allocated our module-private pool, do so now.
       */
!     if (example_pool == NULL) {
! 	example_pool = make_sub_pool (NULL);
!     };
      /*
       * The arbitrary text we add to our trace entry indicates for which server
       * we're being called.
       */
      sname = (sname != NULL) ? sname : "";
      note = pstrcat (p, "example_init(", sname, ")", NULL);
!     trace_add (s, NULL, note);
  }
  
  /*
--- 667,682 ----
      char    *sname = s->server_hostname;
  
      /*
!      * Set up any module cells that ought to be initialised.
       */
!     setup_module_cells ();
      /*
       * The arbitrary text we add to our trace entry indicates for which server
       * we're being called.
       */
      sname = (sname != NULL) ? sname : "";
      note = pstrcat (p, "example_init(", sname, ")", NULL);
!     trace_add (s, NULL, NULL, note);
  }
  
  /*
***************
*** 590,596 ****
       */
      dname = (dname != NULL) ? dname : "";
      cfg->loc = pstrcat (p, "DIR(", dname, ")", NULL);
!     trace_add (NULL, cfg, "example_dir_create()");
      return (void *) cfg;
  }
  
--- 714,720 ----
       */
      dname = (dname != NULL) ? dname : "";
      cfg->loc = pstrcat (p, "DIR(", dname, ")", NULL);
!     trace_add (NULL, NULL, cfg, "example_dir_create()");
      return (void *) cfg;
  }
  
***************
*** 654,660 ****
  		"\")",
  		NULL
  	    );
!     trace_add (NULL, merged_config, note);
      return (void *) merged_config;
  }
  
--- 778,784 ----
  		"\")",
  		NULL
  	    );
!     trace_add (NULL, NULL, merged_config, note);
      return (void *) merged_config;
  }
  
***************
*** 685,691 ****
       */
      sname = (sname != NULL) ? sname : "";
      cfg->loc = pstrcat (p, "SVR(", sname, ")", NULL);
!     trace_add (s, cfg, "example_server_create()");
      return (void *) cfg;
  }
  
--- 809,815 ----
       */
      sname = (sname != NULL) ? sname : "";
      cfg->loc = pstrcat (p, "SVR(", sname, ")", NULL);
!     trace_add (s, NULL, cfg, "example_server_create()");
      return (void *) cfg;
  }
  
***************
*** 736,742 ****
  		"\")",
  		NULL
  	    );
!     trace_add (NULL, merged_config, note);
      return (void *) merged_config;
  }
  
--- 860,866 ----
  		"\")",
  		NULL
  	    );
!     trace_add (NULL, NULL, merged_config, note);
      return (void *) merged_config;
  }
  
***************
*** 759,765 ****
       * We don't actually *do* anything here, except note the fact that we were
       * called.
       */
!     trace_add (r->server, cfg, "example_xlate()");
      return DECLINED;
  }
  
--- 883,889 ----
       * We don't actually *do* anything here, except note the fact that we were
       * called.
       */
!     trace_add (r->server, r, cfg, "example_xlate()");
      return DECLINED;
  }
  
***************
*** 782,788 ****
      /*
       * Don't do anything except log the call.
       */
!     trace_add (r->server, cfg, "example_ckuser()");
      return DECLINED;
  }
  
--- 906,912 ----
      /*
       * Don't do anything except log the call.
       */
!     trace_add (r->server, r, cfg, "example_ckuser()");
      return DECLINED;
  }
  
***************
*** 807,813 ****
       * Log the call and return OK, or access will be denied (even though we
       * didn't actually do anything).
       */
!     trace_add (r->server, cfg, "example_ckauth()");
      return OK;
  }
  
--- 931,937 ----
       * Log the call and return OK, or access will be denied (even though we
       * didn't actually do anything).
       */
!     trace_add (r->server, r, cfg, "example_ckauth()");
      return OK;
  }
  
***************
*** 827,833 ****
  	    *cfg;
  
      cfg = our_dconfig (r);
!     trace_add (r->server, cfg, "example_ckaccess()");
      return OK;
  }
  
--- 951,957 ----
  	    *cfg;
  
      cfg = our_dconfig (r);
!     trace_add (r->server, r, cfg, "example_ckaccess()");
      return OK;
  }
  
***************
*** 850,856 ****
       * Log the call, but don't do anything else - and report truthfully that
       * we didn't do anything.
       */
!     trace_add (r->server, cfg, "example_typer()");
      return DECLINED;
  }
  
--- 974,980 ----
       * Log the call, but don't do anything else - and report truthfully that
       * we didn't do anything.
       */
!     trace_add (r->server, r, cfg, "example_typer()");
      return DECLINED;
  }
  
***************
*** 872,878 ****
      /*
       * Log the call and exit.
       */
!     trace_add (r->server, cfg, "example_fixer()");
      return OK;
  }
  
--- 996,1002 ----
      /*
       * Log the call and exit.
       */
!     trace_add (r->server, r, cfg, "example_fixer()");
      return OK;
  }
  
***************
*** 890,896 ****
  	    *cfg;
  
      cfg = our_dconfig (r);
!     trace_add (r->server, cfg, "example_logger()");
      return DECLINED;
  }
  
--- 1014,1020 ----
  	    *cfg;
  
      cfg = our_dconfig (r);
!     trace_add (r->server, r, cfg, "example_logger()");
      return DECLINED;
  }
  
***************
*** 909,915 ****
  	    *cfg;
  
      cfg = our_dconfig (r);
!     trace_add (r->server, cfg, "example_hparser()");
      return DECLINED;
  }
  
--- 1033,1039 ----
  	    *cfg;
  
      cfg = our_dconfig (r);
!     trace_add (r->server, r, cfg, "example_hparser()");
      return DECLINED;
  }
  

Mime
View raw message