Task #2300
closedFace: use Signal
Added by Junxiao Shi almost 10 years ago. Updated almost 10 years ago.
100%
Description
Change Face API to use ndn::util::signal::Signal
in on(Receive|Send)(Interest|Data)
.
Updated by Junxiao Shi almost 10 years ago
- Blocks Bug #2213: EventEmitter::clear() leads to double free when called from Face::fail() added
Updated by Junxiao Shi almost 10 years ago
- Blocks Feature #1999: Strategy for access router added
Updated by Junxiao Shi almost 10 years ago
- Status changed from New to In Progress
- Assignee set to Junxiao Shi
Updated by Junxiao Shi almost 10 years ago
- Blocked by Bug #2329: Signal: emitSignal doesn't compile when used with non-zero-argument Signal added
Updated by Junxiao Shi almost 10 years ago
- Status changed from In Progress to Resolved
- % Done changed from 0 to 80
http://gerrit.named-data.net/1570
Change won't compile until #2329 is merged.
I'm unsure about one question:
Previously, EventEmitter::clear()
is called in FaceTable::remove
. But there's no Signal::clear()
.
Currently, 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?
Updated by Davide Pesavento almost 10 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 onReceive(Interest|Data)
and onFail
.
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?
Updated by Junxiao Shi almost 10 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.
Thus, Connection
for a handler which holds a shared_ptr to the face must be stored somewhere, and be disconnected upon onFail
.
Updated by Davide Pesavento almost 10 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.
Updated by Junxiao Shi almost 10 years ago
- Blocked by Feature #2331: Signal: once added
Updated by Junxiao Shi almost 10 years ago
Updated by Junxiao Shi almost 10 years ago
- Status changed from In Progress to Code review
- % Done changed from 80 to 100
I have implemented note-10 solution, and checked UdpFace has no #1718 regression.
Updated by Junxiao Shi almost 10 years ago
- Status changed from Code review to Closed