[WIP] - Libuv 1.0 Support #59

steverhoades · 2014-11-10T04:44:43Z

Updates are based on the migration guide:
https://github.com/joyent/libuv/blob/a4f88760be1838603fe2eae89a651066cc42eedd/docs/src/migration_010_100.rst

There are still some segfault/segabrt issues that i am trying to get re-produceable tests in place for, leaving this here so that those with more experience can provide feedback.

…al tests

…uild

This change modifies the uv_fs_read() function signature slightly by adding a new $offset parameter. This allows the same file handle to be reused for multiple reads. Previously the only way to access a section of a file that had already been read was to open a new handle. Old: uv_fs_read(resoruce $loop, zval $fd, long $length, callable $callback) New: uv_fs_read(resoruce $loop, zval $fd, long $offest, long $length, callable $callback) This change represents a minor BC break for existing code using uv_fs_read().

…ainfiles or php related streams resolving a SIGABRT as epoll doesn't support those stream types

steverhoades · 2014-11-15T00:59:26Z

This issue is rather strange, so I am documenting it here.

The following code works fine with no segfaults.

 <?php
 $loop = uv_loop_new();

 $timerHandle = uv_timer_init($loop);
 $innerCallback = function($timerHandle) {
     echo "OK";
     uv_timer_stop($timerHandle);
     uv_unref($timerHandle);
 };
 $outerCallback = function() use ($timerHandle, $innerCallback) {
     call_user_func($innerCallback, $timerHandle);
 };
 uv_timer_start($timerHandle, 1000, 1000, $outerCallback);

 uv_run($loop, UV::RUN_ONCE);

However this code, which performs the same functionality except inside a class causes a segfault on php_shutdown.

 <?php
 class test 
 {
     private $timers;
     private $loop;

     public function __construct()
     {
         $this->loop = uv_loop_new();
         $this->timers = new SplObjectStorage();
     }

     public function addTimer($timer)
     {
         $resource = uv_timer_init($this->loop);

         $this->timers->attach($timer, $resource);
         $outerCallback = function() use ($timer) {
             call_user_func($timer->callback, $this->timers[$timer]);
         };

         uv_timer_start($resource, 1000, 1000, $outerCallback);
     }

     public function run()
     {
         uv_run($this->loop, UV::RUN_ONCE);
     }   
 }

 class timer 
 {
     public $callback;
 }

 $innerCallback = function($timerHandle) {
     echo "OK";
     uv_timer_stop($timerHandle);
     uv_unref($timerHandle);
 };
 $timer = new timer();
 $timer->callback = $innerCallback;

 $test = new test();
 $test->addTimer($timer);
 $test->run();

Removing the uv_unref call fixes the issue with the above. Code should be updated so that it does not segfault.

Stack trace:

#0  uv_walk (loop=0x7ffff5dfa268, walk_cb=0x7ffff6181ffd <close_walk_cb>, arg=0x0) at src/uv-common.c:324
#1  0x00007ffff6182070 in destruct_uv_loop (rsrc=0x7ffff5dfbef0, tsrm_ls=0xff6ee0) at /home/vagrant/cpackages/php-uv/php_uv.c:1192
#2  0x0000000000878c55 in list_entry_destructor (ptr=0x7ffff5dfbef0) at /home/vagrant/cpackages/php-src/Zend/zend_list.c:183
#3  0x00000000008740e7 in i_zend_hash_bucket_delete (ht=0xffa840, p=0x7ffff5df9798) at /home/vagrant/cpackages/php-src/Zend/zend_hash.c:182
#4  0x00000000008759c0 in zend_hash_del_key_or_index (ht=0xffa840, arKey=0x0, nKeyLength=0, h=39, flag=1)
    at /home/vagrant/cpackages/php-src/Zend/zend_hash.c:526
#5  0x00000000008786d7 in _zend_list_delete (id=39, tsrm_ls=0xff6ee0) at /home/vagrant/cpackages/php-src/Zend/zend_list.c:57
#6  0x000000000085cd5c in _zval_dtor_func (zvalue=0x7ffff5dfa1e8, 
    __zend_filename=0xccc2d8 "/home/vagrant/cpackages/php-src/Zend/zend_execute.h", __zend_lineno=79)
    at /home/vagrant/cpackages/php-src/Zend/zend_variables.c:65
#7  0x0000000000845f08 in _zval_dtor (zvalue=0x7ffff5dfa1e8, __zend_filename=0xccc2d8 "/home/vagrant/cpackages/php-src/Zend/zend_execute.h", 
    __zend_lineno=79) at /home/vagrant/cpackages/php-src/Zend/zend_variables.h:35
#8  0x0000000000845ff7 in i_zval_ptr_dtor (zval_ptr=0x7ffff5dfa1e8, 
    __zend_filename=0xcd24c8 "/home/vagrant/cpackages/php-src/Zend/zend_objects.c", __zend_lineno=54, tsrm_ls=0xff6ee0)
    at /home/vagrant/cpackages/php-src/Zend/zend_execute.h:79
#9  0x0000000000848197 in _zval_ptr_dtor (zval_ptr=0x7ffff5df9b00, 
    __zend_filename=0xcd24c8 "/home/vagrant/cpackages/php-src/Zend/zend_objects.c", __zend_lineno=54)
    at /home/vagrant/cpackages/php-src/Zend/zend_execute_API.c:427
#10 0x000000000089c9aa in zend_object_std_dtor (object=0x7ffff5dfa068, tsrm_ls=0xff6ee0)
    at /home/vagrant/cpackages/php-src/Zend/zend_objects.c:54
#11 0x000000000089cfba in zend_objects_free_object_storage (object=0x7ffff5dfa068, tsrm_ls=0xff6ee0)
    at /home/vagrant/cpackages/php-src/Zend/zend_objects.c:137
#12 0x00000000008a5160 in zend_objects_store_del_ref_by_handle_ex (handle=12, handlers=0xfddc40, tsrm_ls=0xff6ee0)
    at /home/vagrant/cpackages/php-src/Zend/zend_objects_API.c:226
#13 0x00000000008a4ce0 in zend_objects_store_del_ref (zobject=0x7ffff5dfaa10, tsrm_ls=0xff6ee0)
    at /home/vagrant/cpackages/php-src/Zend/zend_objects_API.c:178
#14 0x000000000085cd32 in _zval_dtor_func (zvalue=0x7ffff5dfaa10, 
    __zend_filename=0xccc2d8 "/home/vagrant/cpackages/php-src/Zend/zend_execute.h", __zend_lineno=79)
    at /home/vagrant/cpackages/php-src/Zend/zend_variables.c:57
#15 0x0000000000845f08 in _zval_dtor (zvalue=0x7ffff5dfaa10, __zend_filename=0xccc2d8 "/home/vagrant/cpackages/php-src/Zend/zend_execute.h", 
    __zend_lineno=79) at /home/vagrant/cpackages/php-src/Zend/zend_variables.h:35
#16 0x0000000000845ff7 in i_zval_ptr_dtor (zval_ptr=0x7ffff5dfaa10, 
    __zend_filename=0xcce480 "/home/vagrant/cpackages/php-src/Zend/zend_variables.c", __zend_lineno=187, tsrm_ls=0xff6ee0)
    at /home/vagrant/cpackages/php-src/Zend/zend_execute.h:79
#17 0x0000000000848197 in _zval_ptr_dtor (zval_ptr=0x7ffff5dff970, 
    __zend_filename=0xcce480 "/home/vagrant/cpackages/php-src/Zend/zend_variables.c", __zend_lineno=187)
    at /home/vagrant/cpackages/php-src/Zend/zend_execute_API.c:427
#18 0x000000000085d165 in _zval_ptr_dtor_wrapper (zval_ptr=0x7ffff5dff970) at /home/vagrant/cpackages/php-src/Zend/zend_variables.c:187
#19 0x00000000008740e7 in i_zend_hash_bucket_delete (ht=0xffa738, p=0x7ffff5dff958) at /home/vagrant/cpackages/php-src/Zend/zend_hash.c:182
#20 0x00000000008741be in zend_hash_bucket_delete (ht=0xffa738, p=0x7ffff5dff958) at /home/vagrant/cpackages/php-src/Zend/zend_hash.c:192
#21 0x00000000008762be in zend_hash_reverse_apply (ht=0xffa738, apply_func=0x846ec3 <zval_call_destructor>, tsrm_ls=0xff6ee0)
    at /home/vagrant/cpackages/php-src/Zend/zend_hash.c:733
#22 0x0000000000847003 in shutdown_destructors (tsrm_ls=0xff6ee0) at /home/vagrant/cpackages/php-src/Zend/zend_execute_API.c:217
#23 0x0000000000860227 in zend_call_destructors (tsrm_ls=0xff6ee0) at /home/vagrant/cpackages/php-src/Zend/zend.c:933
#24 0x00000000007a2f7f in php_request_shutdown (dummy=0x0) at /home/vagrant/cpackages/php-src/main/main.c:1826
#25 0x000000000092913d in do_cli (argc=7, argv=0xff6cc0, tsrm_ls=0xff6ee0) at /home/vagrant/cpackages/php-src/sapi/cli/php_cli.c:1177
#26 0x0000000000929920 in main (argc=7, argv=0xff6cc0) at /home/vagrant/cpackages/php-src/sapi/cli/php_cli.c:1378

rdlowrey · 2014-11-17T16:26:47Z

This segfault isn't surprising. You're manually decrementing the reference count for the timer resource from one to zero inside your callback and this leads to the actual UV resource being cleared:

zend_list_delete(uv->resource_id);

This is fine in your procedural code because there are no other references to the timer resource that need to be garbage collected. But your class example is storing a reference to the timer resource inside the SplObjectStorage thing. So you've removed the resource entry as far as PHP is concerned but retained a reference to that deleted entry in your userland code. Now when PHP tries to garbage collect the referenced memory down the road you get a segfault because it's already gone!

In short, you're only asking for trouble if you start fiddling with refcounts in userland. Just call uv_timer_stop() and remove any references in your code to the timer resource created from uv_timer_init($loop);.

Honestly, unless someone has a compelling reason to expose uv_unref() to userland I'm not sure it makes sense to have this functionality available in the extension at all. The only thing I can see it doing is causing segfaults without any actual benefit. Just because libuv exposes a uv_unref() doesn't necessarily mean it belongs in userland PHP ...

Anybody else have thoughts on this?

steverhoades · 2014-11-17T16:37:27Z

@rdlowrey I owe you a beer my friend. I have been trying to track this down for several days. Removing the zend_list_delete calls in uv_unref fixes the segfaulting issues as these are cleared in destruct_uv and destruct_uv_loop - this allows for proper cleanup with libuv 1.0. I'll check this in shortly, thanks sooooo much!

rdlowrey · 2014-11-17T16:44:51Z

@steverhoades no problem man; I appreciate your efforts to bring the extension up to 1.0 compatibility!

👍

…freed by destruct_uv

steverhoades · 2014-11-20T19:32:51Z

libuv 1.0 has been officially released: joyent/libuv@feb2a9e

chobie · 2014-12-04T18:13:07Z

@rdlowrey @steverhoades Thank you! this patch is awesome!
I'm playing this branch right now.

php tests/300-fs.php | hexdump -C
00000000  65 6c 6c 6f 0a 0a 65 6c  6c 6f 0a                 |ello..ello.|
0000000b

Hmm, This is my fault. static char uv_fs_read_buf[8192]; seems broken.
probably this should be prepare a buffer for each uv_fs_read call.

I'd like to merge this and fix this problem later. I suppose libuv 1.0 brings big worth for us.

rdlowrey · 2015-02-03T05:38:20Z

@chobie Yeah I realized later that the buffer was stored there. I've since fixed this locally and have been using it to great effect. I'll try to update my PR with the working changes this week.

steverhoades · 2015-02-03T07:03:50Z

@rdlowrey fantastic! I haven't had any time recently to continue to look into this further however it's still something I would like to get done. Let me know if there is anything I can do to help.

steverhoades added 3 commits November 7, 2014 18:40

saving changes, still 3 failing tests

84d7e09

update for libuv 1.0.0rc2, updated tests for api changes

1ea6c5a

fix issue with uv_loop_new based on 1.0 migration guide, add addition…

59f74ea

…al tests

This was referenced Nov 10, 2014

Segfault while spawning a new process with uv_spawn on PHP 5.5.9 #56

Open

Coredump on Ubuntu when using polls #40

Open

add test for fs_close issue described in chobie#36, updating travis b…

9d6e66d

…uild

steverhoades force-pushed the libuv-1.0 branch from 5771c0d to 9d6e66d Compare November 10, 2014 17:10

rdlowrey and others added 6 commits November 11, 2014 10:47

fix uv_fs_scandir

459b614

cleanup uv_fs_scandir

73a295a

Fix Issue chobie#59 segfault on null passed as handle to uv_stdio_new

dd9aac9

Fix issue chobie#36, Fix issue chobie#56, uv_poll no longer allows pl…

2408f16

…ainfiles or php related streams resolving a SIGABRT as epoll doesn't support those stream types

fix notice

f2052a3

steverhoades force-pushed the libuv-1.0 branch from 64cc8fe to f2052a3 Compare November 14, 2014 02:18

update test, just checking for no segfault

a026d75

fix segfault issue caused by uv_unref deleting resource, resource is …

42361ad

…freed by destruct_uv

steverhoades added 2 commits November 20, 2014 19:47

updating to libuv 1.0 release

99791cf

merging offset changes from PR chobie#60

605a20a

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[WIP] - Libuv 1.0 Support #59

[WIP] - Libuv 1.0 Support #59

steverhoades commented Nov 10, 2014

steverhoades commented Nov 15, 2014

rdlowrey commented Nov 17, 2014

steverhoades commented Nov 17, 2014

rdlowrey commented Nov 17, 2014

steverhoades commented Nov 20, 2014

chobie commented Dec 4, 2014

rdlowrey commented Feb 3, 2015

steverhoades commented Feb 3, 2015

[WIP] - Libuv 1.0 Support #59

Are you sure you want to change the base?

[WIP] - Libuv 1.0 Support #59

Conversation

steverhoades commented Nov 10, 2014

steverhoades commented Nov 15, 2014

rdlowrey commented Nov 17, 2014

steverhoades commented Nov 17, 2014

rdlowrey commented Nov 17, 2014

steverhoades commented Nov 20, 2014

chobie commented Dec 4, 2014

rdlowrey commented Feb 3, 2015

steverhoades commented Feb 3, 2015