Added kevent hook following Dean McNamee

[piannucci]

It looks like plask has been carrying a diff against libuv for a few years, and when it came in handy for me as well, I thought I'd propose that it be upstreamed.

There is extensive commentary in
https://github.com/deanm/plask/blob/master/main.mm

My own attempts to run a uv loop as a guest inside a CFRunLoop in a Python interpreter have proven unsuccessful, perhaps for the same reasons that @deanm encountered.

Hooking kevent permits the opposite approach -- running a CFRunLoop as a guest in a uv loop. I need a CFRunLoop to pump the Grand Central Dispatch main queue, and I want a uv loop to take advantage of the uvloop asyncio backend on the same thread.

What do you think?

[santigimeno]

/cc @indutny

[santigimeno]

In case this approach is considered valid, it would need tests and documentation. Thanks!

[saghul]

Hi @piannucci and welcome to libuv! A quick glance at the comments made me remember the implementation in Electron, which is different, but shares the same goal: electron/node@6389e78

Ideally I'd like to find a way to do this which works for every project out there.

[bnoordhuis]

While I appreciate the spirit I think this particular pull request is too much of a one-off hack to be acceptable. Like Saúl says, it should be broadly usable and not just on OS X.

(Aside: loop.c won't compile on other UNIX platforms now.)

[piannucci]

Thank you for the welcome, and the comments.

One alternative would be to just straight-up incorporate a CFRunLoop adapter into libuv by default. I see that src/unix/fsevents.c offers to run a CFRunLoop in a separate thread; that might be simplified if the CFRunLoop ran on the main thread (whenever it is idling).

Here's the gist of how the hook is being used in my application, following the plask code. Apologies for any syntax errors; it's been translated back and forth to Cython a few times.

If CFRunLoop support were upstreamed, then Plask et al would have to do some skulduggery to reimplement parts of NSApplication, but my current understanding is that that is doable. Also, if the approach described in the gist were upstreamed, then rather than a kevent hook, all calls to kevent would be refactored into a platform-specific function that either calls kevent directly (BSD) or runs the CFRunLoop (Darwin).

[deanm]

In my experience, there won't be a nice cross platform way to achieve what Plask is currently doing. I've tried a lot of different approaches, and this is the only thing I've come up with that really works properly. I've unfortunately had to become familiar with some of the details of platform eventloops a few times before (I worked on Chrome's event loop and did a lot of that Linux/GTK code). I've gone as far as talking to a kernel developer at Microsoft and reverse engineering a bunch of the OSX event loop / UI message system. In general all major platform event loops are really hairy and very tightly coupled with the peculiarities of corresponding UI frameworks. For applications like Plask that need to do UI, system messages, networking, etc on the main thread it's very easy for one subsystem to starve or block the other (networking starve UI, etc). On platforms like Windows there are many complicated aspects to the implementation like synthetic paint messages. Have a look at the complexity in https://chromium.googlesource.com/chromium/src.git/+/master/base/message_loop/ and that is when you have control over the entire system and a dedicated network thread... Cases like Plask are even more complicated due to the single threaded requirement for JavaScript to be able to interact with the UI system, so everything needs to run together on the main thread.

Anyway, back to the original point. I tried a lot of cleaner / simpler / etc patches to Node first. Many times I thought a came up with a cleaner solution only to eventually find a deadlock / race / block. It would be great if someone manages to crack the nut but my bet is strongly on a lot of subtle bugs that will turn out to be fundamental design problems.

Personally I think having something like the kqueue hook isn't really a big deal and it's a small patch and I don't think a maintenance burden. It's effectively just allowing you to overwrite the pointer to the kqueue function, you could do it without any overhead since it otherwise will be going through a PLT entry for the import. Yes it's platform specific but so is kqueue, so I don't think it's too strange to have some kqueue-only control. The alternative for Plask would have been to try to hook/interpose the kqueue import but this too messy for a lot of other reasons.

Btw I did something similar with Node on windows, and I had to take a completely different approach, so at least from my perspective this type of code by nature will have to be platform specific and the host UI code will have to have different versions for the different UI systems it supports anyhow.

PS: I don't know what Electron is doing but I once looked at the node webkit integration, and I remember it looking to me to not be a solid approach (ie just spinning at 200ms to check the UI events or something like that). For a high performance graphics application like Plask that's really not acceptable.

[indutny]

It doesn't seem to be a good idea to me.

Is there any reason to not use backend_fd explicitly, and to not call uv_run() in no-wait mode when there are changes on backend_fd? Should be very similar, if not the same?

[deanm]

@indutny that's an approach I tried and no it's not the same... if I remember part of the problem is that there are changes that aren't committed until the kqueue call itself, etc. My memory in a bit hazy and this point now but I remember explicitly trying a few approaches exactly like what you've mentioned and no it's not enough. Also you need to be able to get things like the proper timeout value for the kqueue call (based on the timer heap), etc, etc. I really tried, I don't think there is a simpler approach.

[indutny]

@deanm re: timeout. There is a method for this:

UV_EXTERN int uv_backend_timeout(const uv_loop_t*);

I'd love to learn more details about problems with kqueue. Could you please elaborate? Do you have a test case?

[deanm]

@indutny This is exactly what I used to do, see: deanm/plask@3db9c66#diff-860ea3021871d5efa8973e439a74cdd0L96

Long story short I am certain it wasn't a reliable approach.

If I remember correctly part of the problem is that this isn't necessarily the right timeout with respect to the fact that more user code can run in all sorts of points in the event loop cycle. Perhaps it's more about Node than libuv specifically, but a lot of things can happen when you run through a tick of the event loop, watchers and pre/post code. So basically there are a lot of side effects that can happen which change the event loop processing. It's not enough to just call uv_once and then expect that value to be correct, because it could change on the next call to uv_once (but before the internal kqueue call that blocks). It's not enough to call it twice because it could change on the second call, etc, etc : ) And kqueue is designed to minimize syscalls so that the same call you use for polling also updates the state of what is being polled, so it's sort of update/check in a single step, so it's hard to just "update" the kqueue state so that you could do the polling on your own via backend_fd, because there could be uncommitted pending changes internally in libuv that weren't updated on the kqueue fd. And calling uv_loop_once could cause more JavaScript / Node / etc code to run creating more uncommitted pending changes, etc. I remember looking at a few approaches around this but basically because outside code can run in some many different points in the cycle, and all of the points that code runs can change the event loop, add timers, etc, etc, really the only reasonable point to have a consistent state of the world is the actual kevent() call itself.

[piannucci]

So the question seems to be, is there a way to call uv_run that (a) does not block and (b) guarantees that on return there are no uncommitted changes to the kqueue.

[deanm]

@piannucci My memory is rusty, but basically I think the point is looking at something like http://docs.libuv.org/en/v1.x/_images/loop_iteration.png you just want to replace the "Poll for I/O" block, but uv_once is doing everything, so I don't think it's enough to do what you're proposing because you also need to be able to run everything that happens before polling for IO... Really what you want to do is just hook that point in the loop which is what Plask is doing. What everyone else is proposing is somehow running through the entire loop, and then handling the poll outside of that, which definitely can be made to work in theory, but it will effectively mean doing something to break up that diagram into 3 blocks basically so that you can run all the libuv preparation stuff, doing the poll, and then run all of libuv's post handling.

[deanm]

@piannucci my opinion is that the kqueue hook is the best approach, if libuv doesn't want to accept the patch (I had figured they wouldn't so I didn't even try), if I were you I would just use a custom patched version of libuv like Plask has been doing for years.

[piannucci]

Thank you. I'm interested in the broader architectural issues; part of the purpose of this pull request was to start a discussion about whether libuv's API for backend event loops is sufficiently robust. The specific proposal is not so critical to me.

[piannucci]

@deanm, one problem with your kevent mechanism is that callouts from the CFRunLoop may cause mutations to the uv loop, which will not result in the kqueue being reconfigured until the kevent hook returns control to uv_run. Unless the user adopts the discipline of e.g. sending a special NSEvent any time they mutate the uv loop, I don't see how the kevent hook can know when to break the CFRunLoop and return.

[piannucci]

I was going to suggest that uv_prepare_t is the answer, and that the call to uv__run_prepare should be moved from before uv__io_poll to inside uv__io_poll, after epoll_ctl/kevent(..., NULL, 0, NULL) and right before the blocking syscall. But that has the same problem. If the prepare_cb mutates the uv loop, then those mutations won't make it into the backend fd.

[bnoordhuis]

This PR seems stalled so I'll go ahead and close it out.

@deanm @piannucci If you still want to pursue this, can you open a new issue so we can hash out the details?