You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@perl.apache.org by Stas Bekman <st...@stason.org> on 2004/02/09 21:43:00 UTC

[mp2] read/write access to server/process records (+patches)

I'm trying to resolve two related items in todo/release:
------------------
   some autogenerated record accessors shouldn't be get/set but only get (e.g.
   Apache->server->is_virtual(1) is wrong). Need to add a new flag supported by
   MapUtil and map files, where we can say which accessors should be read-only

   Apache::{Server,Process} classes:
   require mutex lock for writing (e.g. $s->(error_fname|error_log)
   Status: most likely some server/process datastructures aren't
   supposed to be modified at request time. So instead of mutex
   locking, we think we should simply have a flag that will be down
   during the startup and will allow methods modifying $s/$proc structs
   (the method will check that flag and if it's up it'll die). At the
   beginning of child_init it'll raise the flag and lower it at the end
   of child_exit.
------------------

The first step: provide support for read-only structure accessors,
made all perl server_rec and process_rec accessors read-only (the patch is 
below).

When applying the change the test suite breaks on the following:

   -  t/response/TestAPI/aplog.pm:65
      $s->loglevel(Apache::LOG_INFO);

   - t/response/TestCompat/apache.pm:65
     $r->server->server_admin($admin);

I don't think these should be writable, since it has the same effect
as allowing 'cwd' in the multithreaded process, where one thread will
affect another. All changes to the server_rec structures need to be
done after the config and before the child_init phase.  process_rec
could be modified during the child_init phase, but I don't think
anybody will want to modify the process_rec struct at all.

On the other hand during the request one can call add_config() and
modify things like ServerAdmin as in t/response/TestCompat/apache.pm:

     my $admin = $r->server->server_admin;
     Apache->httpd_conf('ServerAdmin foo@bar.com');
     ok t_cmp('foo@bar.com', $r->server->server_admin,
              'Apache->httpd_conf');
     $r->server->server_admin($admin);

So if Apache allows doing that and is not concerned about mutexing the
modification of the server_admin field, then it's a bug in Apache and
should be addressed by httpd-dev. If they provide the mutexing, we can
then re-use it for mutexing r/w process/server accessors.

If we do need to support mutexed r/w accessors then we will change
TypeMap.pm and WrapXS.pm to support a new accessor type (@?) similar
to my extension of the maps/*_structures.map '<' to allow read-only
accessors.

Also it's may be an idea to support r/w server accessors only for
those records that can be modified via add_config(), e.g. ServerAdmin.

For records that we aren't sure about making r/w, we should make them
read-only, since we can always re-enable that back after 2.0 API is
frozen. It shouldn't affect anybody.

Here is the patch:

Index: lib/ModPerl/MapUtil.pm
===================================================================
RCS file: /home/cvs/modperl-2.0/lib/ModPerl/MapUtil.pm,v
retrieving revision 1.3
diff -u -r1.3 MapUtil.pm
--- lib/ModPerl/MapUtil.pm	26 Aug 2001 03:38:27 -0000	1.3
+++ lib/ModPerl/MapUtil.pm	9 Feb 2004 20:35:36 -0000
@@ -11,6 +11,7 @@

  our @ISA = qw(Exporter);

+#    '<' => 'auto-generated but gives only a read-only access'
  my %disabled_map = (
      '!' => 'disabled or not yet implemented',
      '~' => 'implemented but not auto-generated',
Index: lib/ModPerl/StructureMap.pm
===================================================================
RCS file: /home/cvs/modperl-2.0/lib/ModPerl/StructureMap.pm,v
retrieving revision 1.3
diff -u -r1.3 StructureMap.pm
--- lib/ModPerl/StructureMap.pm	18 Apr 2001 04:52:27 -0000	1.3
+++ lib/ModPerl/StructureMap.pm	9 Feb 2004 20:35:36 -0000
@@ -103,11 +103,18 @@
          }

          if (s/^(\W)\s*// or $disabled) {
-            $map->{$class}->{$_} = undef;
-            push @{ $self->{disabled}->{ $1 || '!' } }, "$class.$_";
+            # < denotes a read-only accessor
+            if ($1 && $1 eq '<') {
+                $map->{$class}->{$_} = 'ro';
+            }
+            else {
+                $map->{$class}->{$_} = undef;
+                push @{ $self->{disabled}->{ $1 || '!' } }, "$class.$_";
+            }
+
          }
          else {
-            $map->{$class}->{$_} = 1;
+            $map->{$class}->{$_} = 'rw';
          }
      }

Index: lib/ModPerl/TypeMap.pm
===================================================================
RCS file: /home/cvs/modperl-2.0/lib/ModPerl/TypeMap.pm,v
retrieving revision 1.18
diff -u -r1.18 TypeMap.pm
--- lib/ModPerl/TypeMap.pm	30 Dec 2002 00:27:13 -0000	1.18
+++ lib/ModPerl/TypeMap.pm	9 Feb 2004 20:35:36 -0000
@@ -275,19 +275,24 @@

      return unless $class = $self->map_type($stype);

+    use Apache::TestTrace;
+
      for my $e (@{ $struct->{elts} }) {
          my($name, $type) = ($e->{name}, $e->{type});
          my $rtype;

-        next unless $self->structure_map->{$stype}->{$name};
+        # ro/rw/undef(disabled)
+        my $access_mode = $self->structure_map->{$stype}->{$name};
+        next unless $access_mode;
          next unless $rtype = $self->map_type($type);

          push @elts, {
-           name    => $name,
-           type    => $rtype,
-           default => $self->null_type($type),
-           pool    => $self->class_pool($class),
-           class   => $self->{map}->{$type} || "",
+           name        => $name,
+           type        => $rtype,
+           default     => $self->null_type($type),
+           pool        => $self->class_pool($class),
+           class       => $self->{map}->{$type} || "",
+           access_mode => $access_mode,
          };
      }

Index: lib/ModPerl/WrapXS.pm
===================================================================
RCS file: /home/cvs/modperl-2.0/lib/ModPerl/WrapXS.pm,v
retrieving revision 1.65
diff -u -r1.65 WrapXS.pm
--- lib/ModPerl/WrapXS.pm	9 Feb 2004 18:44:43 -0000	1.65
+++ lib/ModPerl/WrapXS.pm	9 Feb 2004 20:35:36 -0000
@@ -181,8 +181,8 @@
          my $class = $struct->{class};

          for my $e (@{ $struct->{elts} }) {
-            my($name, $default, $type) =
-              @{$e}{qw(name default type)};
+            my($name, $default, $type, $access_mode) =
+              @{$e}{qw(name default type access_mode)};

              (my $cast = $type) =~ s/:/_/g;
              my $val = get_value($e);
@@ -196,7 +196,25 @@

              my $attrs = $self->attrs($name);

-            my $code = <<EOF;
+            my $code;
+            if ($access_mode eq 'ro') {
+                $code = <<EOF;
+$type
+$name(obj)
+    $class obj
+
+$attrs
+
+    CODE:
+    RETVAL = ($cast) obj->$name;
+
+    OUTPUT:
+    RETVAL
+
+EOF
+            }
+            elsif ($access_mode eq 'rw') {
+                $code = <<EOF;
  $type
  $name(obj, val=$default)
      $class obj
@@ -217,6 +235,8 @@
      RETVAL

  EOF
+            }
+
              push @{ $self->{XS}->{ $struct->{module} } }, {
                 code  => $code,
                 class => $class,
Index: xs/maps/apache_structures.map
===================================================================
RCS file: /home/cvs/modperl-2.0/xs/maps/apache_structures.map,v
retrieving revision 1.20
diff -u -r1.20 apache_structures.map
--- xs/maps/apache_structures.map	9 Feb 2004 19:05:59 -0000	1.20
+++ xs/maps/apache_structures.map	9 Feb 2004 20:35:39 -0000
@@ -69,31 +69,31 @@
  </request_rec>

  <server_rec>
-   process
-   next
+<  process
+<  next
  -  defn_name
  -  defn_line_number
-   server_admin
-   server_hostname
-   port
-   error_fname
-   error_log
-   loglevel
-   is_virtual
-   module_config
-   lookup_defaults
-   addrs
-   timeout
-   keep_alive_timeout
-   keep_alive_max
-   keep_alive
-   path
+<  server_admin
+<  server_hostname
+<  port
+<  error_fname
+<  error_log
+<  loglevel
+<  is_virtual
+<  module_config
+<  lookup_defaults
+<  addrs
+<  timeout
+<  keep_alive_timeout
+<  keep_alive_max
+<  keep_alive
+<  path
  -  pathlen
-   names
-   wild_names
-   limit_req_line
-   limit_req_fieldsize
-   limit_req_fields
+<  names
+<  wild_names
+<  limit_req_line
+<  limit_req_fieldsize
+<  limit_req_fields
  </server_rec>

  <conn_rec>
@@ -145,11 +145,11 @@
  </module>

  <process_rec>
-   pool
-   pconf
+<  pool
+<  pconf
  -  argc
  !  argv
-   short_name
+<  short_name
  </process_rec>

  <command_rec>





If we decide that for some server methods it's OK to be writable only
before the child_init phase, we could use a new flag introduced by the
below patch: STRUCT_READ_ONLY



Index: lib/ModPerl/Code.pm
===================================================================
RCS file: /home/cvs/modperl-2.0/lib/ModPerl/Code.pm,v
retrieving revision 1.115
diff -u -r1.115 Code.pm
--- lib/ModPerl/Code.pm	9 Feb 2004 18:18:15 -0000	1.115
+++ lib/ModPerl/Code.pm	9 Feb 2004 20:35:35 -0000
@@ -120,7 +120,7 @@
  my @ithread_opts = qw(CLONE PARENT);
  my %flags = (
      Srv => ['NONE', @ithread_opts, qw(ENABLE AUTOLOAD MERGE_HANDLERS),
-            @hook_flags, 'UNSET'],
+            @hook_flags, qw(STRUCT_READ_ONLY UNSET)],
      Dir => [qw(NONE PARSE_HEADERS SETUP_ENV MERGE_HANDLERS GLOBAL_REQUEST 
UNSET)],
      Req => [qw(NONE SET_GLOBAL_REQUEST PARSE_HEADERS SETUP_ENV
                 CLEANUP_REGISTERED)],
Index: src/modules/perl/mod_perl.c
===================================================================
RCS file: /home/cvs/modperl-2.0/src/modules/perl/mod_perl.c,v
retrieving revision 1.206
diff -u -r1.206 mod_perl.c
--- src/modules/perl/mod_perl.c	10 Jan 2004 05:01:04 -0000	1.206
+++ src/modules/perl/mod_perl.c	9 Feb 2004 20:35:39 -0000
@@ -591,6 +591,21 @@
      return OK;
  }

+static int modperl_hook_post_config_last(apr_pool_t *pconf, apr_pool_t *plog,
+                                         apr_pool_t *ptemp, server_rec *s)
+{
+#ifdef USE_ITHREADS
+    MP_dSCFG(s);
+    dTHXa(scfg->mip->parent->perl);
+#endif
+
+    /* no server_rec/process_rec modifications should be done beyond
+     * this point */
+    MpSrvSTRUCT_READ_ONLY_On(scfg);
+
+    return OK;
+}
+
  static int modperl_hook_create_request(request_rec *r)
  {
      MP_dRCFG;
@@ -678,6 +693,9 @@

      ap_hook_post_config(modperl_hook_post_config,
                          NULL, NULL, APR_HOOK_FIRST);
+
+    ap_hook_post_config(modperl_hook_post_config_last,
+                        NULL, NULL, APR_HOOK_REALLY_LAST);

      ap_hook_handler(modperl_response_handler,
                      NULL, NULL, APR_HOOK_MIDDLE);

-- 
__________________________________________________________________
Stas Bekman            JAm_pH ------> Just Another mod_perl Hacker
http://stason.org/     mod_perl Guide ---> http://perl.apache.org
mailto:stas@stason.org http://use.perl.org http://apacheweek.com
http://modperlbook.org http://apache.org   http://ticketmaster.com

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@perl.apache.org
For additional commands, e-mail: dev-help@perl.apache.org


Re: [mp2] read/write access to server/process records (+patches)

Posted by Stas Bekman <st...@stason.org>.
Geoffrey Young wrote:
>>>in general, I don't agree with this - modifying several of the server_rec
>>>slots at request time is something that some (few) mp1 user required.
>>
>>
>>Did they wanted to affect the server for the future requests, or just
>>the current request?
>>
> 
> 
> the only value I see is in scoping server_rec features for the current
> request.  if they mean to change things for the entire server across
> requests then there is no contention.

That's exactly what I was suggesting. And therefore asking whether you want 
the set() accessor to affect the server across requests or not. You just said 
that mp1 users are required to restore any changes at the end of request. 
Doesn't it make it local?

>>>however, threaded environments continue to represent an endless shift
>>>in the
>>>way we typically think of doing things.
>>>
>>>I think one solution that I offered at ApacheCon was to allow
>>>modification
>>>of server_rec slots and other server attributes under unthreaded mpms and
>>>read-only under threaded mpms.  I still think this offers a happy medium
>>>between the two - if you require request-time modification of server
>>>attributes, either use prefork or rethink your requirements.
>>
>>
>>I don't find this a happy medium. An application/module developed on
>>prefork mpm won't work on threaded mpm, e.g. cutting off windows users.
> 
> 
> but we have that limitation already, and will continue to have it in that
> threaded environments are limited to threadsafe modules.  if an application
> requires threads, the developers are forced to make different decisions anyway.

what different decisions you are referring to, Geoff?

>>My definition of the happy medium is to provide the API that works
>>indentically everywhere, and not create a burden for the developers to
>>check their modules/apps on a bunch of platforms. It's possible that the
>>threaded implementation will be slower, but it should work all the same
>>from the user point of view.
> 
> 
> sure, that's the ideal.  however, I find it difficult to accept that we keep
> _all_ users away from certain parts of the API because _some_ environments
> can't handle it.  of course, with properly implemented mutexes it becomes
> possible, but at what cost to the maintainability of the codebase I wonder?

But we already go through the pains of supporting threads, and already have 
the maintainability overhead, I don't think that another mutex will make much 
of a difference.

what is the point of even spending any time on the threaded mpms, if people 
won't use them, since modules/apps won't run on them?

>>If the server_rec mods are required only
>>for the current request, perhaps introducing a concept of localized
>>server_rec is needed.
> 
> 
> perhaps, but that would involve a magical layer that isn't required in
> prefork, so long as you remember to set the server_rec back to its original
> state.  the real problem is threads.

actually that won't quite work. I was thinking to have localized values just 
for the user calls (when they ask to retrieve the data), forgetting that the 
real server_rec needs to be updated too.

obviously threads and writable server records don't go together, since we 
introduce the cwd-syndrom problem.

as for non-threaded mpm, I think it'd be cool to let users change the server 
rec, w/o requiring them to restore it. We could register request cleanup 
handler which will restore the original values.


>>Also some server_rec records could be safely modified at the startup,
> 
> 
> sure.
> 
> 
>>so
>>that's another reason for reviewing what should be r/o and what r/w. So
>>startup and request time modifications are two different things, and
>>should be each addressed separately.
> 
> 
> with the addition of substantial complexity.

But it's a necessary evil, no? If you have a perl API to configure the server, 
why forcing people to use the ugly add_config("Foo bar"), when they can say a 
more perlish $s->foo('bar')?

we could even have two functions for each accessor and fiddle with function 
pointers, reinstalling the r/o implementation after the server startup.

>>But doing:
>>
>>  $r->server->add_config('KeepAlive On');
>>
>>at request time, is not forbidden by the API, no? 
> 
> 
> I would have thought that both this and ServerAdmin would have been
> prevented due to their RSRC_CONF implementation - my experience let me to
> believe that this was exactly equivalent to an .htaccess file, with the same
> restrictions.  but cleary it works in TestCompat/apache.pm so I guess I'm
> wrong about something.

I wrote that as a random example, w/o checking on it. My point is that we need 
to make sure that direct accessor API and config API should match. If you can 
run $r->server->add_config('KeepAlive On'); at request time, you should be 
able to do $r->server->keep_alive(1); and vice versa.

>>Or is it only mod_perl
>>which exposes add_config to users, and thus makes it easy to do unsafe
>>things?
> 
> 
> I don't see anyplace in httpd core that uses ap_build_config at request time.

but as you say $r->server->add_config is the exact equivalent of .htaccess, so 
we can't go wrong here. (exact, minus the timing: .htaccess is run at around 
trans handler, whereas add_config may run much later into the request phases.

Also it looks like it allows only a limited number of Apache core directives:
http://httpd.apache.org/docs-2.0/howto/htaccess.html

> in general I think that, like many things which become popular beyond their
> original scope, the Apache API was never designed to be subjected to the
> many things we twisted Perl people do with it.  for the most part Apache
> handles the strain well, but in cases like this we may be pushing it beyond
> its intention.  cool :)

so may be we should tighten things up in mp2, now that it's no longer safe to 
using the Apache API beyond its scope?

>>I was talking about structs we have no experience with. I thought we
>>have agreed that won't be able to go through every possible
>>method/accessors apache/apr provide and mark only those supported as a
>>2.0 API and the rest 'experimental'.
>>
>>I was just suggesting that it's safe to turn all those 'experimental'
>>accessors, read-only and then add the write access if we learn that it's
>>needed, as it won't break any one's code.
> 
> 
> I'm still not entirely sure I agree with that - it seems like we would then
> be in the same position as mp1 wrt adding features piecemeal.  but let's
> discuss that another time :)

ok

>>>>+<  port
>>>>+<  loglevel
>>>
>>>
>>>
>>>I've required write access to these in the past
>>
>>
>>Sure, I can see the use for loglevel, but it should be local to the
>>request, right?
>>
>>What about the port? Can you give an example of why would you want to
>>change it? Doesn't make much sense to me, since you are already
>>listen'ing to port value. Unless you try to hop to another vhost.
> 
> 
> whoops, that was supposed to be timeout, which would be useful to set on a
> per-request basis for an operation you know to be long-running.

Yes, indeed that was very useful in mp1. Perhaps there are other ways to 
handle that in mp2.

>>>>+<  keep_alive
>>>
>>>I can see write access to this being useful
>>
>>
>>Examples?
> 
> 
> none that I can think of at the moment.  and I suspect that you could always
> force the issue by setting "Connection: close" in the headers, and you
> probably wouldn't want to unset it for non-HTTP/1.1 clients.

Oh, you mean if you have keepalive by default on, but want to turn it off for 
this request. But I don't think this is a right solution at all. If you turn 
it off, you affect all other future requests. Where would you have turned it 
on again?

> but again, I'm a "if you build it, they will come" kind of guy and don't
> believe in cutting off parts of the API just because we can't immediately
> see the benefit.  closing off dangerous parts, of course.  but other than
> that I'm in favor of an open API, even if it means users can shoot
> themselves in the foot sometimes.  there are always learning curves :)

I say, let's take it in a baby steps and finish a sub-set of a safe API and 
release 2.0, then incrementally expose new things.  mp1-1.0 didn't have all 
the features mp1-1.29 has.

__________________________________________________________________
Stas Bekman            JAm_pH ------> Just Another mod_perl Hacker
http://stason.org/     mod_perl Guide ---> http://perl.apache.org
mailto:stas@stason.org http://use.perl.org http://apacheweek.com
http://modperlbook.org http://apache.org   http://ticketmaster.com

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@perl.apache.org
For additional commands, e-mail: dev-help@perl.apache.org


Re: [mp2] read/write access to server/process records (+patches)

Posted by Geoffrey Young <ge...@modperlcookbook.org>.
>> in general, I don't agree with this - modifying several of the server_rec
>> slots at request time is something that some (few) mp1 user required.
> 
> 
> Did they wanted to affect the server for the future requests, or just
> the current request?
> 

the only value I see is in scoping server_rec features for the current
request.  if they mean to change things for the entire server across
requests then there is no contention.


>> however, threaded environments continue to represent an endless shift
>> in the
>> way we typically think of doing things.
>>
>> I think one solution that I offered at ApacheCon was to allow
>> modification
>> of server_rec slots and other server attributes under unthreaded mpms and
>> read-only under threaded mpms.  I still think this offers a happy medium
>> between the two - if you require request-time modification of server
>> attributes, either use prefork or rethink your requirements.
> 
> 
> I don't find this a happy medium. An application/module developed on
> prefork mpm won't work on threaded mpm, e.g. cutting off windows users.

but we have that limitation already, and will continue to have it in that
threaded environments are limited to threadsafe modules.  if an application
requires threads, the developers are forced to make different decisions anyway.

> 
> My definition of the happy medium is to provide the API that works
> indentically everywhere, and not create a burden for the developers to
> check their modules/apps on a bunch of platforms. It's possible that the
> threaded implementation will be slower, but it should work all the same
> from the user point of view.

sure, that's the ideal.  however, I find it difficult to accept that we keep
_all_ users away from certain parts of the API because _some_ environments
can't handle it.  of course, with properly implemented mutexes it becomes
possible, but at what cost to the maintainability of the codebase I wonder?

> If the server_rec mods are required only
> for the current request, perhaps introducing a concept of localized
> server_rec is needed.

perhaps, but that would involve a magical layer that isn't required in
prefork, so long as you remember to set the server_rec back to its original
state.  the real problem is threads.

> 
> Also some server_rec records could be safely modified at the startup,

sure.

> so
> that's another reason for reviewing what should be r/o and what r/w. So
> startup and request time modifications are two different things, and
> should be each addressed separately.

with the addition of substantial complexity.

> But doing:
> 
>   $r->server->add_config('KeepAlive On');
> 
> at request time, is not forbidden by the API, no? 

I would have thought that both this and ServerAdmin would have been
prevented due to their RSRC_CONF implementation - my experience let me to
believe that this was exactly equivalent to an .htaccess file, with the same
restrictions.  but cleary it works in TestCompat/apache.pm so I guess I'm
wrong about something.

> Or is it only mod_perl
> which exposes add_config to users, and thus makes it easy to do unsafe
> things?

I don't see anyplace in httpd core that uses ap_build_config at request time.

in general I think that, like many things which become popular beyond their
original scope, the Apache API was never designed to be subjected to the
many things we twisted Perl people do with it.  for the most part Apache
handles the strain well, but in cases like this we may be pushing it beyond
its intention.  cool :)

> I was talking about structs we have no experience with. I thought we
> have agreed that won't be able to go through every possible
> method/accessors apache/apr provide and mark only those supported as a
> 2.0 API and the rest 'experimental'.
> 
> I was just suggesting that it's safe to turn all those 'experimental'
> accessors, read-only and then add the write access if we learn that it's
> needed, as it won't break any one's code.

I'm still not entirely sure I agree with that - it seems like we would then
be in the same position as mp1 wrt adding features piecemeal.  but let's
discuss that another time :)

> 
> 
>>> +<  port
>>> +<  loglevel
>>
>>
>>
>> I've required write access to these in the past
> 
> 
> Sure, I can see the use for loglevel, but it should be local to the
> request, right?
> 
> What about the port? Can you give an example of why would you want to
> change it? Doesn't make much sense to me, since you are already
> listen'ing to port value. Unless you try to hop to another vhost.

whoops, that was supposed to be timeout, which would be useful to set on a
per-request basis for an operation you know to be long-running.

> 
>>> +<  keep_alive
>>
>> I can see write access to this being useful
> 
> 
> Examples?

none that I can think of at the moment.  and I suspect that you could always
force the issue by setting "Connection: close" in the headers, and you
probably wouldn't want to unset it for non-HTTP/1.1 clients.

but again, I'm a "if you build it, they will come" kind of guy and don't
believe in cutting off parts of the API just because we can't immediately
see the benefit.  closing off dangerous parts, of course.  but other than
that I'm in favor of an open API, even if it means users can shoot
themselves in the foot sometimes.  there are always learning curves :)

--Geoff


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@perl.apache.org
For additional commands, e-mail: dev-help@perl.apache.org


Re: [mp2] read/write access to server/process records (+patches)

Posted by Stas Bekman <st...@stason.org>.
Geoffrey Young wrote:
> 
> Stas Bekman wrote:
> 
>>I'm trying to resolve two related items in todo/release:
> 
> 
>>The first step: provide support for read-only structure accessors,
>>made all perl server_rec and process_rec accessors read-only (the patch
>>is below).
> 
> 
> I agree with most of this, save certain attributes of the server_rec below.
> 
> 
>>I don't think these should be writable, since it has the same effect
>>as allowing 'cwd' in the multithreaded process, where one thread will
>>affect another. All changes to the server_rec structures need to be
>>done after the config and before the child_init phase.
> 
> 
> in general, I don't agree with this - modifying several of the server_rec
> slots at request time is something that some (few) mp1 user required.

Did they wanted to affect the server for the future requests, or just the 
current request?

> however, threaded environments continue to represent an endless shift in the
> way we typically think of doing things.
> 
> I think one solution that I offered at ApacheCon was to allow modification
> of server_rec slots and other server attributes under unthreaded mpms and
> read-only under threaded mpms.  I still think this offers a happy medium
> between the two - if you require request-time modification of server
> attributes, either use prefork or rethink your requirements.

I don't find this a happy medium. An application/module developed on prefork 
mpm won't work on threaded mpm, e.g. cutting off windows users.

My definition of the happy medium is to provide the API that works 
indentically everywhere, and not create a burden for the developers to check 
their modules/apps on a bunch of platforms. It's possible that the threaded 
implementation will be slower, but it should work all the same from the user 
point of view. If the server_rec mods are required only for the current 
request, perhaps introducing a concept of localized server_rec is needed.

Also some server_rec records could be safely modified at the startup, so 
that's another reason for reviewing what should be r/o and what r/w. So 
startup and request time modifications are two different things, and should be 
each addressed separately.

>>So if Apache allows doing that and is not concerned about mutexing the
>>modification of the server_admin field, then it's a bug in Apache and
>>should be addressed by httpd-dev. If they provide the mutexing, we can
>>then re-use it for mutexing r/w process/server accessors.
> 
> 
> I don't recall any mutex code out there, and I suspect that their answer
> would be that modifying server attributes is not a supported use of the API
> and at your own risk.  actually, even in mp1 this was true - if you set a
> server_rec slot at request time you were required to set it back at the end
> of the request or understand that the value would leak across requests if
> you didn't.

But doing:

   $r->server->add_config('KeepAlive On');

at request time, is not forbidden by the API, no? Or is it only mod_perl which 
exposes add_config to users, and thus makes it easy to do unsafe things?

>>For records that we aren't sure about making r/w, we should make them
>>read-only, since we can always re-enable that back after 2.0 API is
>>frozen. It shouldn't affect anybody.
> 
> 
> I think this needs to be sorted out before 2.0.

I was talking about structs we have no experience with. I thought we have 
agreed that won't be able to go through every possible method/accessors 
apache/apr provide and mark only those supported as a 2.0 API and the rest 
'experimental'.

I was just suggesting that it's safe to turn all those 'experimental' 
accessors, read-only and then add the write access if we learn that it's 
needed, as it won't break any one's code.


>>+<  port
>>+<  loglevel
> 
> 
> I've required write access to these in the past

Sure, I can see the use for loglevel, but it should be local to the request, 
right?

What about the port? Can you give an example of why would you want to change 
it? Doesn't make much sense to me, since you are already listen'ing to port 
value. Unless you try to hop to another vhost.

>>+<  keep_alive
> 
> 
> I can see write access to this being useful

Examples?

>> <process_rec>
> 
> 
> as I said above, I agree the process_rec should be off limits.

Thanks.

__________________________________________________________________
Stas Bekman            JAm_pH ------> Just Another mod_perl Hacker
http://stason.org/     mod_perl Guide ---> http://perl.apache.org
mailto:stas@stason.org http://use.perl.org http://apacheweek.com
http://modperlbook.org http://apache.org   http://ticketmaster.com

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@perl.apache.org
For additional commands, e-mail: dev-help@perl.apache.org


Re: [mp2] read/write access to server/process records (+patches)

Posted by Geoffrey Young <ge...@modperlcookbook.org>.

Stas Bekman wrote:
> I'm trying to resolve two related items in todo/release:

> 
> The first step: provide support for read-only structure accessors,
> made all perl server_rec and process_rec accessors read-only (the patch
> is below).

I agree with most of this, save certain attributes of the server_rec below.

> I don't think these should be writable, since it has the same effect
> as allowing 'cwd' in the multithreaded process, where one thread will
> affect another. All changes to the server_rec structures need to be
> done after the config and before the child_init phase.

in general, I don't agree with this - modifying several of the server_rec
slots at request time is something that some (few) mp1 user required.
however, threaded environments continue to represent an endless shift in the
way we typically think of doing things.

I think one solution that I offered at ApacheCon was to allow modification
of server_rec slots and other server attributes under unthreaded mpms and
read-only under threaded mpms.  I still think this offers a happy medium
between the two - if you require request-time modification of server
attributes, either use prefork or rethink your requirements.

> process_rec
> could be modified during the child_init phase, but I don't think
> anybody will want to modify the process_rec struct at all.

I don't think it's a good idea either.

> So if Apache allows doing that and is not concerned about mutexing the
> modification of the server_admin field, then it's a bug in Apache and
> should be addressed by httpd-dev. If they provide the mutexing, we can
> then re-use it for mutexing r/w process/server accessors.

I don't recall any mutex code out there, and I suspect that their answer
would be that modifying server attributes is not a supported use of the API
and at your own risk.  actually, even in mp1 this was true - if you set a
server_rec slot at request time you were required to set it back at the end
of the request or understand that the value would leak across requests if
you didn't.

> For records that we aren't sure about making r/w, we should make them
> read-only, since we can always re-enable that back after 2.0 API is
> frozen. It shouldn't affect anybody.

I think this needs to be sorted out before 2.0.

> +<  port
> +<  loglevel

I've required write access to these in the past

> +<  keep_alive

I can see write access to this being useful

>  <process_rec>

as I said above, I agree the process_rec should be off limits.

--Geoff


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@perl.apache.org
For additional commands, e-mail: dev-help@perl.apache.org