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