-
-
Notifications
You must be signed in to change notification settings - Fork 127
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
ExtEvLoop: Add new ExtEvLoop (PECL source ext-ev) #12
Changes from 4 commits
6241c0e
8e847ea
b2b68c6
a496c63
e21c0ab
804d13c
126a6bd
8d12457
1cf5df8
e5b0a0b
34b51b6
5a643f2
7edb3b8
39aff0d
b616955
a643c7a
f01422d
40127e4
f8309da
525d1cf
9edc3f2
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -11,6 +11,6 @@ matrix: | |
- php: hhvm | ||
|
||
install: ./travis-init.sh | ||
|
||
script: | ||
- phpunit --coverage-text | ||
- phpunit --coverage-text | ||
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,3 @@ | ||
set env MALLOC_CHECK_=3 | ||
run | ||
backtrace full | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This file, |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,249 @@ | ||
<?php | ||
|
||
namespace React\EventLoop; | ||
|
||
use SplObjectStorage; | ||
use React\EventLoop\Timer\Timer; | ||
use React\EventLoop\Timer\TimerInterface; | ||
use React\EventLoop\Tick\FutureTickQueue; | ||
use React\EventLoop\Tick\NextTickQueue; | ||
|
||
class ExtEvLoop implements LoopInterface | ||
{ | ||
private $loop; | ||
private $nextTickQueue; | ||
private $futureTickQueue; | ||
private $timers; | ||
private $readEvents = array(); | ||
private $writeEvents = array(); | ||
|
||
private $running = false; | ||
|
||
public function __construct() | ||
{ | ||
$this->loop = new \EvLoop(); | ||
$this->timers = new SplObjectStorage(); | ||
$this->nextTickQueue = new NextTickQueue($this); | ||
$this->futureTickQueue = new FutureTickQueue($this); | ||
$this->timers = new SplObjectStorage(); | ||
} | ||
|
||
/** | ||
* Add a readable stream to the event loop | ||
* @param stream $stream PHP stream resource | ||
* @param callable $listener Callback | ||
* @return void | ||
*/ | ||
public function addReadStream($stream, callable $listener) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. You should use |
||
{ | ||
$this->addStream($stream, $listener, \Ev::READ); | ||
} | ||
|
||
/** | ||
* Add a writable stream to the event loop | ||
* @param stream $stream PHP stream resource | ||
* @param callable $listener Callback | ||
* @return void | ||
*/ | ||
public function addWriteStream($stream, callable $listener) | ||
{ | ||
$this->addStream($stream, $listener, \Ev::WRITE); | ||
} | ||
|
||
/** | ||
* Remove a readable stream from the event loop. | ||
* @param stream $stream PHP stream resource | ||
* @return void | ||
*/ | ||
public function removeReadStream($stream) | ||
{ | ||
$key = (int) $stream; | ||
if(isset($this->readEvents[$key])) { | ||
$this->readEvents[$key]->stop(); | ||
unset($this->readEvents[$key]); | ||
} | ||
} | ||
|
||
/** | ||
* remove a writable stream from the event loop | ||
* @param stream $stream PHP stream resource | ||
* @return void | ||
*/ | ||
public function removeWriteStream($stream) | ||
{ | ||
$key = (int) $stream; | ||
if(isset($this->writeEvents[$key])) { | ||
$this->writeEvents[$key]->stop(); | ||
unset($this->writeEvents[$key]); | ||
} | ||
} | ||
|
||
/** | ||
* remove a stream from the event loop | ||
* @param stream $stream PHP stream | ||
* @return void | ||
*/ | ||
public function removeStream($stream) | ||
{ | ||
if (isset($this->readEvents[(int)$stream])) { | ||
$this->removeReadStream($stream); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I know using |
||
} | ||
|
||
if (isset($this->writeEvents[(int)$stream])) { | ||
$this->removeWriteStream($stream); | ||
} | ||
} | ||
|
||
/** | ||
* Wraps the listener in a callback which will pass the | ||
* stream to the listener then registers the stream with | ||
* the eventloop. | ||
* | ||
* @param stream $stream PHP Stream resource | ||
* @param callable $listener stream callback | ||
* @param bit $flags flag bitmask | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Please use |
||
*/ | ||
private function addStream($stream, callable $listener, $flags) | ||
{ | ||
$listener = function ($event) use ($stream, $listener) { | ||
call_user_func($listener, $stream); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The callback should be invoked by passing the current loop instance as the 2nd argument, so this line should be |
||
}; | ||
|
||
$event = $this->loop->io($stream, $flags, $listener); | ||
|
||
if (($flags & \Ev::READ) === $flags) { | ||
$this->readEvents[(int)$stream] = $event; | ||
} elseif (($flags & \Ev::WRITE) === $flags) { | ||
$this->writeEvents[(int)$stream] = $event; | ||
} | ||
} | ||
|
||
/** | ||
* {@inheritdoc} | ||
*/ | ||
public function addTimer($interval, callable $callback) | ||
{ | ||
$timer = new Timer($this, $interval, $callback, false); | ||
$this->setupTimer($timer); | ||
|
||
return $timer; | ||
} | ||
|
||
/** | ||
* {@inheritdoc} | ||
*/ | ||
public function addPeriodicTimer($interval, callable $callback) | ||
{ | ||
$timer = new Timer($this, $interval, $callback, true); | ||
$this->setupTimer($timer); | ||
|
||
return $timer; | ||
} | ||
|
||
/** | ||
* {@inheritdoc} | ||
*/ | ||
public function cancelTimer(TimerInterface $timer) | ||
{ | ||
if (isset($this->timers[$timer])) { | ||
/* stop EvTimer */ | ||
$this->timers[$timer]->stop(); | ||
|
||
/* defer timer */ | ||
$this->nextTick(function() use ($timer) { | ||
$this->timers->detach($timer); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Just curious: why detaching the timer on There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is to avoid segfaults that occur on PHP 5.4, 5.5. There is some delay needed before the call to detach the time is actually made - hacky, but it does solve the issue. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. For future reference, I'd put in the comment a short explanation for the reason why we need There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Thats a good point, i'll get a comment put in there for future reference. |
||
}); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. hey, this defer was added to work around a segfault, right? is it still needed? |
||
} | ||
} | ||
|
||
/** | ||
* Add timer object as | ||
* @param TimerInterface $timer [description] | ||
* @return [type] [description] | ||
*/ | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Missing something here? :-) Seems incomplete.... There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Looks ok to me, what strikes you as incomplete? There are some work arounds necessary with the EV extension in order to avoid segfaults across different versions of PHP, this is what your seeing here. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Hah sorry, I mean the contents of the phpdoc block :-) There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Hah, missed that - i'll get that updated accordingly |
||
private function setupTimer(TimerInterface $timer) | ||
{ | ||
$callback = function () use ($timer) { | ||
call_user_func($timer->getCallback(), $timer); | ||
|
||
if (!$timer->isPeriodic()) { | ||
$timer->cancel(); | ||
} | ||
}; | ||
|
||
$interval = $timer->getInterval(); | ||
|
||
$libevTimer = $this->loop->timer($interval, $interval, $callback); | ||
|
||
$this->timers->attach($timer, $libevTimer); | ||
|
||
return $timer; | ||
} | ||
|
||
public function isTimerActive(TimerInterface $timer) | ||
{ | ||
return $this->timers->contains($timer); | ||
} | ||
|
||
|
||
/** | ||
* {@inheritdoc} | ||
*/ | ||
public function nextTick(callable $listener) | ||
{ | ||
$this->nextTickQueue->add($listener); | ||
} | ||
|
||
/** | ||
* {@inheritdoc} | ||
*/ | ||
public function futureTick(callable $listener) | ||
{ | ||
$this->futureTickQueue->add($listener); | ||
} | ||
|
||
|
||
public function tick() | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Missing phpdoc headers with |
||
{ | ||
$this->nextTickQueue->tick(); | ||
$this->futureTickQueue->tick(); | ||
|
||
$flags = \Ev::RUN_ONCE; | ||
if (!$this->running || !$this->nextTickQueue->isEmpty() || !$this->futureTickQueue->isEmpty()) { | ||
$flags |= \Ev::RUN_NOWAIT; | ||
} elseif (!$this->readEvents && !$this->writeEvents && !$this->timers->count()) { | ||
$this->running = false; | ||
return; | ||
} | ||
$this->loop->run($flags); | ||
} | ||
|
||
public function run() | ||
{ | ||
$this->running = true; | ||
while($this->running) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'd put an empty line before this |
||
$this->tick(); | ||
} | ||
} | ||
|
||
public function stop() | ||
{ | ||
$this->running = false; | ||
} | ||
|
||
public function __destruct() | ||
{ | ||
// mannually stop all watchers | ||
foreach($this->timers as $timer) { | ||
$this->timers[$timer]->stop(); | ||
} | ||
|
||
foreach($this->readEvents as $event) { | ||
$event->stop(); | ||
} | ||
|
||
foreach($this->writeEvents as $event) { | ||
$event->stop(); | ||
} | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,40 @@ | ||
<?php | ||
|
||
namespace React\Tests\EventLoop; | ||
|
||
use React\EventLoop\ExtEvLoop; | ||
|
||
class ExtEvLoopTest extends AbstractLoopTest | ||
{ | ||
private $file; | ||
|
||
public function createLoop() | ||
{ | ||
if (!class_exists('EvLoop')) { | ||
$this->markTestSkipped('ev tests skipped because pecl/ev is not installed.'); | ||
} | ||
|
||
return new ExtEvLoop(); | ||
} | ||
|
||
public function tearDown() | ||
{ | ||
if (file_exists($this->file)) { | ||
unlink($this->file); | ||
} | ||
} | ||
|
||
public function createStream() | ||
{ | ||
$this->file = tempnam(sys_get_temp_dir(), 'react-'); | ||
|
||
$stream = fopen($this->file, 'r+'); | ||
|
||
return $stream; | ||
} | ||
|
||
public function testEvConstructor() | ||
{ | ||
$loop = new ExtEvLoop(); | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -20,6 +20,18 @@ if [ "$TRAVIS_PHP_VERSION" != "hhvm" ]; then | |
popd | ||
echo "extension=libevent.so" >> "$(php -r 'echo php_ini_loaded_file();')" | ||
|
||
# install 'pecl-ev' PHP extension | ||
git clone http://bitbucket.org/osmanov/pecl-ev.git | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I was wondering why fetching the library from its repository when it's available on PECL, but when I tried installing it via PECL on my computer I received this message:
Is this the reason why you prefer cloning from the repository, or is it just my dev environment acting weird? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is an artifact from debugging the segfaults in the earlier versions of the extension with PHP 5.6, needed to test against the latest version. I'll test again against the pecl install to see if I can replicate this issue. |
||
# 0.2.10 | ||
pushd pecl-ev | ||
phpize | ||
./configure | ||
make | ||
# make test | ||
make install | ||
popd | ||
echo "extension=ev.so" >> "$(php -r 'echo php_ini_loaded_file();')" | ||
|
||
# install 'libev' PHP extension | ||
git clone --recursive https://github.com/m4rw3r/php-libev | ||
pushd php-libev | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This line should be indented by 2, not 4 spaces.