Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:105525
To: Dan Ackroyd <Danack@basereality.com>, Nikita Popov <nikita.ppv@gmail.com>
Cc: PHP internals <internals@lists.php.net>
References: <CAF+90c-3TcEME_993vxvbhgCsKCrVVMSLobjrc=-EUZOmxYRYg@mail.gmail.com>
 <CA+kxMuSPDckN+YDN-PuqU-YXKm08fyTWh_7yK0YecTSJOFLfvA@mail.gmail.com>
Openpgp: preference=signencrypt
Autocrypt: addr=smalyshev@gmail.com; prefer-encrypt=mutual; keydata=
 mQMuBE9mqaARCACFSqcGmNunkjQQu3X+yXnTmFeEkvM4JXZTOBdR8aEevNGmmFEfyvjaDjWi
 9hcwp4E/lYtC+P7VsVjM1OSX9eq0jC/lGL0ZyRXek+mNy0n5H1NSuTpf9Y18LMqhc4G+RU+L
 cNiZ9K0DJuOOvNLPxW7OHZguxb3wdKPXNVa2jyRfJAKm2uaJJMT1mTmFT9a0Q8SKr+mUrrJk
 uG0H2o6SzrKt8Wwoint1eh67zVsJaJtQFchnEZnlawIcqP2yC4nLGR3MkubowxoEBYCZet18
 aHVVRbvpG2Qtob8Lu5xrsGbmXymTkHTdpvkfcJFADa8MzOL90zOxXwbGfbIZOlh5En8jAQCX
 lfnx2eQL3BSW/6XANa51dbWiEp1d1BAkpGKtZvlk0Qf+M9WAi+9aXMe3xP5krxtgnRNUf2WN
 6Zdy2MxL1RRJCFbytLhl0ronC49BsGYVGshdEH8xhBbiIOJKuVZ/DTl9bEm7P9c7CC7iJyVC
 khUAhouH6xzZQNLR+RU+QebYzXypVfl99Qk7EdMmr/WAZCHLuvanyqepC5EBsa3VnAfQemSN
 oBeGBKWWLiOsPjvS72+y1z4RUMAfXHn4l/sFMt8zt7/74AmJPwZquV41p4mPO12V4+xPyc6R
 sB84sfsk2QVivU8w8AkvGQeYjXoz7Iwao95+fWteVzZ36KRQvUckP8pGjHlDXnHxJ0HI1I/k
 OBZSjwRwUf0dd73y6erPhbLk+gf+NdI3H9KGJBzG5/rVyWKwUeQ9d5ud4jTJRkQGvAP5pg76
 vEa9dogbpe4W5Z+0BfbiJSnQmQWSHiZddj/t33ptbup44Ck6ZTgdlmFYMLF1hR47PIZTDKER
 EuKYGci/vq8snZvEJP9YCw/TtiHcMdrMKcY/+Lp8lQO0GHLPB9glVhnC0db6l1Xpg1CMI8/R
 ozBMcij30EgATggC/y2zbiqAFoS9FN9nXPbe4phStqABEyeZ+nXudt7PUYTjVgcrqo8bHZCi
 sBobWC7OnKyUzxVxzUeuPkIfmZuzkLaMw2McQdvwwsNvQ0DzaLP30c1Xsm/7EIYJcOWpzlVJ
 5QrdmE0/BbQyU3RhbmlzbGF2IE1hbHlzaGV2IChQSFAga2V5KSA8c21hbHlzaGV2QGdtYWls
 LmNvbT6IegQTEQgAIgUCT2aqtAIbAwYLCQgHAwIGFQgCCQoLBBYCAwECHgECF4AACgkQL3lW
 vF2gS12XMwD9HuRIolSwIK77u8EY461y2u6sbX36n5/uo/LDQuxoi3sA/0MvpnvzOhv9Iufv
 vsZEj3E7i3h+iD5648YMwfTFCij+uQINBE9mqaAQCADfZPMpjZkkGZj3BY/7ApoLq4mwqzbh
 +CpLXwNn20tFNvSXfb8RdeXvVEb7Scx+W9qYpiaun2iXJgCVH8fgpZpR856ulT1q6uCG++CX
 ubEvip/eJkZl93/84h04KQJwsgOrAh0Om3OePRn8Pr+++0LNS0EL8uX/YHeTOGOnnmTqYTey
 SBVFdov6L4mepddfjekicKQqhL7mZh/xuq29JijT0uNNX8v4vDWQDu5dlAcdd+uB3gcXMD/P
 ginD11zp+6wtrWCm/+yBqpvDwXQX5PGUnwvbRfl7Ay3MmwmoXiecZMg0dwTSc7e0lhB4HGRH
 ZdBMJB4rHUVGdzqujK/ctOvrAAMFB/0Utb76Qe6sCMlHxVAmeE/fbo7Pi05btZ/x01r67dHf
 aMSP0riCKJ7M0OW+jAXtu9+z/BVnYisW67WWfxl2cS5tZDgiHgJARXWUOO72+sScHP8KQmTl
 1z16gyKbwY3SmyBkwcpOL35nhUWNLy93syPoY6sZUTikr2bZYukHDQ33XBPs4e6MbWKfsa9q
 aVmnlOF3k5UqChjutfHaEa4Q7VP4wBIpphHBi9MI16oJIzzBPbGl2uoedjwiZ6QeQZnSuOVY
 ZxU2d3lRA8PrtfFN1VSlpEm/VcAvtieHUYWHN0wOu+cp3Slr5XJVNjTjJhl28SlinMME54mK
 AGf2Ldr/dRwXiGEEGBEIAAkFAk9mqaACGwwACgkQL3lWvF2gS126EQD/VVd3FgjLKglClRQP
 zdfU847tqDK4zJjbmRv5vLLwoE0A+wbrQs7jVGU3NrS0AIl5vUmewpp2BKzSkepy23nWmejw
Message-ID: <2b141597-e68e-1843-51f5-16d6ed8c41fa@gmail.com>
Date: Tue, 30 Apr 2019 11:49:42 -0700
User-Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.13; rv:60.0)
 Gecko/20100101 Thunderbird/60.6.1
MIME-Version: 1.0
In-Reply-To: <CA+kxMuSPDckN+YDN-PuqU-YXKm08fyTWh_7yK0YecTSJOFLfvA@mail.gmail.com>
Content-Type: text/plain; charset=utf-8
Content-Language: en-US
Content-Transfer-Encoding: 7bit
Subject: Re: [PHP-DEV] [RFC] Allow throwing exceptions from __toString()
From: smalyshev@gmail.com (Stanislav Malyshev)

Hi!

> Please could someone anyone who understands the implications for what
> this means for the internals of the PHP engine provide some detailed
> reasons for why this shouldn't be approved, if there are any reasons.
> 
> Otherwise voters might 'irresponsibly' vote yes, without being aware of them.

This is the tricky one because it gets into one of the corner cases of
the engine that has never worked perfectly. We have a bunch of places
where we can get some code executed in the middle of other code
executed, and it never worked particularly well. One of such places is
(un)serialization, another is error and exception handling. Generally,
running code from the guts of the engine is dangerous, because there
might be stale pointers, race conditions (not the parallel kind but the
kind "we checked it above, but below it may no longer be the case
because we run code in between"), shared mutable state, etc. and it's
nigh impossible to ensure nothing gets broken by it.

That said, the work done on the patch is super impressive. I am
reasonably sure that Nikita caught as many instances of something going
wrong when string conversion fails as humanly possible. And I agree that
banning exceptions didn't actually solve the problem anyway, because
error handlers present the same exactly issue - exceptions are just much
more prominent and immediate in their effects, but with some hostile
coding - or even some inadvertently convoluted one - you'd run into the
same issues again. Some extensions will probably have some trouble -
though with high likelyhood they already do anyway.

I think since the reason why it was banned is largely gone with this
patch it's a good step forward. There are probably still some corner
cases that would not work properly - I am especially worried about
variables left un-initialized in places where they are expected to be
initialized - like internal classes - but I think the awesome job done
on this patch is a good step forward and unless we discover some
critical issue blocking it, it makes sense to do it.

I plan to try and review the patch in my copious free time, but so far I
do not have any objections beyond "yet another place where things could
go wrong". Since that place effectively already existed, I think this
RFC is good.

-- 
Stas Malyshev
smalyshev@gmail.com