expressif / pecl-event-libevent

PHP7 Libevent - event notification
http://pecl.php.net/package/libevent
56 stars 16 forks source link

_php_bufferevent_readcb segfult #1

Closed Joungkyun closed 8 years ago

Joungkyun commented 8 years ago

Hi, I migrated pecl-libevent too for PHP7. But I met segfault at _php_bufferevent_readcb, and I can not solve this problems.

And, I found your patch, and test your code. But your code makes same problems too.

confirm follow codes plez.

thanks

test environment CentOS 7.2 php-7.0.1 libevent-2.0.21-4.el7.x86_64 or libevent14-1.4.13-4.an3.x86_64

-- cut here-- Example #2 polling STDIN using buffered event API on http://php.net/manual/en/libevent.examples.php:

<?php

$socket = stream_socket_server ('tcp://0.0.0.0:2000', $errno, $errstr);
stream_set_blocking($socket, 0);
$base = event_base_new();
$event = event_new();
event_set($event, $socket, EV_READ | EV_PERSIST, 'ev_accept', $base);
event_base_set($event, $base);
event_add($event);
event_base_loop($base);

$GLOBALS['connections'] = array();
$GLOBALS['buffers'] = array();

function ev_accept($socket, $flag, $base) {
    static $id = 0;

    $connection = stream_socket_accept($socket);
    stream_set_blocking($connection, 0);

    $id += 1;

    $buffer = event_buffer_new($connection, 'ev_read', NULL, 'ev_error', $id);
    event_buffer_base_set($buffer, $base);
    event_buffer_timeout_set($buffer, 30, 30);
    event_buffer_watermark_set($buffer, EV_READ, 0, 0xffffff);
    event_buffer_priority_set($buffer, 10);
    event_buffer_enable($buffer, EV_READ | EV_PERSIST);

    // we need to save both buffer and connection outside
    $GLOBALS['connections'][$id] = $connection;
    $GLOBALS['buffers'][$id] = $buffer;
}

function ev_error($buffer, $error, $id) {
    event_buffer_disable($GLOBALS['buffers'][$id], EV_READ | EV_WRITE);
    event_buffer_free($GLOBALS['buffers'][$id]);
    fclose($GLOBALS['connections'][$id]);
    unset($GLOBALS['buffers'][$id], $GLOBALS['connections'][$id]);
}

function ev_read($buffer, $id) {
    while ($read = event_buffer_read($buffer, 256)) {
        var_dump($read);
    }
}
?>
ichiriac commented 8 years ago

Hi,

In my tests I was not using the optional argument ($id in the example).

The reason of the fail is explained on the phpng doc :

The main difference is that now we handle scalar and complex types differently. PHP doesn't allocate scalar values in heap but do it directly on VM stack, inside HashTables and object. They are not subjects for reference counting and garbage collection anymore. Scalar values don't have reference counter and don't support ZADDREF(), ZDELREF(), ZREFCOUNT() and Z_SETREFCOUNT() macros anymore. In most cases you should check if zval supports these macros before calling them. Otherwise you'll get an assert() or crash.

- Z_ADDREF_P(zv)
+ if (Z_REFCOUNTED_P(zv)) {Z_ADDREF_P(zv);}
# or equivalently
+ Z_TRY_ADDREF_P(zv);

The reason of the crash is that the argument is a scalar (primitive) integer, and z_addref_p MUST not be used on scalars.

I will publish a patch and send a pull request on this ...