ExtEvLoop: Add new ExtEvLoop (PECL source ext-ev) #12
Conversation
|
Don't believe travis, the unit tests are not passing - dealing with segfault issues in 5.6 and after unit tests run successfully in the other PHP versions. Have open ticket with pecl-ev author here: https://bitbucket.org/osmanov/pecl-ev/issue/9/segfault#comment-12153834 |
|
The issue appears to be isolated to thread safe PHP builds. A PHP 5.6 build with thread safe off works fine. |
|
Updating to capture status. pecl-ev extension is fixed, all tests are now passing. |
- Change `stream` and `numeric` to valid phpdoc internal types - Add some docs to TimerInterface and Timer
[ci skip]
[ci skip]
From DaveRandom:fix/docblocks.
[ci skip]
| script: | ||
| - phpunit --coverage-text | ||
| - phpunit --coverage-text |
There was a problem hiding this comment.
This line should be indented by 2, not 4 spaces.
|
I'm done with the code review for now, I did a few tests and everything seems to be working fine but now we need to do a few stress tests and check that there are no memory leaks. @cboden will probably join us in the test-fest next week :-) |
|
Thanks! Don't worry and take your time, we are not in a hurry :-) |
|
@nrk ok so attempted to run tests with pecl ev package install instead of clone, unfortunately it looks like in order to install the pecl ev package you need root access. Travis reports no pecl found when attempting to run the command via sudo. For now, I'm reverting back to cloning the repo. |
|
Functionally looks good but I believe there's a memory leak. I tested the simple memory test and it kept going up and up where as the other loops remain steady. |
| /* defer timer */ | ||
| $this->nextTick(function() use ($timer) { | ||
| $this->timers->detach($timer); | ||
| }); |
There was a problem hiding this comment.
hey, this defer was added to work around a segfault, right? is it still needed?
|
ping @cboden @steverhoades @markkimsal how can we move forward on #12 & #25 to make something mergeable at some time? This is already a long story for the 2 PRs, should be sad to give up now ;) |
|
Tested on PHP7 with updated Ev extension over the weekend. Still seeing memory leaks. |
ghost
mentioned this pull request
|
I'd like to make this actionable What is the current status here? How is this related to #25? Is there still interest in this ticket or can either be closed? |
clue
changed the title
|
@clue both this PR and #25 target the same PECL extension and they look very similar aside from a few minor differences. On a related note our own |
clue
changed the title
|
Thank you for working on this and helping push this forward! This is kind of an old PR and it looks like #97 is the most up to date version of this feature, so I'm closing this one in favor of the newer PR |
Brought reactphp/reactphp#178 inline with current branch. Fixed unit tests. added additional unit test for periodic timer.