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

eliminate flux_reactor_active_incref() #6510

Closed
garlick opened this issue Dec 16, 2024 · 1 comment
Closed

eliminate flux_reactor_active_incref() #6510

garlick opened this issue Dec 16, 2024 · 1 comment

Comments

@garlick
Copy link
Member

garlick commented Dec 16, 2024

Problem: flux_reactor_active_incref() and flux_reactor_active_decref() have no users and have a questionable design:

  • From 5bd6620: they only work with certain watchers

    The use of flux_reactor_active_incref/decref() only works with
    simple watchers that only implement a single watch. More complex
    watchers, which may have multiple watchers internal to it, cannot
    use this mechanism.

  • From libflux/reactor: add flux_reactor_active_incref(), _decref() #2387: the interface is lame

    My only worry is that incref/stop phase of this approach will be pretty easily missed.

  • From replace libev with libuv #6492: they cannot be provided if we swap out libev for libuv

I would vote that we remove these from the public API now and if the need arises for them again, we try to implement an interface like the per-watcher ("handle" in libuv speak) uv_ref().

@garlick
Copy link
Member Author

garlick commented Dec 16, 2024

I missed that this is used in flux-start(1) and in the python bindings in handle.py::reactor_run().

The flux-start usage is easily worked around. The python one, added in c1afc01, might require a little more thought, or possibly argue for the uv_ref() style interface above.

garlick added a commit to garlick/flux-core that referenced this issue Dec 20, 2024
Problem: flux_reactor_active_incref/decref() is an inferior interface
compared to flux_watcher_ref/unref().

Drop these reactor methods.
Drop unit test.

Fixes flux-framework#6510
garlick added a commit to garlick/flux-core that referenced this issue Dec 22, 2024
Problem: flux_reactor_active_incref/decref() is an inferior interface
compared to flux_watcher_ref/unref().

Drop these reactor methods.
Drop unit test.

Fixes flux-framework#6510
garlick added a commit to garlick/flux-core that referenced this issue Dec 23, 2024
Problem: flux_reactor_active_incref/decref() is an inferior interface
compared to flux_watcher_ref/unref().

Drop these reactor methods.
Drop unit test.

Fixes flux-framework#6510
@mergify mergify bot closed this as completed in 177932a Dec 23, 2024
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

1 participant