Skip to content
This repository

Plack handler apache2 improvements #341

Open
wants to merge 2 commits into from

2 participants

Ævar Arnfjörð Bjarmason Tatsuhiko Miyagawa
Ævar Arnfjörð Bjarmason

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
Ævar Arnfjörð Bjarmason 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
Ævar Arnfjörð Bjarmason 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
Tatsuhiko 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.

Ævar Arnfjörð Bjarmason

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.

Tatsuhiko 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.

Tatsuhiko 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

Showing 2 unique commits by 1 author.

Oct 16, 2012
Ævar Arnfjörð Bjarmason 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
Ævar Arnfjörð Bjarmason 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
This page is out of date. Refresh to see the latest.

Showing 1 changed file with 42 additions and 18 deletions. Show diff stats Hide diff stats

  1. +42 18 lib/Plack/Handler/Apache2.pm
60 lib/Plack/Handler/Apache2.pm
@@ -50,18 +50,19 @@ sub call_app {
50 50
51 51 my $env = {
52 52 %ENV,
53   - 'psgi.version' => [ 1, 1 ],
54   - 'psgi.url_scheme' => ($ENV{HTTPS}||'off') =~ /^(?:on|1)$/i ? 'https' : 'http',
55   - 'psgi.input' => $r,
56   - 'psgi.errors' => *STDERR,
57   - 'psgi.multithread' => Plack::Util::FALSE,
58   - 'psgi.multiprocess' => Plack::Util::TRUE,
59   - 'psgi.run_once' => Plack::Util::FALSE,
60   - 'psgi.streaming' => Plack::Util::TRUE,
61   - 'psgi.nonblocking' => Plack::Util::FALSE,
62   - 'psgix.harakiri' => Plack::Util::TRUE,
63   - 'psgix.cleanup' => Plack::Util::TRUE,
64   - 'psgix.cleanup.handlers' => [],
  53 + 'plack.handler.apache.request.object' => $r,
  54 + 'psgi.version' => [ 1, 1 ],
  55 + 'psgi.url_scheme' => ($ENV{HTTPS}||'off') =~ /^(?:on|1)$/i ? 'https' : 'http',
  56 + 'psgi.input' => $r,
  57 + 'psgi.errors' => *STDERR,
  58 + 'psgi.multithread' => Plack::Util::FALSE,
  59 + 'psgi.multiprocess' => Plack::Util::TRUE,
  60 + 'psgi.run_once' => Plack::Util::FALSE,
  61 + 'psgi.streaming' => Plack::Util::TRUE,
  62 + 'psgi.nonblocking' => Plack::Util::FALSE,
  63 + 'psgix.harakiri' => Plack::Util::TRUE,
  64 + 'psgix.cleanup' => Plack::Util::TRUE,
  65 + 'psgix.cleanup.handlers' => [],
65 66 };
66 67
67 68 if (defined(my $HTTP_AUTHORIZATION = $r->headers_in->{Authorization})) {
@@ -90,14 +91,37 @@ sub call_app {
90 91 }
91 92
92 93 if (@{ $env->{'psgix.cleanup.handlers'} }) {
  94 + my %ENV_copy_for_PerlCleanupHandler = %ENV;
  95 + # This is referred to in the comment below
93 96 $r->push_handlers(
94 97 PerlCleanupHandler => sub {
95   - for my $cleanup_handler (@{ $env->{'psgix.cleanup.handlers'} }) {
96   - $cleanup_handler->($env);
97   - }
98   -
99   - if ($env->{'psgix.harakiri.commit'}) {
100   - $r->child_terminate;
  98 + my $do_cleanup_handlers = sub {
  99 + for my $cleanup_handler (@{ $env->{'psgix.cleanup.handlers'} }) {
  100 + $cleanup_handler->($env);
  101 + }
  102 +
  103 + if ($env->{'psgix.harakiri.commit'}) {
  104 + $r->child_terminate;
  105 + }
  106 +
  107 + return;
  108 + };
  109 +
  110 + if (keys %ENV) {
  111 + $do_cleanup_handlers->();
  112 + } else {
  113 + # Sometimes Apache will seemingly empty out %ENV,
  114 + # i.e. if we were to dump out %ENV when we assign to
  115 + # %ENV_copy_for_PerlCleanupHandler above we'd get a
  116 + # populated %ENV, but it'll be empty by the time we
  117 + # get here.
  118 + #
  119 + # I haven't tracked down *why* this happens, so this
  120 + # might be cargo-culting around something, but this
  121 + # workaround works for me and isn't too expensive.
  122 + local %ENV = %ENV_copy_for_PerlCleanupHandler;
  123 +
  124 + $do_cleanup_handlers->();
101 125 }
102 126 },
103 127 );

Tip: You can add notes to lines in a file. Hover to the left of a line to make a note

Something went wrong with that request. Please try again.