Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP

Loading…

Plack handler apache2 improvements #341

Open
wants to merge 2 commits into from

2 participants

@avar

Here's a couple of commits I've had kicking around in my fork of Plack::Handler::Apache2 and am already using in production.

avar added some commits
@avar avar Plack::Handler::Apache2: add a new plack.handler.apache.request.objec…
…t key

Add a plack.handler.apache.request.object key which contains the
Apache request object, this allows the application to access the
original Apache request object. Originally I had used psgi.input for
this, but in some cases it's going to be overwritten to e.g.:

    bless( \*{'Plack::TempBuffer::PerlIO::$io'}, 'FileHandle' )

I think something like Plack::Request might be overwriting it, but in
any case we need a new key to access this since other things are going
to munge it.

This patch only adds a new key and doesn't change any existing ones,
it's best viewed with "git show -w".
5eba0ce
@avar avar Plack::Handler::Apache2: preserve a copy of %ENV for cleanup handlers
For reasons unknown we'll *sometimes* have an emptied out %ENV by the
time we run the cleanup phase, I can reproduce this by requesting any
URL in my application a few times.

With this patch applied to Plack::Handler::Apache2:

    @@ -90,9 +92,12 @@ sub call_app {
             die "Bad response $res";
         }

    +    print STDERR "Have done everything, have the env = " . Data::Dumper::Dumper(\%ENV);
    +
         if (@{ $env->{'psgix.cleanup.handlers'} }) {
             $r->push_handlers(
                 PerlCleanupHandler => sub {
    +                print STDERR "Running the cleanup handler, have the env = " . Data::Dumper::Dumper(\%ENV);
                     for my $cleanup_handler (@{ $env->{'psgix.cleanup.handlers'} }) {
                         $cleanup_handler->($env);
                     }

I'll always get the full %ENV when I dump it in "Have done
everything", but by the time I get to running the cleanup handlers
it's sometimes been completely emptied.

Work around this by preserving %ENV and restoring it if it gets lost,
this helps a lot with migrating existing Apache applications which
look at %ENV, and since we normally set %ENV with the "subprocess_env"
cal it's really confusing to only provide it some of the time.
0b1bc01
@miyagawa
Owner

out of curiosity, isn't there a way to retrieve Apache object like Apache->request like you did with mp1? I hesitate to put $r into $env since it might make a little easier to make a circular ref.

@avar

There is, you can do:

my $r = MOD_PERL2 ? Apache2::RequestUtil->request : Apache->request;

But if you're concerned about putting $r in env it's already there as psgi.input, it just occasionally gets overwritten.

@miyagawa
Owner

But if you're concerned about putting $r in env it's already there as psgi.input, it just occasionally gets overwritten.

right. I still am not sure why this cleanup handlers would not cause issues with circular refs:

       $r->push_handlers(
            PerlCleanupHandler => sub {
                for my $cleanup_handler (@{ $env->{'psgix.cleanup.handlers'} }) {
                    $cleanup_handler->($env);
                }
...

so we push $env into cleanup handlers and now $env has the reference to $r.

@miyagawa
Owner

To me it looks like it should get a copy of $env->{'psgix.cleanup.handlers'} before passing them to PerlCleanupHandler.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Commits on Oct 16, 2012
  1. @avar

    Plack::Handler::Apache2: add a new plack.handler.apache.request.objec…

    avar authored
    …t key
    
    Add a plack.handler.apache.request.object key which contains the
    Apache request object, this allows the application to access the
    original Apache request object. Originally I had used psgi.input for
    this, but in some cases it's going to be overwritten to e.g.:
    
        bless( \*{'Plack::TempBuffer::PerlIO::$io'}, 'FileHandle' )
    
    I think something like Plack::Request might be overwriting it, but in
    any case we need a new key to access this since other things are going
    to munge it.
    
    This patch only adds a new key and doesn't change any existing ones,
    it's best viewed with "git show -w".
  2. @avar

    Plack::Handler::Apache2: preserve a copy of %ENV for cleanup handlers

    avar authored
    For reasons unknown we'll *sometimes* have an emptied out %ENV by the
    time we run the cleanup phase, I can reproduce this by requesting any
    URL in my application a few times.
    
    With this patch applied to Plack::Handler::Apache2:
    
        @@ -90,9 +92,12 @@ sub call_app {
                 die "Bad response $res";
             }
    
        +    print STDERR "Have done everything, have the env = " . Data::Dumper::Dumper(\%ENV);
        +
             if (@{ $env->{'psgix.cleanup.handlers'} }) {
                 $r->push_handlers(
                     PerlCleanupHandler => sub {
        +                print STDERR "Running the cleanup handler, have the env = " . Data::Dumper::Dumper(\%ENV);
                         for my $cleanup_handler (@{ $env->{'psgix.cleanup.handlers'} }) {
                             $cleanup_handler->($env);
                         }
    
    I'll always get the full %ENV when I dump it in "Have done
    everything", but by the time I get to running the cleanup handlers
    it's sometimes been completely emptied.
    
    Work around this by preserving %ENV and restoring it if it gets lost,
    this helps a lot with migrating existing Apache applications which
    look at %ENV, and since we normally set %ENV with the "subprocess_env"
    cal it's really confusing to only provide it some of the time.
This page is out of date. Refresh to see the latest.
Showing with 42 additions and 18 deletions.
  1. +42 −18 lib/Plack/Handler/Apache2.pm
View
60 lib/Plack/Handler/Apache2.pm
@@ -50,18 +50,19 @@ sub call_app {
my $env = {
%ENV,
- 'psgi.version' => [ 1, 1 ],
- 'psgi.url_scheme' => ($ENV{HTTPS}||'off') =~ /^(?:on|1)$/i ? 'https' : 'http',
- 'psgi.input' => $r,
- 'psgi.errors' => *STDERR,
- 'psgi.multithread' => Plack::Util::FALSE,
- 'psgi.multiprocess' => Plack::Util::TRUE,
- 'psgi.run_once' => Plack::Util::FALSE,
- 'psgi.streaming' => Plack::Util::TRUE,
- 'psgi.nonblocking' => Plack::Util::FALSE,
- 'psgix.harakiri' => Plack::Util::TRUE,
- 'psgix.cleanup' => Plack::Util::TRUE,
- 'psgix.cleanup.handlers' => [],
+ 'plack.handler.apache.request.object' => $r,
+ 'psgi.version' => [ 1, 1 ],
+ 'psgi.url_scheme' => ($ENV{HTTPS}||'off') =~ /^(?:on|1)$/i ? 'https' : 'http',
+ 'psgi.input' => $r,
+ 'psgi.errors' => *STDERR,
+ 'psgi.multithread' => Plack::Util::FALSE,
+ 'psgi.multiprocess' => Plack::Util::TRUE,
+ 'psgi.run_once' => Plack::Util::FALSE,
+ 'psgi.streaming' => Plack::Util::TRUE,
+ 'psgi.nonblocking' => Plack::Util::FALSE,
+ 'psgix.harakiri' => Plack::Util::TRUE,
+ 'psgix.cleanup' => Plack::Util::TRUE,
+ 'psgix.cleanup.handlers' => [],
};
if (defined(my $HTTP_AUTHORIZATION = $r->headers_in->{Authorization})) {
@@ -90,14 +91,37 @@ sub call_app {
}
if (@{ $env->{'psgix.cleanup.handlers'} }) {
+ my %ENV_copy_for_PerlCleanupHandler = %ENV;
+ # This is referred to in the comment below
$r->push_handlers(
PerlCleanupHandler => sub {
- for my $cleanup_handler (@{ $env->{'psgix.cleanup.handlers'} }) {
- $cleanup_handler->($env);
- }
-
- if ($env->{'psgix.harakiri.commit'}) {
- $r->child_terminate;
+ my $do_cleanup_handlers = sub {
+ for my $cleanup_handler (@{ $env->{'psgix.cleanup.handlers'} }) {
+ $cleanup_handler->($env);
+ }
+
+ if ($env->{'psgix.harakiri.commit'}) {
+ $r->child_terminate;
+ }
+
+ return;
+ };
+
+ if (keys %ENV) {
+ $do_cleanup_handlers->();
+ } else {
+ # Sometimes Apache will seemingly empty out %ENV,
+ # i.e. if we were to dump out %ENV when we assign to
+ # %ENV_copy_for_PerlCleanupHandler above we'd get a
+ # populated %ENV, but it'll be empty by the time we
+ # get here.
+ #
+ # I haven't tracked down *why* this happens, so this
+ # might be cargo-culting around something, but this
+ # workaround works for me and isn't too expensive.
+ local %ENV = %ENV_copy_for_PerlCleanupHandler;
+
+ $do_cleanup_handlers->();
}
},
);
Something went wrong with that request. Please try again.