From 3480c30674f239185ec3a362c2f45d1fc591505f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=86var=20Arnfj=C3=B6r=C3=B0=20Bjarmason?= Date: Tue, 25 Feb 2014 17:47:12 +0000 Subject: [PATCH] perl: Don't run BEGIN blocks twice in the provided *.psgi The psgi loaded was calling perl_parse() with the script ostensibly to set up xsinit. However it would also call perl_parse() with the path to our *.psgi file, whith the result that any BEGIN block in the *.psgi file would be run twice, but anything outside BEGIN blocks would only run once. This means that any code within explicit BEGIN blocks will run twice, and any "use" statement in the *.psgi file will run its import() routine twice, but due to the module being in %INC already we won't actually compile things twice. The previous behavior dates all the way back to the initial introduction of the PSGI plugin in 299fd9c. Then when support for local::lib was added in 7cbe751 we initially did a perl_eval_pv() of a "use" statement like I'm doing here again now, but later on in 1561dd3 changed it to call perl_parse with the commit message "another PSGI loading fix". Since there's no info on what that fixed or what was broken before I have no idea if I'm introducing a regression here, but I don't see why this way of loding local::lib shouldn't work, and it correctly munges @INC for me when I try it. We may still have this bug in the remaining perl_parse() calls that remain for supporting "preinit" and "mule". I haven't tested those modes (I don't use them), but when we load the Perl apps we should only perl_parse() once with -e1, and then perl_eval_pv() to actually load the application. We should not call perl_parse() on code that we're just about to perl_eval_pv(), or we'll run into this bug. To test this just run: ./uwsgi --http 127.0.0.1:8080 --psgi ./t/perl/test.psgi It'll no longer PANIC on the BEGIN block being run twice now, at least in that simplistic non-"preinit" non-"mule" mode. --- plugins/psgi/psgi_loader.c | 33 ++++++++++++++------------------- t/perl/test.psgi | 4 ++++ 2 files changed, 18 insertions(+), 19 deletions(-) diff --git a/plugins/psgi/psgi_loader.c b/plugins/psgi/psgi_loader.c index f8cc4b8f..dceaf1b8 100644 --- a/plugins/psgi/psgi_loader.c +++ b/plugins/psgi/psgi_loader.c @@ -368,31 +368,26 @@ int init_psgi_app(struct wsgi_request *wsgi_req, char *app, uint16_t app_len, Pe uperl.tmp_current_i = i; - - if (uperl.locallib) { - uwsgi_log("using %s as local::lib directory\n", uperl.locallib); - uperl.embedding[1] = uwsgi_concat2("-Mlocal::lib=", uperl.locallib); - uperl.embedding[2] = app_name; - if (perl_parse(interpreters[i], xs_init, 3, uperl.embedding, NULL)) { - // what to do here ? i hope no-one will use threads with dynamic apps... but clear the whole stuff... - free(uperl.embedding[1]); - uperl.embedding[1] = app_name; - free(callables); - uwsgi_perl_free_stashes(); - goto clear; - } - free(uperl.embedding[1]); - uperl.embedding[1] = app_name; - } - else { - if (perl_parse(interpreters[i], xs_init, 2, uperl.embedding, NULL)) { + // We need to initialize the interpreter to execute + // our xs_init hook, but we're *not* calling it with + // uperl.embedding as an argument so we won't execute + // BEGIN blocks in app_name twice. + { + char *perl_init_arg[] = { "", "-e", "1" }; + if (perl_parse(interpreters[i], xs_init, 3, perl_init_arg, NULL)) { // what to do here ? i hope no-one will use threads with dynamic apps... but clear the whole stuff... free(callables); uwsgi_perl_free_stashes(); goto clear; - } + } } + if (uperl.locallib) { + uwsgi_log("using %s as local::lib directory\n", uperl.locallib); + char *local_lib_use = uwsgi_concat3("use local::lib qw(", uperl.locallib, ");"); + perl_eval_pv(local_lib_use, 1); + free(local_lib_use); + } perl_eval_pv("use IO::Handle;", 1); perl_eval_pv("use IO::File;", 1); perl_eval_pv("use IO::Socket;", 1); diff --git a/t/perl/test.psgi b/t/perl/test.psgi index 1e7a6bc6..2535e36a 100644 --- a/t/perl/test.psgi +++ b/t/perl/test.psgi @@ -1,5 +1,9 @@ use strict; use warnings; +BEGIN { + die "PANIC: We should only load this once" if ++$main::count_BEGIN > 1; +} +die "PANIC: We should only run this once" if ++$main::count_runs > 1; uwsgi::register_rpc('hello', sub { my ($one, $two, $three) = @_;