1155009 - Cache::Memcached causes email_in.pl to stop working

Status: RESOLVED FIXED

(Bugzilla :: Incoming Email, defect)

Description

[Frédéric Buclin]

Due to |$SIG{__DIE__} = \&die_handler| in email_in.pl line 492, the script dies everytime Cache::Memcached calls eval() to check which optional modules are installed. For instance:

  $HAVE_SOCKET6 = eval "use Socket6 qw(AF_INET6 PF_INET6); 1;"

causes:

  Can't locate Socket6.pm in @INC


And if I install Socket6, then:

  my $HAVE_XS = eval "use Cache::Memcached::GetParserXS; 1;"

causes:

  Can't locate Cache/Memcached/GetParserXS.pm in @INC

etc...

Comment 1

[Frédéric Buclin]

Attachment: patch, v1

Comment 2

[:glob ✱]

Comment on attachment 8593144 [details] [diff] [review]
patch, v1

Review of attachment 8593144 [details] [diff] [review]:
-----------------------------------------------------------------

that script's die_handler already has a check for eval blocks, so i'm not convinced that's the issue, and it appears your fix will causes an mta to send a failure notice.

::: email_in.pl
@@ -480,5 @@
>         MessageToMTA($reply->as_string);
>      }
> -    print STDERR "$msg\n";
> -    # We exit with a successful value, because we don't want the MTA
> -    # to *also* send a failure notice.

the comment indicates that die'ing would be bad, however that's what this patch does.

Comment 3

[David Lawrence [:dkl]]

glob do you have a moment to look at a revised patch for this? we need this for 5.0 and then we can proceed with the release.

Comment 4

[:glob ✱]

(In reply to David Lawrence [:dkl] from comment #3)
> glob do you have a moment to look at a revised patch for this? we need this
> for 5.0 and then we can proceed with the release.

when i have some time to get things set up i could have a look.

Comment 5

[Frédéric Buclin]

Attachment: patch, v2

No idea why, but it seems that $^S is lost somewhere, and so I fallback to detect "eval" in the name of the filename. Definitely a hack, but I cannot figure the root cause of the problem.

Comment 6

[Frédéric Buclin]

Attachment: patch, v2.1

Better fix, as $^S == 0 is a valid state.

Comment 7

[:glob ✱]

Comment on attachment 8604356 [details] [diff] [review]
patch, v2.1

Review of attachment 8604356 [details] [diff] [review]:
-----------------------------------------------------------------

thanks for revisiting this lpsolit.

i'm fine with this approach, but we should probably check the entire call stack not just the immediate caller.
Bugzilla::Error::_in_eval() shows how to do this.

Comment 8

[Frédéric Buclin]

Attachment: patch, v3

Comment 9

[:glob ✱]

Comment on attachment 8604759 [details] [diff] [review]
patch, v3

Review of attachment 8604759 [details] [diff] [review]:
-----------------------------------------------------------------

r=glob

Comment 10

[Frédéric Buclin]

To ssh://[e-mail]/bugzilla/bugzilla.git
   3ff9b3e..2102acc  master -> master

To ssh://[e-mail]/bugzilla/bugzilla.git
   3d2b724..f261808  5.0 -> 5.0