Newsgroups: php.internals Path: news.php.net Xref: news.php.net php.internals:70571 Return-Path: Mailing-List: contact internals-help@lists.php.net; run by ezmlm Delivered-To: mailing list internals@lists.php.net Received: (qmail 38654 invoked from network); 10 Dec 2013 15:56:29 -0000 Received: from unknown (HELO lists.php.net) (127.0.0.1) by localhost with SMTP; 10 Dec 2013 15:56:29 -0000 Authentication-Results: pb1.pair.com header.from=nikita.ppv@gmail.com; sender-id=pass Authentication-Results: pb1.pair.com smtp.mail=nikita.ppv@gmail.com; spf=pass; sender-id=pass Received-SPF: pass (pb1.pair.com: domain gmail.com designates 209.85.219.50 as permitted sender) X-PHP-List-Original-Sender: nikita.ppv@gmail.com X-Host-Fingerprint: 209.85.219.50 mail-oa0-f50.google.com Received: from [209.85.219.50] ([209.85.219.50:62013] helo=mail-oa0-f50.google.com) by pb1.pair.com (ecelerity 2.1.1.9-wez r(12769M)) with ESMTP id A6/E4-04453-CA937A25 for ; Tue, 10 Dec 2013 10:56:29 -0500 Received: by mail-oa0-f50.google.com with SMTP id n16so5803890oag.9 for ; Tue, 10 Dec 2013 07:56:25 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20120113; h=mime-version:in-reply-to:references:date:message-id:subject:from:to :cc:content-type; bh=CDPJ8o4p3lXcBClPsQ+OrrxywCXV0XkCFyqT7d0bjDs=; b=GZU9wDzn7bA8yeoaLrC6X2P86WCpwKTioFXDCvmrk0qEXRzl9J7IGTbTV0IVYTwQ39 Wijb7GkRMowlY4LQ9aEnfKr69dec1CjCiMgKU2ndu/FDiV06tQX2WfSNVAULH+NcXvl0 vTa2lfKi3vzyJQRthy635JI4f9AXX4NtjsFHlQRJtlCp7v16HVfOuMcFufe7GrsDT+E0 Y+TXjUxhcbREbfIlkurhXqng41eYQGY2TwJ7qxKSmGGu6MpHjJ2FOB3vzerlaj5cp1Z8 1R3DNK9ziSgHzMbJC/yyU4sT8EY4Xt3slM516IQPEcU5oDXiCWqWAe0je2jDmoR3RjZi wXsw== MIME-Version: 1.0 X-Received: by 10.182.40.201 with SMTP id z9mr16970214obk.45.1386690985530; Tue, 10 Dec 2013 07:56:25 -0800 (PST) Received: by 10.182.54.112 with HTTP; Tue, 10 Dec 2013 07:56:25 -0800 (PST) In-Reply-To: References: <6dc040b1d4917ab616951694d3f37575@mail.gmail.com> Date: Tue, 10 Dec 2013 16:56:25 +0100 Message-ID: To: Zeev Suraski Cc: Philip Sturgeon , PHP internals Content-Type: multipart/alternative; boundary=001a11c33b029b1d0904ed3027ad Subject: Re: [PHP-DEV] [VOTE] Allowing use of exceptions in the engine From: nikita.ppv@gmail.com (Nikita Popov) --001a11c33b029b1d0904ed3027ad Content-Type: text/plain; charset=ISO-8859-1 Thanks for the detailed reply Zeev. On Tue, Dec 10, 2013 at 1:55 PM, Zeev Suraski wrote: > Nikita, Philip, see below: > > > On Mon, Dec 9, 2013 at 11:41 AM, Nikita Popov > > wrote: > > > You can't convert fatal errors to exceptions using an error handler. > > > That's why I'm targeting fatal errors here and not all errors in > > > general. While I'm not a big fan of PHP's error system in general, > > > it's easy enough to turn warnings or notices into exceptions via error > > > handlers. But for fatal errors that's just not possible. > > That's correct, but you seem to be ignoring the reason for that. It's not > an arbitrary decision, but rather, the semantics of what fatal errors are. > They're fatal, not recoverable (with the reason being that the > engine/extension/whatever may be in an unstable state), and therefore it > should not be possible to use *any* mechanism to recover from them, be > them error handlers or exceptions. > Ostensibly, yes. Practically that's just not true. Most (nearly all) fatal errors occur in situation where there is no global engine instability, but only some local *inconvenience*. Like, I tried to fetch this zval, but got NULL instead (because it's a string offset), what do I do now? No instability, just not pleasant to continue from here. I have already ported approximately 40% of the fatal errors in the engine to use exceptions. I can assure you that all of those have nothing to do with engine instability and changing them to exception was mostly trivial (the hard part is adjusting phpt outputs...) I'm pretty sure that most of the remaining ~100 fatal errors will be just as easy to convert. I just looked through the list to find the ones which *can't* be converted: * "Invalid opcode %d/%d/%d" => This error only occurs in case of a serious skrewup from our side. Shouldn't occur in practice * "Arrived at end of main loop which shouldn't happen" => This is effectively dead code, just a debugging message. * "Balloc() failed to allocate memory" and "Balloc() allocation exceeds list boundary" => Those two are from zend_strtod. Again, you won't see them in practice (unless you try to parse a 100K digit float maybe. don't know the exact background here) * "Error installing signal handler for %d" => From zend_signals, occurs when a sigaction call fails. This seems like a "just to be sure" check that shouldn't occur in practice. But in either case, this is only relevant for pcntl, as nothing else makes use of the function. * "Attempt to destruct pending exception" => (Not sure whether this is just a debugging error or whether it can really occur. We don't have a test for it at least) * "Method %s::__toString() must not throw an exception" * + 6 more exception related fatals (like exception must be object etc) => may be problematic to use exceptions for errors about exceptions (but I may be wrong here, it might well be that exceptions are safe here) * "Nesting level too deep - recursive dependency?" => from zend_hash. As this is used in a bunch of places, likely can't be changed. * "Class entry requested for an object without PHP class" => Doesn't occur in practice, because we always use objects with ce. * "Allowed memory size / Out of memory" => The obvious candidates ^^ * "Possible integer overflow in memory allocation" * "Maximum execution time of %d second%s exceeded" * "Corrupted fcall_info provided to zend_call_function()" => Once again, this one doesn't occur in practice, just debugging. That list has 17 errors that can't or likely can't be converted. Add 10 more and round up to 30 to account for stuff I might have missed. This still allows us to convert approximately 85% of the errors. Also note that at least 6 of those are really "debugging errors" that do not practically occur. The most common ones from the list will be the "Allowed memory size" and "Maximum execution time" errors, but we knew from the start that those can't be changed. To also approach this from the other side, here are some examples of errors that I have already converted to exceptions (listing everything would blow things up a bit too much): * "Call to a member function %s() on a non-object" => Arguably the error that annoys people most * "Call to undefined method %s::%s()", "Call to undefined function %s()", "Class '%s' not found" + variations on the "not found" topic * "Cannot call abstract method %s::%s()" + a number of variations on the "cannot call" topic * "Class name must be a valid object or a string", "Method name must be a string" + variations on the "must be xyz" topic * "Cannot use string offset as an array" + a huge number of variations on the same thing. We have a lot of string offset related fatals ;) * "Cannot use [] for reading" * "Cannot pass parameter %d by reference" * "Cannot instantiate %s %s" * ... Hopefully you can now see that fatal errors - in most cases - do not cause engine instability and can be relatively easily recovered. Of course, some fatal errors will stay, no way around that. But we *can* convert a good portion of them. And many of the errors that are actually important to people (like "Call [...] on a non-object") can be (and are already) easily converted. > > To achieve what I want it would be enough to turn fatal errors into > > > recoverable fatal errors, that would already allow the error handler > > > approach. But that's a lot harder to do (and imho less usable) than > > > directly using exceptions. Keeping the engine stable after an > > > exceptions is relatively simple, the same can't be said about > recoverable > > fatals. > > While you pointed out in the RFC a specific example of a place where an > exception would be a bit easier to implement than an error, it's a very > specialized example - errors that happen on the void between two function > scopes - and doesn't hold true for most errors. I don't think it makes > sense to say it's significantly easier to turn fatal errors into > exceptions than turning fatal errors into recoverable errors. > Again, this just isn't true. If I say that converting fatals to recoverable errors is harder, I say that because I actually tried to do it before going with the exception approach. For exceptions the procedure is always the same: Free resources, then return (+ ensuring exception safety in the calling code, but that's usually a given). For recoverables things are a lot more complicated. Let's forget about the complex cases where function calls and stacks are involved, let's just take the very simplest fatal errors like "Cannot use string offset as an array". Those types of fatals occur whenever a GET_OPx_ZVAL_PTR_PTR call returns NULL. As said, with exceptions handling them is trivial, but what do you do in the case of recoverable? Do you allocate a new IS_NULL zval and work on that? Of course that would just produce a bunch of new warnings or errors down the row (also just allocating a zval may not be enough for the PTR_PTR case). It would be a lot better to make the result of the "whole" operation (rather than an individual fetch) an IS_NULL zval (without additional errors), but that's where things will get technically *very* complicated. With recoverables every single errors needs careful consideration to find some behavior where we can continue execution in some not-totally-meaningless way. With exceptions you just always follow the same dumb scheme ;) > Secondly, the patch radically changes execution behavior in case of > recoverable errors for people using error handlers and arguably in a very > bad way, effectively forcing them to start using exceptions if they want > to truly recover from the error. Otherwise, by the time the exception > turns into an error, there's nothing you can recover (you've already > bubbled up all the way to the global scope). That will break apps relying > on the current behavior. > Yes, there are concerns regarding E_RECOVERABLE_ERROR, that's why I added a separate voting options for it. If recoverable errors are your main issue (which seems so, given that most of your mail was about that), then that's your voting option ;) > Last, in the RFC it mentions that 'recoverable errors are hard to catch'. > That you're forced to use an error_handler. How are they any different > from any other error/warning type? Not any different. Please realize though that "Just use an error handler to turn everything into exceptions" is a somewhat idyllic view that fails when it comes to library inter-compatibility. If you are writing portable code you can't use a global error handler to convert everything to exceptions. Instead you need to create and restore an error handler in every single place where you want to make use of try/catch with a recoverable ;) Nikita --001a11c33b029b1d0904ed3027ad--