On 1 February 2018 at 10:35, demerphq <demerphq@gmail.com> wrote: > 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. Patch pushed as 4764858cb80e76fdba33cc1b3be8fcdef26df754 Note this ticket is a duplicate of Perl #52610 Yves -- perl -Mre=debug -e "/just|another|perl|hacker/"Thread Previous | Thread Next