Rename cancel() to delete()

[AndrewCarterUK]

Hi All,

It's popped up in other discussion threads about whether cancel() means "permanently disabled" or "deleted".

I can see how this confusion has come in, but I think there is an obvious case for the latter - and that we should rename the method to remove any doubt.

Without a method which performs the operation of freeing the resources associated with the loop by the driver, there is no way than an application using the loop can guarantee that it isn't leaking memory without destroying the loop object. For use in a long running application, it's a requirement that there is an operation which can be used to free these resources - thus it can not be considered an implementation detail.

Let the discussion begin!

[bwoebi]

A disadvantage is of delete(): whoever has experience with loops in e.g. C, the libraries are all calling it *_close(). (I don't know much about other languages…)

Also, delete, in my ears, just removes the watcher, but doesn't actually imply a disable (i.e. watcher deleted, but callback still working… somehow).

[AndrewCarterUK]

Maybe then detach() or free() or remove()?

[bwoebi]

Dunno detach() isn't bad, but it again has the same problems than cancel()… I prefer cancel(), because every event loop calls it cancel. It's consistent with the rest of the world.

Let's better just document somewhere that invalidation in general must free a watcher and all its associated memory before the next tick [before next tick is for technical reasons with e.g. libev].

This then also solves the problem with automatic invalidation on defer/delay: they need to free their watchers too.

[kelunik]

Also, delete, in my ears, just removes the watcher, but doesn't actually imply a disable (i.e. watcher deleted, but callback still working… somehow).

How should that work? If the watcher is deleted, there's no information stored about it anymore, so no callback.

A disadvantage is of delete(): whoever has experience with loops in e.g. C, the libraries are all calling it *_close().

That would be an argument to call it close, but not to keep cancel.

[bwoebi]

Deleted, yes, but the callback may still be registered with the loop itself.

Okay, that was wrong … It's event_del (libev/ent), uv_cancel and uv_close… Okay, let's forget that.

[bwoebi]

Also, the permanently disabled vs. vanished discussion is somewhat semantics. We just need to somewhere document the semantics precisely and we're fine.

[kelunik]

Deleted, yes, but the callback may still be registered with the loop itself.

Why do you assume this? You just deleted the watcher, so why should the callback still be kept?

Also, the permanently disabled vs. vanished discussion is somewhat semantics.

Depending on that it might be OK to reuse watcher IDs or not.

[bwoebi]

I do assume nothing. I'm just saying that the specification does not explicitly specify that; and what a spec does not specify is up to implementations. (which may then do what we expect or not.)

[kelunik]

If it says it deletes a watcher, then that's the complete watcher for me, so also the callback, because the callback is part of the watcher.

[AndrewCarterUK]

I think it's clear that we need to add to the specification that this method (whatever we call it) can be used to remove all traces of the watcher id from the loop. Thus, it will have the same effect as disable() but also the further effect that the watcher id will become invalid unless reissued again.

[bwoebi]

See, that's what you assume. If you say "deletes a watcher and all its associated resources", that's explicit.

In case we get this specified, I don't see any reason why to not use cancel().

[kelunik]

See, that's what you assume.

Otherwise the watcher isn't deleted.

[AndrewCarterUK]

I'm happy with using cancel() and adding this to the specification. Only opening the thread for discussion as I thought a different name might make it clearer.

I'll leave this issue open until tomorrow, and if there is no opposition to proceeding with cancel() and clarifying the specification I'll proceed with a PR and close this.

[bwoebi]

Closed via merge of #75.