PHP-1230: Store WriteResult in WriteException struct instead of property

[jmikola]

https://jira.mongodb.org/browse/PHP-1230

[coveralls]

Coverage decreased (-3.34%) when pulling 373c6bf on php1230-writeexception into deafb8d on master.

[bjori]

========DIFF========

023+ Segmentation fault (core dumped)

========DONE========

FAIL MongoDB\Manager::executeWriteBatch() with duplicate key errors (ordered) [tests/standalone/manager-executeWriteBatch-002.phpt]

Seems relevant?

[jmikola]

Yes, both of the write error tests fail due to a segfault in _zend_mm_free_int:

• manager-executeWriteBatch-002.phpt
• manager-executeWriteBatch-003.phpt

I pasted the backtrace in IRC yesterday: http://pastebin.com/FVEWyZdv

[jmikola]

@bjori: I believe I found the problem. Valgrind turned up some of the following errors:

Invalid (read|write) of size 8
Address 0x79dfdf0 is 0 bytes after a block of size 32 alloc'd

The 32-byte block is the default struct with a single zend_object std property. That struct contains 4 pointer fields, so it's 32-bytes (on 64-bit). This means our php_phongo_writeexception_t struct was never getting allocated in the first place. Furthermore, our create_object handler, which does the allocation and memset, was never invoked.

I discovered that the create_object handler cannot be changed when inheriting a class. I'm not sure why this is required, as a child class could assume the responsibility to ensure its struct only extends the parent. Anyway, when WriteException's MINIT does:

ce.create_object = php_phongo_writeexception_create_object;
php_phongo_writeexception_ce = zend_register_internal_class_ex(&ce, spl_ce_RuntimeException, NULL TSRMLS_CC);

We specify a create_object handler, and it promptly gets reverted by zend_register_internal_class_ex(). That function invokes zend_do_inheritance(), which in turn calls do_inherit_parent_constructor() and reaches this lovely bit of code:

/* You cannot change create_object */
ce->create_object = ce->parent->create_object;

Given this limitation, I think we're stuck with using object properties then. Do you concur?

[jmikola]

Closing this for now. If we come up with a work-around in the future we can revisit this.

See #19 instead.