Philippe M. Chiasson wrote:
Stas Bekman wrote:
Philippe M. Chiasson wrote:
Would it be better to localize $@ instead of copying and restoring it?
I wish we could do that, and it was the first thing I attempted.
The problem is that if we do that, even conditionnaly only when $@ is set
on entry, it means that any _real_ errors triggered during the execution
of the handler (setting $@) will also be lost and replaced with the previous
value for [EMAIL PROTECTED]
That's why we need to save $@ if it was set, and only restore if nothing failed during handler execution, otherwise we squish the new [EMAIL PROTECTED]
I see. So in case the filter has failed, and the response handler has failed we won't see the error from a response handler. So that solves only half a problem, as we still lose handlers $@
Ah, I see, I didn't consider that failure case.
That's why I thought that it'll be better to find a different place to localize $@, i.e. outside the scope of the code that runs the handler and checks $@ on return. but this is probably not quite feasible. How about making a special case for filters (inside the filters code).
Yes, this $@ save/restore logic could be added to filters only. But, still, in the case of a failing response handler & filter handler, you'd lose the error from the filter.
At least, here is a patch that does the same thing, but only for filters.
Maybe we need to be smarter about this a bit and somehow concatenate the errors ?
-- -------------------------------------------------------------------------------- Philippe M. Chiasson m/gozer\@(apache|cpan|ectoplasm)\.org/ GPG KeyID : 88C3A5A5 http://gozer.ectoplasm.org/ F9BF E0C2 480E 7680 1AE5 3631 CB32 A107 88C3A5A5
Index: Changes
===================================================================
RCS file: /home/cvs/modperl-2.0/Changes,v
retrieving revision 1.431
diff -u -I$Id -r1.431 Changes
--- Changes 9 Aug 2004 21:42:34 -0000 1.431
+++ Changes 9 Aug 2004 22:08:11 -0000
@@ -12,6 +12,9 @@
=item 1.99_15-dev
+Filters should not reset $@ if it was already set before
+invocation [Gozer]
+
Apache::compat server_root_relative now correctly handles absolute
paths like ap_server_root_relative does [Gozer]
Index: src/modules/perl/modperl_filter.c
===================================================================
RCS file: /home/cvs/modperl-2.0/src/modules/perl/modperl_filter.c,v
retrieving revision 1.93
diff -u -I$Id -r1.93 modperl_filter.c
--- src/modules/perl/modperl_filter.c 2 Jun 2004 21:35:58 -0000 1.93
+++ src/modules/perl/modperl_filter.c 9 Aug 2004 22:08:11 -0000
@@ -432,6 +432,7 @@
int modperl_run_filter(modperl_filter_t *filter)
{
AV *args = Nullav;
+ SV *errsv = Nullsv;
int status;
modperl_handler_t *handler =
((modperl_filter_ctx_t *)filter->f->ctx)->handler;
@@ -443,6 +444,15 @@
MP_dINTERP_SELECT(r, c, s);
+ if (SvTRUE(ERRSV)) { /* Save current value of $@ if set */
+ errsv = newSVsv(ERRSV);
+ MP_TRACE_f(MP_FUNC, "Saving [EMAIL PROTECTED]'%s' for %s filter",
+ SvPVX(errsv),
+ modperl_handler_name(handler)
+ );
+ }
+
+
modperl_handler_make_args(aTHX_ &args,
"Apache::Filter", filter->f,
"APR::Brigade",
@@ -503,6 +513,14 @@
MP_TRACE_f(MP_FUNC, MP_FILTER_NAME_FORMAT
"return: %d\n", modperl_handler_name(handler), status);
+ if (errsv && !SvTRUE(ERRSV)) {
+ sv_setsv(ERRSV, errsv);
+ MP_TRACE_f(MP_FUNC, "Restoring [EMAIL PROTECTED]'%s' for %s handler",
+ SvPVX(errsv),
+ modperl_handler_name(handler)
+ );
+ }
+
return status;
}
Index: t/filter/TestFilter/out_str_eval.pm
===================================================================
RCS file: /home/cvs/modperl-2.0/t/filter/TestFilter/out_str_eval.pm,v
retrieving revision 1.3
diff -u -I$Id -r1.3 out_str_eval.pm
--- t/filter/TestFilter/out_str_eval.pm 3 Jul 2004 18:45:46 -0000 1.3
+++ t/filter/TestFilter/out_str_eval.pm 9 Aug 2004 22:08:11 -0000
@@ -25,9 +25,8 @@
sub response {
my $r = shift;
- plan $r, tests => 1, todo => [1];
- # XXX: see if we can fix filter handlers to restore the original
- # $@ when the callback completes
+ plan $r, tests => 1;
+ #Filters do not reset $@
eval { i_do_not_exist_really_i_do_not() };
# trigger the filter invocation, before using $@
$r->print("# whatever");
Index: todo/release
===================================================================
RCS file: /home/cvs/modperl-2.0/todo/release,v
retrieving revision 1.41
diff -u -I$Id -r1.41 release
--- todo/release 9 Aug 2004 21:42:35 -0000 1.41
+++ todo/release 9 Aug 2004 22:08:11 -0000
@@ -8,11 +8,6 @@
http://marc.theaimsgroup.com/?l=apache-modperl-dev&m=108967266419527&w=2
owner: gozer
-* filters reset $@ generated by eval, see if we can fix that. The TODO
- test: TestFilter::out_str_eval presents the case
- The description is here:
- http://marc.theaimsgroup.com/?l=apache-modperl-dev&m=108639632031457&w=2
-
* on windows $pool->clean, followed by $pool->destroy breaks other tests
See test TestAPR::pool
http://marc.theaimsgroup.com/?l=apache-modperl-dev&m=108547894817083&w=2
signature.asc
Description: OpenPGP digital signature
