develooper Front page | perl.perl5.porters | Postings from February 2018

Re: [perl #131046] [PATCH] Carp: Do not crash when reading @DB::args

Thread Previous | Thread Next
From:
demerphq
Date:
February 1, 2018 09:35
Subject:
Re: [perl #131046] [PATCH] Carp: Do not crash when reading @DB::args
Message ID:
CANgJU+X100QRWt+0PJKUuRy-oRQwX9JicXh7HZ9G8sS9tRtEYg@mail.gmail.com
On 1 February 2018 at 10:17, Zefram <zefram@fysh.org> wrote:
> demerphq wrote:
>>                                        We can encounter a stack
>>refcounting bug /only/ because we are serializing the stack to report
>>some other exception.
>
> We can indeed, but that doesn't make it any more possible to work around
> the bug.  It does give you the opportunity to entirely avoid tickling
> these stack refcounting bugs, by not attempting to serialise arguments
> from the stack at all.  But if you do look at arguments on the stack,
> you're subject to the refcounting bug, and can't work around that.
>
>>This ticket isn't about fixing the stack refcounting, it is about not
>>dieing while trying to report some other issue.
>
> Yes, and that objective is not achievable.  The stack-not-refcounted
> bug cannot be reliably detected.

I really feel like you are missing the point. By adding the eval we do
not die in at least /some/ situations. That is all our users care
about. In most of the cases we are going to die a second later anyway.

If the stack refcounting bug leads to a SEGV, then so be it, there is
not much an end user can do about it. But we do not need to mess up
peoples stack traces and hide user thrown exceptions when we /can/
detect it and handle it gracefully, which as I said already /is
clearly happening at least some of the time, or I wouldn't be aware of
this issue at all/.

By that I mean the fact I know about this issue is because our error
reporting pipeline at Booking collects all errors via $SIG{__DIE__}
and then sends them over the network to centralized collectors. All of
the code involved to send the error is Perl. Thus intercepting these
errors does not appear to be harmful. So I see no reason why Carp
should not intercept them.

Your argument comes down to "we cant do this perfectly so we shouldn't
do it at all". To me that is the fallacy of the excluded middle.

Carp throwing trappable exceptions while serializing the stack is of
no use to anyone other than someone interested in debugging stack
refcounting issues, and losing valuable user data is an unacceptable
price to pay. If we can reduce the surface area of data loss then we
can and should do so.

With respect, unless you can provide a better argument or you decide
to invoke the Pumpking I will merge smoke-me/rt52610 (assuming it
passes tests). Leaving this as is is not in the
best interest of our user community.

cheers,
Yves


-- 
perl -Mre=debug -e "/just|another|perl|hacker/"

Thread Previous | Thread Next


nntp.perl.org: Perl Programming lists via nntp and http.
Comments to Ask Bjørn Hansen at ask@perl.org | Group listing | About