Skip to content
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

replace libev with libuv #6492

Open
garlick opened this issue Dec 9, 2024 · 1 comment
Open

replace libev with libuv #6492

garlick opened this issue Dec 9, 2024 · 1 comment

Comments

@garlick
Copy link
Member

garlick commented Dec 9, 2024

After a discussion in our meeting last week, I did an experiment to convert flux-core to use libuv.

First, I did some refactoring to localize all the libev usage to reactor.c. This involved reimplementing the buffer watchers, handle watcher, and zeromq watcher using the flux API instead of libev. Then I began systematically converting reactor.c to libuv. Although, as discussed, libuv derives from liibev and thus many of its idioms carry over, I ran into the following problems:

  • no equivalent to ev_child
  • no equivalent to ev_periodic
  • no equivalent to ev_stat (oops, I missed uv_fs_event_t)
  • no equivalent to ev_set_priority()
  • no equivalent to ev_ref()
  • memory for handle (libuv name for watcher) objects cannot be freed in flux_watcher_destroy(). One calls uv_close() on the handle (registering a callback), the reactor runs, and someday you get a close callback, after which memory can be freed. Grr. Because Windows I guess.

It's a fair bit of work to reimplement the missing watchers though probably not a huge deal. I'm not sure what to do about the destructor issue though. It doesn't map well onto our API.

This is on my port_libuv branch if anyone wants to take a look. libflux compiles with the missing functionality. The reactor unit test runs successfully, but when run under valgrind a bunch of invalid reads show up because watcher memory is freed too soon. memory is leaked. There is also an unexplained segfault in the composite watcher test.

garlick added a commit to garlick/flux-core that referenced this issue Dec 10, 2024
This is a partial conversion with the following gaps:
- no periodic watchers
- no child watchers
- no stat watchers
- flux_reactor_active_incref/decref
- no flux_watcher_set_priority

Memory will leak if watchers are destroyed outside of the reactor
because uv_close() works asynchronously.

Currently there is an unexplained segfault in the composite watcher
unit test.

See issue flux-framework#6492
@vchuravy
Copy link
Contributor

I hadn't had time in a while to look at flux, but from my perspective this would "fix" #3969 since the two event-loops that were fighting were libuv and libev (with some Julia idiosyncracies on top).

garlick added a commit to garlick/flux-core that referenced this issue Dec 10, 2024
This is a partial conversion with the following gaps:
- no periodic watchers
- no child watchers
- flux_reactor_active_incref/decref
- no flux_watcher_set_priority

Memory will leak if watchers are destroyed outside of the reactor
because uv_close() works asynchronously.

Currently there is an unexplained segfault in the composite watcher
unit test.

See issue flux-framework#6492
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants