Face: use Signal
Change Face API to use
#5 Updated by Junxiao Shi over 4 years ago
- Status changed from In Progress to Resolved
- % Done changed from 0 to 80
Change won't compile until #2329 is merged.
I'm unsure about one question:
EventEmitter::clear() is called in
FaceTable::remove. But there's no
FaceTable::remove does not attempt to disconnect signal connections, and most connections are not saved and are not disconnected until Face itself is destructed.
This doesn't break any test case.
Could this cause other potential problems?
#6 Updated by Davide Pesavento over 4 years ago
It depends on when the face is destructed... unfortunately there is no guarantee on that, but it seems cleaner to me if the FaceTable removes its subscriptions when it's done with the face. And this covers
As far as
onSend(Interest|Data) are concerned, I don't know why they are cleared in the current code, maybe for the same reason as #1718?
#7 Updated by Junxiao Shi over 4 years ago
- Status changed from Resolved to In Progress
note-6 pointed out the problem: shared_ptr to the face itself in bound functions would prevent the face from being destructed.
Connection for a handler which holds a shared_ptr to the face must be stored somewhere, and be disconnected upon
#8 Updated by Davide Pesavento over 4 years ago
On the other hand, wouldn't it be more appropriate to use a
weak_ptr in those cases?
More generally, we should really define who owns the various face objects... FaceTable or the protocol factories? or someone else? Right now they aren't really owned by anyone... in fact they're collectively owned by anything that has a
shared_ptr to them, which doesn't give many guarantees, and that makes it harder to reason about object lifetime, destruction and ownership cycles.