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

Add an overload to monitor that calls a lambda in the context of self #1423

Closed
dominiklohmann opened this issue Jun 12, 2023 · 4 comments
Closed
Assignees
Milestone

Comments

@dominiklohmann
Copy link
Contributor

dominiklohmann commented Jun 12, 2023

This feature request was discussed in Gitter1.

To make our code more composable and to express concerns more locally, I am looking for a mechanism that allows me to add a lambda to self->monitor(dest) as an additional parameter.

The implementation for this probably looks something like this:

auto monitor(const auto& dest, auto fn) -> disposable {
  dest->attach_functor([self, destaddr=dest->address(), fn=move(fn)](error& reason) mutable {
    unsafe_send_as(destaddr, self, make_action([self, fn=move(fn), reason=std::move(reason)]() mutable {
      move(fn)(move(reason));
    }));
  });
  // Q: How do I make the action or functor disposable so I can cancel this?
}

Ideally the function returns a disposable to use the new mechanism for cancelling, and there are probably better ways to express this without going through attach_functor to avoid the additional indirection.

I think the advantages over the current monitor and demonitor APIs are very obvious, especially if—as discussed in Gitter1—down messages may still arrive after demonitoring an actor.

There are two follow-up questions for this, assuming this gets implemented:

  1. Should the current monitor, demonitor, and set_down_handler APIs get deprecated for this, or
  2. should the new overload also call the down handler?

Footnotes

  1. https://app.gitter.im/#/room/#actor-framework_chat:gitter.im/$2oP8-OPG92oBdZ49SwZKJ_Ze6PeZWcN9pXgjS5XN_mE 2

@Neverlord
Copy link
Member

There are two follow-up questions for this, assuming this gets implemented:

  1. Should the current monitor, demonitor, and set_down_handler APIs get deprecated for this, or
  2. should the new overload also call the down handler?

As mentioned in the stashing ticket: I lean towards removing the special handlers altogether. So for this ticket, this would mean using your new monitor signature and dropping the old way eventually.

For the deprecation cycle, I think we can define the old monitor overload in terms of the new one. Something like this:

void monitor(const actor_addr& whom) {
  monitor(whom, down_handler_);
}

// Q: How do I make the action or functor disposable so I can cancel this?

FYI, actions are disposables. You can simply call as_disposable . 🙂

@Neverlord Neverlord added this to the 1.0.0 milestone Jan 5, 2024
@Neverlord
Copy link
Member

@riemass we'll also need monitor(node, callback) for monitoring entire CAF nodes.

@riemass
Copy link
Member

riemass commented Mar 2, 2024

Here's an update:
Implemented with attach_functor, very similar to the idea in the ticket. The function returns a disposable that stops the monitoring when called. We have a counterintuitive API now:

self->monitor(other, [](const error& err) { /* do some stuff */ });  // Attaches a functor, disposable is ignored
self->demonitor(other); // Does nothing, because we are not officially monitoring other...

For the deprecation cycle, I think we can define the old monitor overload in terms of the new one. Something like this:
Sadly, I didn't find an elegant way to detach the attached functor, because currently we can't identify which one is doing the monitoring. So this implementation would break our current demonitor.

Right now, disposing the returned disposable will still call unsafe_send_as,the system will see a message being sent, but won't call the lambda. I could rearrange the set up so that disposing the action would never send the message, (i.e. never call unsafe_send_as) but that would mean it is impossible to dispose the lambda while the message is in-flight and enqueued.

@Neverlord
Copy link
Member

@riemass we'll also need monitor(node, callback) for monitoring entire CAF nodes.

Since this is closely tied to the I/O module, we'll postpone this API. We currently don't know how a caf.net-based replacement for the I/O module functions will look like, so we leave this features as-is for now.

The new callback-based monitor version just landed. Closing.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants