LibEventLoop doesn't notifies userland code immediately

[toverux]

I'm working on a PHP keylogger using the Linux evdev API (characted devices in /dev/input/).

I'm working on the React interface of the library, which is a specialization of my main class.
Using my main (non-React) class and logging the events, I can get this type of log when I press the space key one time (simplified output) :

-- handleData --   -> handleData called
key=57 value=1     -> keydown on space
-- handleData --
key=57 value=0     -> keyup on space

OK. I pressed space one time and I instantly get two events related to the space key. All good.

Now, using the React extend of my keylogger and doing the same key press :

-- handleData --
key=57 value=1     -> keydown on space

You can notice I don't get the keyup event. Nothing happens until I press another key. So, let's press space another time :

-- handleData --   -> previous log
key=57 value=1     -> previous log
-- handleData --
key=57 value=0     -> it is displayed only now!!
-- handleData --
key=57 value=1     -> ok, expected new keydown
                   -> missing new keyup... 'til I press another key

I was using EventLoop\Factory::create() to get the event loop. It was returning a LibEventLoop. I tried to force the loop class used by replacing the factory with the plain-PHP StreamSelectLoop. And it works perfectly with this loop class.

I can't try with LibEvLoop or ExtEventLoop since these libraries aren't installed on my system.

Maybe it's a quirk of LibEventLoop on character files?
I hope I'm clear enough !

[WyriHaximus]

This looks like a very neat project. The factory creates the best performant loop it can find, falling back to the StreamSelectLoop. Could very well be a LibEventLoop quirk or something else. Do you have a gist for me I can use to try and reproduce the problem?

EDIT: Forgot to add this, does the event show up several seconds to a minute or two later when you don't press an extra key?

[mbonneau]

I had issues that sound similar to this #26.

Stream buffering in PHP will cause LibEvent to not fire (because php has already buffered input).

Try adding:

stream_set_read_buffer($stream, 0);

That will force PHP not to try and buffer itself.

[mbonneau]

There is a PR for the issue (if this is the issue) over in react/stream (reactphp/stream#20).

This is the line that is added:
https://github.com/reactphp/stream/blob/4da0128801a7342bbada6a0fe7dea4f5b2092139/src/Stream.php#L27

[toverux]

@WyriHaximus thanks for your answer, I was writing the minimal code to reproduce the bug when mbonneau posted the solution.

@mbonneau Fantastic! Works like a charm. I thought my issue would be much harder to fix than that :D Good to see that a pull request to fix this behaviour is (slowly) integrated.
Note that I'm not using the Stream API because I only need a small subset of it, I use only the event loop, so it could be great to add a note about that into the documentation of this repository, I think it's pretty important to warn users about that (or the React's event loop can also force the buffer to zero when a stream is passed in addReadStream/addWriteStream but I don't know if it is a good idea to do this as the default behaviour).

[clue]

@mbonneau Fantastic! Works like a charm. I thought my issue would be much harder to fix than that :D Good to see that a pull request to fix this behaviour is (slowly) integrated.

Thanks for posting back that this does indeed fix the problem!

Note that I'm not using the Stream API because I only need a small subset of it, I use only the event loop, so it could be great to add a note about that into the documentation of this repository, I think it's pretty important to warn users about that

I agree that we need some documentation about this in this project. After all, the documentation explicitly states that this library can be used standalone without React's stream component (though it probably hardly ever will). See also reactphp/stream#28 for some background on why this may be needed.

[mbonneau]

I did not realize that event-loop was designed to be used standalone.

In this case, it would make sense to move stream_set_read_buffer from the stream library to LibEventLoop so that other users of the library are not caught by this and also because it only effects LibEventLoop (actually, I don't think I have tried Ev). In the long term, this may be something that should be corrected in the extension itself.

[clue]

because it only effects LibEventLoop

These loops support more than stream based resources (e.g. socket resources) and this should also affect other loops that are edge triggered (haven't verified this yet).

As such, I'm unsure we can even address this here?

I think adding a warning to the README is probably what makes most sense as a first step 👍

[clue]

I think adding a warning to the README is probably what makes most sense as a first step 👍

This is everything that we can do here. Does anybody feel like filing a PR? 👍