Everything

php

Third party object support in ZE2

php.internals

Steph

20 years ago

Hi Dmitry,

I gave up on IM now, it's too slow :)

Coming from the PHP-GTK project, we've noticed two areas where
ZEND_INTERNAL_CLASS is wrongly expected to adhere to zend_object structure.

The more important of the two stems from a change you made in mid-November
in CVS_HEAD and PHP_5_1, forcing a call to zend_objects_destroy_object()
when there is no obj->dtor defined. (zend_objects_API.c, revision 1.47.2.3).
Reaching that function causes a crash in PHP-GTK 2 under some circumstances.
Bug report open on this behaviour at http://bugs.php.net/35871.

The second is that we can't guarantee third party objects will pass
zend_do_perform_implementation_check()... so if display_startup_errors is
on, we get to know about it every time GTK+ objects break PHP's inheritance
rules.  Trivial patch attached appears to fix this minor issue without
breaking anything currently in the test suite, but you might know better.

- Steph

Dmitry Stogov

20 years ago

Hi Steph,

> -----Original Message-----
> From: Steph Fox [mailto:steph@zend.com]
> Sent: Monday, January 09, 2006 9:50 PM
> To: Dmitry Stogov
> Cc: Andrei; internals
> Subject: Third party object support in ZE2
>
>
> Hi Dmitry,
>
> I gave up on IM now, it's too slow :)
>
> Coming from the PHP-GTK project, we've noticed two areas where
> ZEND_INTERNAL_CLASS is wrongly expected to adhere to
> zend_object structure.
>
> The more important of the two stems from a change you made in
> mid-November
> in CVS_HEAD and PHP_5_1, forcing a call to
> zend_objects_destroy_object()
> when there is no obj->dtor defined. (zend_objects_API.c,
> revision 1.47.2.3).

Yes. This is my patch. It allws calling destructor in classes those extend
internal ones.
(Probably it can be reverted and all "bad" extensions should be fixed).

> Reaching that function causes a crash in PHP-GTK 2 under some
> circumstances.
> Bug report open on this behaviour at http://bugs.php.net/35871.

I reproduced this crash.
The reason that "style_helper" is incompatible with "zend_object" and it
hasn't pointer to class entry.

I suggest to fix it on PHP-GTK level. I see two ways:
1) Make "style_helper" compatible with "zend_object".
2) Pass dumy dtor handler to zend_objects_store_put()
(php_gtk++-types.c:382)

> The second is that we can't guarantee third party objects will pass
> zend_do_perform_implementation_check()... so if
> display_startup_errors is
> on, we get to know about it every time GTK+ objects break
> PHP's inheritance
> rules.  Trivial patch attached appears to fix this minor
> issue without
> breaking anything currently in the test suite, but you might
> know better.

I didn't understand why we should allow to break inheritance rules?
Right now we break rules only for "parent" internal functions without
arg_info.
I think this is enough. Why should we break ZE for "bad" extensions?
May be I missed something?

BTW: The patch has mistake: ZEND_INTERNAL_CLASS should be
ZEND_INTERNAL_FUNCTION.

Thanks. Dmitry.

Steph

20 years ago

Hi Dmitry,

>> in CVS_HEAD and PHP_5_1, forcing a call to
>> zend_objects_destroy_object()
>> when there is no obj->dtor defined. (zend_objects_API.c,
>> revision 1.47.2.3).
>
> Yes. This is my patch. It allws calling destructor in classes those extend
> internal ones.
> (Probably it can be reverted and all "bad" extensions should be fixed).

We weren't a 'bad' extension until you did this :)

> I reproduced this crash.
> The reason that "style_helper" is incompatible with "zend_object" and it
> hasn't pointer to class entry.
> I suggest to fix it on PHP-GTK level. I see two ways:
> 1) Make "style_helper" compatible with "zend_object".
> 2) Pass dumy dtor handler to zend_objects_store_put()
> (php_gtk++-types.c:382)

Second's saner. I was hoping there'd be some way to differentiate third
party objects at engine level originally, but if there isn't we're better
off having something generic when we touch it.

>> The second is that we can't guarantee third party objects will pass
>> zend_do_perform_implementation_check()... so if
>> display_startup_errors is
>> on, we get to know about it every time GTK+ objects break
>> PHP's inheritance
>> rules.  Trivial patch attached appears to fix this minor
>> issue without
>> breaking anything currently in the test suite, but you might
>> know better.
>
> I didn't understand why we should allow to break inheritance rules?

Because we used to do precisely that for third-party objects?

> Right now we break rules only for "parent" internal functions without
> arg_info.

Which is the point where internal SPL classes etc etc get through, yes I
saw.

> I think this is enough. Why should we break ZE for "bad" extensions?
> May be I missed something?
>
> BTW: The patch has mistake: ZEND_INTERNAL_CLASS should be
> ZEND_INTERNAL_FUNCTION.

Mm, I thought that too originally, except that it works (and shouldn't if
that's the case).

Thanks for the overview.  I guess that's a 'no', then...

- Steph