Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:95020
Mailing-List: contact internals-help@lists.php.net; run by ezmlm
Received-SPF: pass (pb1.pair.com: domain gmx.de designates 212.227.17.22 as permitted sender)
To: internals@lists.php.net, Nikita Popov <nikita.ppv@gmail.com>
References: <ed7bc05c-90a3-c21a-b43e-186221a73006@gmx.de>
 <CAF+90c9VBtbveOtbK7rRGAd+DJtq7dSzR4aYNo2q2vOs5624kQ@mail.gmail.com>
 <9e9c440e-05d2-a452-556e-a6c62a9f195e@gmx.de>
 <208237ee-a22a-97b7-be63-f7cba986b137@fleshgrinder.com>
Message-ID: <16fdbdc7-bad4-f978-32e9-e4bf7443e592@gmx.de>
Date: Wed, 10 Aug 2016 20:23:57 +0200
User-Agent: Mozilla/5.0 (Windows NT 10.0; WOW64; rv:45.0) Gecko/20100101
 Thunderbird/45.2.0
MIME-Version: 1.0
In-Reply-To: <208237ee-a22a-97b7-be63-f7cba986b137@fleshgrinder.com>
Content-Type: text/plain; charset=utf-8
Content-Transfer-Encoding: 8bit
Subject: Re: [PHP-DEV] GC issue wrt. to resource <-> object cycles?
From: cmbecker69@gmx.de ("Christoph M. Becker")

On 10.08.2016 at 19:34, Fleshgrinder wrote:

> On 8/10/2016 4:17 PM, Christoph M. Becker wrote:
>> On 10.08.2016 at 12:59, Nikita Popov wrote:
>>
>>> The simplest way to fix ext/xml in particular is probably to migrate it to
>>> use an object instead of a resource.
>>
>> I guess that is the best action in the long run (i.e. 7.2+), but that
>> would of course cause a BC break, so probably it's best to document that
>> the user has to break the cycle manually, if xml_set_object() is used
>> (7.0, 7.1).
>>
>> Wrt. to PHP 5.6 it appears there are no issues at all.  While the code
>> out-commented by Thies is still there (which should be removed to avoid
>> further confusion), in the following a (shallow) copy of the object is
>> made, and apparently due to the additional refcount in the object store
>> of PHP 5, everything is okay.
>>
>> Another alternative for PHP 7.2 might be to drop (after deprecation)
>> xml_set_object(); actually, it seems to me this function is a relict of
>> pre PHP 5.  Cf. example #1[1].  Instead of doing:
>>
>>   xml_set_object($this->parser, $this);
>>   xml_set_element_handler($this->parser, "tag_open", "tag_close");
>>
>> one could simply do:
>>
>>   xml_set_element_handler($this->parser,
>>       [$this, "tag_open"], [$this, "tag_close]);
>>
>> [1]
>> <http://php.net/manual/en/function.xml-set-object.php#refsect1-function.xml-set-object-examples>
> 
> It does not have to be a BC if we simply introduce a new API for this
> extension and keep the old one as is. The whole set of functions just
> cries out loud for a class that encapsulates the state of the parser and
> both the utf8_* functions simply do not belong in this extension.

That might be a good idea, even if we already have the XMLReader.  And
yes, the utf8_* functions don't really belong into ext/xml; frankly, I
think they don't belong anywhere – we already have iconv, mbstring and
recode which offer a subset of utf8_decode()/utf8_encode.  OTOH,
utf8_decode()/_encode() appear to be often used (IIRC there are a lot of
user notes suggesting to use these functions).

-- 
Christoph M. Becker