Re: ap_read_config in 2.3.11

2011-04-25 Thread Torsten Förtsch 
On Tuesday, April 19, 2011 23:35:23 Stefan Fritsch wrote:
 The attached patch adds two functions, ap_reserve_module_slots() and 
 ap_reserve_module_slots_directive(), which can be called by relevant 
 modules in the pre config stage.
 
 Can you please test this patch and call 
 ap_reserve_module_slots_directive(PerlLoadModule) in mod_perl's 
 pre_config hook?

Thanks a lot! It seems your patch works flawlessly after I figured out 
another bug in the modperl test suite.

Torsten Förtsch

-- 
Need professional modperl support? Hire me! (http://foertsch.name)

Like fantasy? http://kabatinte.net

Re: ap_read_config in 2.3.11

2011-04-25 Thread Stefan Fritsch 
On Monday 25 April 2011, Torsten Förtsch wrote:
 Thanks a lot! It seems your patch works flawlessly after I figured
 out  another bug in the modperl test suite.

Thanks for testing. Committed as r1096569

Re: ap_read_config in 2.3.11

2011-04-19 Thread Stefan Fritsch 
On Monday 18 April 2011, Stefan Fritsch wrote:
 I don't really like that. The admin should not have to count the
 LoadModule directives in order to tune DynamicModulesMax for
 minimum memory usage. Httpd should do the counting.

 - a module that wants to add modules later (like mod_perl) could
 call some function in its insert_hooks function, which would cause
 the adjustment of conf_vector_length to happen in post_config
 instead

The attached patch adds two functions, ap_reserve_module_slots() and 
ap_reserve_module_slots_directive(), which can be called by relevant 
modules in the pre config stage.

Can you please test this patch and call 
ap_reserve_module_slots_directive(PerlLoadModule) in mod_perl's 
pre_config hook?
diff --git a/include/http_config.h b/include/http_config.h
index 6d8e185..80956c0 100644
--- a/include/http_config.h
+++ b/include/http_config.h
@@ -988,6 +988,23 @@ AP_DECLARE(void) ap_register_hooks(module *m, apr_pool_t *p);
 AP_DECLARE(void) ap_fixup_virtual_hosts(apr_pool_t *p,
 server_rec *main_server);
 
+/**
+ * Reserve some modules slots for modules loaded by other means than
+ * EXEC_ON_READ directives.
+ * Relevant modules should call this in the pre_config stage.
+ * @param count The number of slots to reserve.
+ */
+AP_DECLARE(void) ap_reserve_module_slots(int count);
+
+/**
+ * Reserve some modules slots for modules loaded by a specific
+ * non-EXEC_ON_READ config directive.
+ * This counts how often the given directive is used in the config and calls
+ * ap_reserve_module_slots() accordingly.
+ * @param directive The name of the directive
+ */
+AP_DECLARE(void) ap_reserve_module_slots_directive(const char *directive);
+
 /* For http_request.c... */
 
 /**
@@ -995,7 +1012,7 @@ AP_DECLARE(void) ap_fixup_virtual_hosts(apr_pool_t *p,
  * @param p The pool to allocate the config vector from
  * @return The config vector
  */
-AP_CORE_DECLARE(ap_conf_vector_t*) ap_create_request_config(apr_pool_t *p);
+AP_DECLARE(ap_conf_vector_t*) ap_create_request_config(apr_pool_t *p);
 
 /**
  * Setup the config vector for per dir module configs
diff --git a/include/http_core.h b/include/http_core.h
index ba66de8..7b822a5 100644
--- a/include/http_core.h
+++ b/include/http_core.h
@@ -766,7 +766,8 @@ typedef struct {
 unsigned int min_loglevel;
 } ap_errorlog_format_item;
 
-AP_DECLARE(void) ap_register_log_hooks(apr_pool_t *p);
+AP_CORE_DECLARE(void) ap_register_log_hooks(apr_pool_t *p);
+AP_CORE_DECLARE(void) ap_register_config_hooks(apr_pool_t *p);
 
 /* --
  *
diff --git a/modules/generators/mod_cgid.c b/modules/generators/mod_cgid.c
index 651c6a0..91d290e 100644
--- a/modules/generators/mod_cgid.c
+++ b/modules/generators/mod_cgid.c
@@ -421,7 +421,7 @@ static apr_status_t get_req(int fd, request_rec *r, char **argv0, char ***env,
 }
 
 /* handle module indexes and such */
-rconf = (void **) apr_pcalloc(r-pool, sizeof(void *) * (total_modules + DYNAMIC_MODULE_LIMIT));
+rconf = (void **)ap_create_request_config(r-pool);
 
 temp_core = (core_request_config *)apr_palloc(r-pool, sizeof(core_module));
 rconf[req-core_module_index] = (void *)temp_core;
diff --git a/server/config.c b/server/config.c
index 85dd709..3d30b5b 100644
--- a/server/config.c
+++ b/server/config.c
@@ -198,6 +198,8 @@ static int max_modules = 0;
  */
 static int conf_vector_length = 0;
 
+static int reserved_module_slots = 0;
+
 AP_DECLARE_DATA module *ap_top_module = NULL;
 AP_DECLARE_DATA module **ap_loaded_modules=NULL;
 
@@ -548,6 +550,10 @@ AP_DECLARE(const char *) ap_add_module(module *m, apr_pool_t *p,
 reached. Please increase 
 DYNAMIC_MODULE_LIMIT and recompile., m-name);
 }
+/*
+ * If this fails some module forgot to call ap_reserve_module_slots*.
+ */
+ap_assert(total_modules  conf_vector_length);
 
 m-module_index = total_modules++;
 dynamic_modules++;
@@ -2257,10 +2263,36 @@ static server_rec *init_server_config(process_rec *process, apr_pool_t *p)
 
 static apr_status_t reset_conf_vector_length(void *dummy)
 {
+reserved_module_slots = 0;
 conf_vector_length = max_modules;
 return APR_SUCCESS;
 }
 
+static int conf_vector_length_pre_config(apr_pool_t *pconf, apr_pool_t *plog,
+ apr_pool_t *ptemp)
+{
+/*
+ * We have loaded all modules that are loaded by EXEC_ON_READ directives.
+ * From now on we reduce the size of the config vectors to what we need,
+ * plus what has been reserved (e.g. by mod_perl) for additional modules
+ * loaded later on.
+ * If max_modules is too small, ap_add_module() will abort.
+ */
+if (total_modules + reserved_module_slots  max_modules) {
+conf_vector_length = total_modules + reserved_module_slots;
+}
+

Re: ap_read_config in 2.3.11

2011-04-18 Thread Stefan Fritsch 
On Sunday 17 April 2011, Torsten Förtsch wrote:
 On Tuesday, April 12, 2011 18:24:28 William A. Rowe Jr. wrote:
  Suggestion - an EXEC_ON_READ 'DynamicModulesMax' directive, which
  would let us conf_vector_length = total_modules +
  dyn_modules_max; after the read_config, and finally lock down
  conf_vector_length = total_modules; at the end of post_config. 
  WDYT?
 
 The attached patch implements the DynamicModulesMax directive. It
 can appear anywhere in the config file but is best placed *before*
 any LoadModule directive. Up to post_config the number of
 prelinked modules plus DynamicModulesMax is used as
 conf_vector_length. In core_post_config the length is then settled
 on total_modules.

I don't really like that. The admin should not have to count the 
LoadModule directives in order to tune DynamicModulesMax for minimum 
memory usage. Httpd should do the counting.

Maybe something like this:

- normally, conf_vector_length is adjusted after all EXEC_ON_READ 
directives have been processed
- a module that wants to add modules later (like mod_perl) could call 
some function in its insert_hooks function, which would cause the 
adjustment of conf_vector_length to happen in post_config instead

The function to be called could optionally take a number of module 
slots that should be kept in reserve. Then conf_vector_length would be 
adjusted to total_modules + reserve after all EXEC_ON_READ is done, 
and to total_modules in post_config. Or maybe the reserve should 
simply be set by some new directive like DynamicModulesMax?

What do you think?

BTW, I think there is something fishy with PerlLoadModule (or at least 
the documentation): The docs says that perl modules can use 
Apache2::Const::EXEC_ON_READ, but this cannot work because 
PerlLoadModule itself is executed too late.

Cheers,
Stefan

Re: ap_read_config in 2.3.11

2011-04-17 Thread Torsten Förtsch 
On Tuesday, April 12, 2011 18:24:28 William A. Rowe Jr. wrote:
 Suggestion - an EXEC_ON_READ 'DynamicModulesMax' directive, which would
 let us conf_vector_length = total_modules + dyn_modules_max; after the
 read_config, and finally lock down conf_vector_length = total_modules;
 at the end of post_config.  WDYT?

The attached patch implements the DynamicModulesMax directive. It can appear 
anywhere in the config file but is best placed *before* any LoadModule 
directive. Up to post_config the number of prelinked modules plus 
DynamicModulesMax is used as conf_vector_length. In core_post_config the 
length is then settled on total_modules.

Torsten Förtsch

-- 
Need professional modperl support? Hire me! (http://foertsch.name)

Like fantasy? http://kabatinte.net
Index: server/config.c
===
--- server/config.c	(revision 1094174)
+++ server/config.c	(working copy)
@@ -181,22 +181,22 @@
 static int total_modules = 0;
 
 /* dynamic_modules is the number of modules that have been added
- * after the pre-loaded ones have been set up. It shouldn't be larger
- * than DYNAMIC_MODULE_LIMIT.
+ * after the pre-loaded ones have been set up. It should be at most
+ * dynamic_modules_max.
  */
 static int dynamic_modules = 0;
 
-/* The maximum possible value for total_modules, i.e. number of static
- * modules plus DYNAMIC_MODULE_LIMIT.
+/* The maximum possible value for dynamic_modules. Shouldn't be larger than
+ * DYNAMIC_MODULE_LIMIT.
  */
-static int max_modules = 0;
+static int dynamic_modules_max;
 
 /* The number of elements we need to alloc for config vectors. Before loading
- * of dynamic modules, we must be liberal and set this to max_modules. After
- * loading of dynamic modules, we can trim it down to total_modules. On
- * restart, reset to max_modules.
+ * of dynamic modules, we must be liberal and set this to the largest possible
+ * value (number of prelinked modules + dynamic_modules_max). After loading of
+ * dynamic modules, we can trim it down to total_modules.
  */
-static int conf_vector_length = 0;
+static int conf_vector_length;
 
 AP_DECLARE_DATA module *ap_top_module = NULL;
 AP_DECLARE_DATA module **ap_loaded_modules=NULL;
@@ -234,6 +234,22 @@
  * overridden).
  */
 
+AP_DECLARE(void) ap_set_dynamic_modules_max(int lim)
+{
+dynamic_modules_max = lim;
+conf_vector_length = dynamic_modules_max + total_modules - dynamic_modules;
+}
+
+AP_DECLARE(int) ap_get_dynamic_modules_max(void)
+{
+return dynamic_modules_max;
+}
+
+AP_DECLARE(void) ap_settle_module_count(void)
+{
+conf_vector_length = total_modules;
+}
+
 static ap_conf_vector_t *create_empty_config(apr_pool_t *p)
 {
 void *conf_vector = apr_pcalloc(p, sizeof(void *) * conf_vector_length);
@@ -542,7 +558,7 @@
 }
 
 if (m-module_index == -1) {
-if (dynamic_modules = DYNAMIC_MODULE_LIMIT) {
+if (dynamic_modules = dynamic_modules_max) {
 return apr_psprintf(p, Module \%s\ could not be loaded, 
 because the dynamic module limit was 
 reached. Please increase 
@@ -740,8 +756,7 @@
 for (m = ap_preloaded_modules; *m != NULL; m++)
 (*m)-module_index = total_modules++;
 
-max_modules = total_modules + DYNAMIC_MODULE_LIMIT + 1;
-conf_vector_length = max_modules;
+ap_set_dynamic_modules_max(DYNAMIC_MODULE_LIMIT);
 
 /*
  *  Initialise list of loaded modules and short names
@@ -2255,12 +2270,6 @@
 }
 
 
-static apr_status_t reset_conf_vector_length(void *dummy)
-{
-conf_vector_length = max_modules;
-return APR_SUCCESS;
-}
-
 AP_DECLARE(server_rec*) ap_read_config(process_rec *process, apr_pool_t *ptemp,
const char *filename,
ap_directive_t **conftree)
@@ -2272,6 +2281,7 @@
 return s;
 }
 
+ap_set_dynamic_modules_max(DYNAMIC_MODULE_LIMIT);
 init_config_globals(p);
 
 /* All server-wide config files now have the SAME syntax... */
@@ -2309,14 +2319,6 @@
 return NULL;
 }
 
-/*
- * We have loaded the dynamic modules. From now on we know exactly how
- * long the config vectors need to be.
- */
-conf_vector_length = total_modules;
-apr_pool_cleanup_register(p, NULL, reset_conf_vector_length,
-  apr_pool_cleanup_null);
-
 error = process_command_config(s, ap_server_post_read_config, conftree,
p, ptemp);
 
Index: server/core.c
===
--- server/core.c	(revision 1094174)
+++ server/core.c	(working copy)
@@ -3167,6 +3167,19 @@
 }
 #endif
 
+static const char *set_dynamic_modules_max(cmd_parms *cmd, void *dummy,
+	   const char *arg)
+{
+const char *err = ap_check_cmd_context(cmd, NOT_IN_DIR_LOC_FILE);
+
+if (err != NULL) {
+return err;
+}
+
+

Re: ap_read_config in 2.3.11

2011-04-12 Thread William A. Rowe Jr. 
On 4/11/2011 2:14 PM, Stefan Fritsch wrote:
 On Monday 11 April 2011, Torsten Förtsch wrote:
 I am working on porting modperl to the upcoming httpd 2.4. One
 problem is the line

   conf_vector_length = total_modules;

 in ap_read_config (config.c:2300).

 Modperl provides a way to write modules in Perl. These modules are
 loaded by the directive PerlLoadModule which is executed later
 than line 2300. Such modules are then appended to the module list,
 get a module structure, can create config directives etc. They
 also have a module index and allocate their place in config
 vectors.

 Now, the line above limits the config vector length to the number
 of modules known before any one perl module could be loaded.

 How can this situation be solved?
 
 When exactly is PerlLoadModule executed? The above code assumes that 
 modules are loaded with EXEC_ON_READ during reading of the config.
 
 I guess the optimization could be done later, after the config has 
 been parsed completely. That would waste some memory in pconf but 
 still preserve the optimization during request processing, which is 
 more important.

Suggestion - an EXEC_ON_READ 'DynamicModulesMax' directive, which would
let us conf_vector_length = total_modules + dyn_modules_max; after the
read_config, and finally lock down conf_vector_length = total_modules;
at the end of post_config.  WDYT?

Re: ap_read_config in 2.3.11

2011-04-12 Thread Torsten Förtsch 
On Monday, April 11, 2011 21:14:53 Stefan Fritsch wrote:
 I guess the optimization could be done later, after the config has 
 been parsed completely. That would waste some memory in pconf but 
 still preserve the optimization during request processing, which is 
 more important.

The enclosed patch moves the conf_vector_length adjustment to when 
ap_process_config_tree() has done its work. I think that is enough for 
mod_perl.

On Tuesday, April 12, 2011 18:24:28 William A. Rowe Jr. wrote:
 Suggestion - an EXEC_ON_READ 'DynamicModulesMax' directive, which would
 let us conf_vector_length = total_modules + dyn_modules_max; after the
 read_config, and finally lock down conf_vector_length = total_modules;
 at the end of post_config.  WDYT?

But yes, moving the adjustment to postconfig will solve the problem more 
generally. I'll see if I can conceive a patch.

Torsten Förtsch

-- 
Need professional modperl support? Hire me! (http://foertsch.name)

Like fantasy? http://kabatinte.net
Index: server/config.c
===
--- server/config.c	(revision 1091323)
+++ server/config.c	(working copy)
@@ -1953,6 +1953,12 @@
 return NULL;
 }
 
+static apr_status_t reset_conf_vector_length(void *dummy)
+{
+conf_vector_length = max_modules;
+return APR_SUCCESS;
+}
+
 AP_DECLARE(int) ap_process_config_tree(server_rec *s,
ap_directive_t *conftree,
apr_pool_t *p,
@@ -1981,6 +1987,20 @@
 return HTTP_INTERNAL_SERVER_ERROR;
 }
 
+/*
+ * We have loaded the dynamic modules. From now on we know exactly how
+ * long the config vectors need to be.
+ * 
+ * You may be tempted to move the next 3 lines to ap_read_config()
+ * just after ap_process_resource_config() to save a few bytes in pconf
+ * since at that point all the LoadModule directives have been processed.
+ * Don't do that because it breaks stuff like mod_perl that offer a
+ * way to register modules as part of their configuration.
+ */
+conf_vector_length = total_modules;
+apr_pool_cleanup_register(p, NULL, reset_conf_vector_length,
+  apr_pool_cleanup_null);
+
 return OK;
 }
 
@@ -2255,12 +2275,6 @@
 }
 
 
-static apr_status_t reset_conf_vector_length(void *dummy)
-{
-conf_vector_length = max_modules;
-return APR_SUCCESS;
-}
-
 AP_DECLARE(server_rec*) ap_read_config(process_rec *process, apr_pool_t *ptemp,
const char *filename,
ap_directive_t **conftree)
@@ -2309,14 +2323,6 @@
 return NULL;
 }
 
-/*
- * We have loaded the dynamic modules. From now on we know exactly how
- * long the config vectors need to be.
- */
-conf_vector_length = total_modules;
-apr_pool_cleanup_register(p, NULL, reset_conf_vector_length,
-  apr_pool_cleanup_null);
-
 error = process_command_config(s, ap_server_post_read_config, conftree,
p, ptemp);

ap_read_config in 2.3.11

2011-04-11 Thread Torsten Förtsch 
Hi,

I am working on porting modperl to the upcoming httpd 2.4. One problem is the 
line

  conf_vector_length = total_modules;

in ap_read_config (config.c:2300).

Modperl provides a way to write modules in Perl. These modules are loaded by 
the directive PerlLoadModule which is executed later than line 2300. Such 
modules are then appended to the module list, get a module structure, can 
create config directives etc. They also have a module index and allocate their 
place in config vectors.

Now, the line above limits the config vector length to the number of modules 
known before any one perl module could be loaded.

How can this situation be solved?

Thanks,
Torsten Förtsch

-- 
Need professional modperl support? Hire me! (http://foertsch.name)

Like fantasy? http://kabatinte.net

Re: ap_read_config in 2.3.11

2011-04-11 Thread Stefan Fritsch 
On Monday 11 April 2011, Torsten Förtsch wrote:
 I am working on porting modperl to the upcoming httpd 2.4. One
 problem is the line
 
   conf_vector_length = total_modules;
 
 in ap_read_config (config.c:2300).
 
 Modperl provides a way to write modules in Perl. These modules are
 loaded by the directive PerlLoadModule which is executed later
 than line 2300. Such modules are then appended to the module list,
 get a module structure, can create config directives etc. They
 also have a module index and allocate their place in config
 vectors.
 
 Now, the line above limits the config vector length to the number
 of modules known before any one perl module could be loaded.
 
 How can this situation be solved?

When exactly is PerlLoadModule executed? The above code assumes that 
modules are loaded with EXEC_ON_READ during reading of the config.

I guess the optimization could be done later, after the config has 
been parsed completely. That would waste some memory in pconf but 
still preserve the optimization during request processing, which is 
more important.