Re: [MP2 Patch] Bug ? Recursion in parsing of PerlSection

Philippe M. Chiasson wrote:
> On Tue, 2003-05-27 at 13:01, Stas Bekman wrote:
>
> > Philippe M. Chiasson wrote:
> >
> > > On Tue, 2003-05-27 at 12:00, Stas Bekman wrote:
> > >
> > >
> > > > Philippe M. Chiasson wrote:
> > > > [...]
> > > >
> > > >
> > > > > Is evaluated in Apache::ReadConfig's namespace, even if it does nothing.
> > > > >
> > > > > Once again Apache::PerlSections is called in with Apache::ReadConfig as
> > > > > the namespace
> > > > >
> > > > > This handler will once again see
> > > > >
> > > > > @Apache::ReadConfig::include = "foo.conf";
> > > > >
> > > > > and here starts the recursion.
> > > > >
> > > > >
> > > > > This is a hairy problem indeed. I realized this partially in the past,
> > > > > when re-introducing PerlSaveConfig but didn't think of this particular
> > > > > recursive possibility.
> > > > >
> > > > > The way I see it, there is only a few ways to proprely fix this.
> > > > >
> > > > > 1. To use one _extra_ namespace to save the config into, if
> > > > > PerlSaveConfig is specified, and delete or move elements from
> > > > > Apache::ReadConfig as it is being processed.
> > > > >
> > > > > 2. To use automatically-generated namespaces for each <Perl > sections,
> > > > > like ModPerl::Registry, like Apache::ReadConfig::blocknnn or something.
> > > > >
> > > > > I personally prefer solution #2.
> > > > >
> > > > > Ideas/suggestions ? If not, I'll probably give a shot at implementing #2
> > > > > sometime this week.
> > > >
> > > > Do you have to process again the include directive? 
> > >
> > >
> > > Well, if I PerlSection was smart enough it could treat @Include
> > > directives as a special case, but that wouldn't solve the whole problem.
> > >
> > >
> > >
> > > > If not, why not use the 
> > > > same solution as used by require, which stores name=>path of loaded files in 
> > > > %INC. I don't know where you currently store the processed result of include, 
> > > > but if you lose it, why not store it in %Apache::ReadConfig::INC, with key 
> > > > being the included filename and value the result of the include? Though need 
> > > > to ensure that we have no duplication when two files with indentical names 
> > > > (but located at different paths) are included. Should probably resolve these 
> > > > to a full path? or append the parent's path?
> > >
> > >
> > > Yes, I thought along those lines, but the simplest problem with this
> > > approach is this (unrelated to first problem, it's a second problem)
> > >
> > > <Perl >
> > > push @Alias = [ '/foo', '/bar' ];
> > > </Perl>
> > >
> > > And later on
> > >
> > > <Perl >
> > > 1;
> > > </Perl>
> > >
> > > Will cause a warning about one Alias shadowing another one.
> > >
> > > The core reason why <Perl > blocks are a bit broken right now is
> > > because multiple <Perl > sections add to the same namespace (if
> > > SaveSections is on, or if @include is used), and when the PerlSection
> > > handler kicks in , it has no way to differentiate between the content it
> > > has processed before vs. the new stuff.
> > >
> > > I thougth about many ways of moving stuff as it's being processed, but
> > > it would be quite complex, as it's all recursive and all.
> > >
> > > The following patch does work nicely for me though.
> > >
> > > It puts each <Perl > block in it's own namespace, and keeps feeding them
> > > to the handler as usual. It all works fine. When it's time to have
> > > Apache::PerlSections->Dump(), it'll be just a matter of iterating over
> > > all those namespaces, striping them out before printing them.
> > >
> > > And since Dump() has not been ported yet (mea culpa), I figured this
> > > change wouldn't break anything.
> > >
> > > Thoughts ?
> >
> > I see at least two drawbacks with this approach:
> >
> > - waste of memory, namespaces aren't cheap (at least use modperl_mgv)
>
>
> By default, the <Perl > namespaces aren't saved, only if you explicitely
> specify it with SaveSections
>
>
> > - if you add a new <Perl > above the existing ones, this code will break:
> >
> >
> > > -    ok not exists $Apache::ReadConfig::Location{'/perl_sections'};
> > > +    ok not exists $Apache::ReadConfig::1::Location{'/perl_sections'};
>
> I know that, it's just test code. Nobody should be peeking in
> $Apache::ReadConfig::* by themselves, it's against the API, IMHO
>
>
> > why not monitoring what information was seen already using some other method? 
> > e.g. push those chunks into an array and flatten it when you need and you 
> > still have a control on what you have seen and what not? in the worst case you 
> > can store unprocessed flattened sections, then undef everything in 
> > Apache::ReadConfig, and process it again. Will that work?
>
>
> That would most likely work, using some soft of mechanism to either flag
> things you've processed or moving things around as you process them.
>
> What bothers me with this is that <Perl > sections offer the possiblilty
> of writing alternate handlers for it. The only service provided to those
> handlers it evaling the code in the <perl > section in some namespace
> and then invoking the handler, providing it with a pointer to that
> namespace.
>
> I know there is only one such handler right now, Apache::PerlSection,
> but if there were others at some point, they would have to implement the
> same protective mechanisms in their own code.
>
> Dunno, but the more I think about it, the more treating <Perl > blocks
> just like ModPerl::Registry treats scripts (to each it's own namespace)
> sounds like a good idea to me. But I could just be lazy or wrong ...

PerlSections or any other handlers still implement their own tracking by 
checking whether gv is defined.

In that case why not use modperl_mgv, which is both: more lightweight and 
hides the implementation from users, who despite the warnings may want to rely 
on it.

I thought that since you use suggested using namespaces 1, 2, 3, 4... and you 
never access those directly other than looping over the numbers an array 
should work just fine.


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