Everything

php

zend_list_delete() doesn't.

php.internals

Sara Golemon

20 years ago

Certain things you take for granted:
The sun will come up.
Politicians are dishonest.
Resource destruction functions (fclose(), mysql_disconnect(), etc...) will
actually shutdown their resources.

Guess which of those three you can't count on.

Seems that zend_list_delete(), which I (and many other extension writers)
have relied on to actually delete list_entry objects from the resource hash,
doesn't actually delete so much as it delref's.  In general use cases this
isn't really a problem since most resource IDs are only referenced once (by
the zval* they were initially stored into).  So calling zend_list_delete()
in those cases actually does destroy it since --le->refcount == 0.

Using a very simple (and not entirely unlikely) bit of userspace code to
force zval separation however, means that the first call to
zend_list_delete() won't actually delete the resource.  Consider this
reproduction snippet:

<?php
$fp1 = fopen('test', 'w');
$fp2 = $fp1; /* Still a single zval* and le->refcount == 1 */
$fp3 = &$fp1; /* Two zval*s now and le->refcount == 2 */

fclose($fp1);

/* le->refcount now == 1, and the file is still open
File operations on $fp1, $fp2, or $fp3 will all succeed */
?>

Calling either unset($fp1); or unset($fp3); at this point would not trigger
the destruction, however calling either both of them or just unset($fp2);
alone would.  Calling fclose() a second time on any of the variables would
also work.

This behavior isn't unique to fclose(), several extensions which implement
the resource type rely on zend_list_delete() in this way, and the specific
naming of this function makes me a little curious:  Did zend_list_delete()
used to behave differently?  Is there a specific motivation against
modifying it to act like a genuine delete, then implementing a
zend_list_delref() which behaves like the current zend_list_delete(), and
modifying zval_dtor() implementations to use that instead.

Am I the only one mislead by the purpose of zend_list_delete()?

-Sara

Stanislav Malyshev

20 years ago

SG>><?php
SG>> $fp1 = fopen('test', 'w');
SG>> $fp2 = $fp1; /* Still a single zval* and le->refcount == 1 */
SG>> $fp3 = &$fp1; /* Two zval*s now and le->refcount == 2 */
SG>>
SG>> fclose($fp1);

I think this was discussed before, though I may be mistaken. The problem
here is that you expect fclose to kill the resource, however $fp2 still
refers it. Looks like there should be two operations for resource - one is
usual delref called when variable is destroyed and second is "hard kill"
called when something like fclose is used and invalidating all referring
variables. I remember there were some issues with that, but I can't
rememebr what they were...

--
Stanislav Malyshev, Zend Products Engineer
stas@zend.com  http://www.zend.com/ +972-3-6139665 ext.115

Wez Furlong

20 years ago

On 9/8/05, Stanislav Malyshev <stas@zend.com> wrote:

> SG>><?php
> SG>> $fp1 = fopen('test', 'w');
> SG>> $fp2 = $fp1; /* Still a single zval* and le->refcount == 1 */
> SG>> $fp3 = &$fp1; /* Two zval*s now and le->refcount == 2 */
> SG>>
> SG>> fclose($fp1);
>
> I think this was discussed before, though I may be mistaken.

We talked about this for the revised oci8 code; we adopted the
practice of ZVAL_NULL()ing the zval just after we zend_list_delete()
it, because the updated oci8 codebase can return zvals that reference
the same underlying list resource.

You could arrive at a similar kind of problem that Sara pointed out by
doing this:

$a = oci_connect(...);
$b = oci_connect(...); // $b is a different zval from $a, but refs the
same resource

oci_close($a);  // kills $a, but $b still owns a ref
oci_close($a);  // since $a still contains the resource id, this now
kills the resource,
// breaking $b

We made oci_close() set $a to NULL before returning, to avoid this
refcount skewing.

> The problem
> here is that you expect fclose to kill the resource, however $fp2 still
> refers it. Looks like there should be two operations for resource - one is
> usual delref called when variable is destroyed and second is "hard kill"
> called when something like fclose is used and invalidating all referring
> variables. I remember there were some issues with that, but I can't
> rememebr what they were...

IMO, "hard kill" doesn't really fit in a system that uses reference
counts; you either use reference counts, or don't.  There are cases
(particularly in streams) where we hold the resource ID to guarantee
that a pointer remains valid.  Hard kill would break the reference
counting contract and lead to a segv.

--Wez.

Stanislav Malyshev

20 years ago

WF>>IMO, "hard kill" doesn't really fit in a system that uses reference
WF>>counts; you either use reference counts, or don't.  There are cases

Obviously, we have here conflicting requirements - on the one side,
fclose($a) should invalidate resourse, on the other side, oci_close($a)
should not. I'm not sure it is possible to satisfy both of these
requirements...

--
Stanislav Malyshev, Zend Products Engineer
stas@zend.com  http://www.zend.com/ +972-3-6139665 ext.115

Andi Gutmans

20 years ago

At 07:56 AM 9/8/2005, Stanislav Malyshev wrote:

>WF>>IMO, "hard kill" doesn't really fit in a system that uses reference
>WF>>counts; you either use reference counts, or don't.  There are cases
>
>Obviously, we have here conflicting requirements - on the one side,
>fclose($a) should invalidate resourse, on the other side, oci_close($a)
>should not. I'm not sure it is possible to satisfy both of these
>requirements...

I think it is, in a way Sara pointed out. In OCI's case you decrement
the refcount and NULL the zval to make sure you don't scew the
refcount. In fclose() you'd just force the resource to be deleted and
have stale references to it (which is perfect fine in PHP).

Andi

Sara Golemon

20 years ago

>>> Stanislav Malyshev Wrote
> SG>><?php
> SG>> $fp1 = fopen('test', 'w');
> SG>> $fp2 = $fp1; /* Still a single zval* and le->refcount == 1 */
> SG>> $fp3 = &$fp1; /* Two zval*s now and le->refcount == 2 */
> SG>>
> SG>> fclose($fp1);
>
> I think this was discussed before, though I may be mistaken. The problem
> here is that you expect fclose to kill the resource, however $fp2 still
> refers it.
>

Right, that was my whole point with le->refcount == 2

> Looks like there should be two operations for resource - one is
> usual delref called when variable is destroyed and second is "hard kill"
> called when something like fclose is used and invalidating all referring
> variables. I remember there were some issues with that, but I can't
> rememebr what they were...
>

Right, that's what I meant by renaming the current zend_list_delete() to
zend_list_delref() and implementing a real zend_list_delete() that "hard
kills".  This isolates the code that would need to be changed to
zval_dtor()'s call and perhaps one or two isolated cases like oci8 which
intentionally use the refcounting.

Not that I expect list destruction to be turned on its head over an edge
case mind you.

It might be nice to just have a zend_list_real_delete() though:

#define zend_list_real_delete(id) zend_hash_index_del(&EG(regular_list), id)

Obviously an extension which knows it wants to ignore refcounts can just do
this one liner itself (and would have to for stable versions), that just
feels a little sloppy though.

>>>Wez Furlong Wrote:
> We talked about this for the revised oci8 code; we
> adopted the practice of ZVAL_NULL()ing the zval
> just after we zend_list_delete() it, because the updated
> oci8 codebase can return zvals that reference the same
> underlying list resource.
>

That's an interresting approach, and makes perfect sense in the oci8 model
where (from the user's perspective) those truly are different connection
resources.

> IMO, "hard kill" doesn't really fit in a system that
> uses reference counts; you either use reference counts,
> or don't.
>

The trouble with that theory is that functions like
fclose()/ftp_quit()/mysql_disconnect()/etc.... the user *does* expect the
resource to die.  Having latent resource IDs sitting around when the
resource has been destructed is what the "Unknown" resource type is for.
Academic purity isn't going to change what the user experience is.

> There are cases (particularly in streams) where we
> hold the resource ID to guarantee that a pointer
> remains valid.  Hard kill would break the reference
> counting contract and lead to a segv.
>

Not for nothin', but where?  If the stream pointer is gone isn't that
because the resource_dtor method has been called and the stream has shutdown
properly?  Once it's shutdown, what's left to try to use that pointer?

-Sara