MongooseIM
MongooseIM copied to clipboard
esl
Refcount for hook handlers
Currently, if you try to add the same handler to the same hook with the same sequence number the call is ignored, handlers are not duplicated; but if you then remove this handler, it is removed. It may lead into issues when two modules use the same handler and one of them is unloaded. We could introduce sort of a "refcount" to avoid this.
I may have a naive questions. Why there is a need to add the same handler for the same hook with the same seq number? Why changing the seq number (which makes ordering explicit) won't work?
Because if you add it again with different sequence number it is going to be executed twice, which may not be what you want.
Actually there does not seem to be a need for it now, but I can imagine a situation where you launch a pool of processes, which may grow and shrink, and those processes want to attach a certain handler. You don't want the handler to disappear when any of the processes terminates, but you want it to be gone when you take down the whole pool.
I get it. To be honest, IMO, adding handler in such a case should be done in a place starting the process or pool (before or after). The handler will not be executed in the context of a process from the pool anyway. It's true that you still want to remove the handler only if the pool stops.
True, that would be a good practice. My point is actually that current behaviour is inconsistent - either we allow multiple registration, then we should do the refcount, or we don't, then it should crash if you try to register something the second time. As far as I'm concerned we can go either way.
So... do we actually have a realistic use case for this? If not, for sake of consistent behaviour I'd vote for deleting #1542 and replacing it with the opposite: a crash/error when identical handler is executed twice. I'd even go for prohibiting adding the same handler for the same hook but with different Seq.
Good point, you're right. That would enforce care and consistency. I don't have a definite opinion which way we should go, so guys feel free to decide.
Are we going to implement what @fenek suggested here? It makes sense to me. Or do we close the issue?
There is a new gen_hook module now. it has the following logic:
- If the handler functions are different, they will be registered separately. On removal the function will need to be provided.
- If the handler functions are identical, a warning is logged. This means that there is some flaw in the module logic e.g. it is registering handlers for the same key for multiple hosts.
Anyway, the original issue is resolved, so I am closing this one.
