Revert "Revert "Store the subscriber, client, service and timer""#449
Revert "Revert "Store the subscriber, client, service and timer""#449
Conversation
|
Hey @wjwwood, Yes, I see the problem. Sorry about that. Looking to clean that up now. |
|
Ok, I found at least one of the root cause of the issue. it is explained here: http://floating.io/2017/07/lambda-shared_ptr-memory-leak/ |
|
No worries, that's why we have CI and nightlies 😄. It's not a hurry. I reverted it, so it's not affecting other people. I'll also have a look at it tomorrow if I can. |
|
Ok, I've got a fix for these. @wjwwood could you please have a look and tell me if it is acceptable? Note, that for simplicity purposes, the node_handle shared pointer (which does not work with lambda) is replaced by a weak pointer. This means that if the node handle is destructed before the service / subscriber handle, the rcl datastructure will leak (since the fini destructor needs a valid node pointer). |
|
I'm also looking at UT this change but currently struggling to come up with a B/W test. |
|
@guillaumeautran the patch looks pretty good to me, can you open a pull request against this pull requests's branch ( |
Converts all rcl_*_t types in the memory allocation strategy to shared pointers to prevent crash happening when a subscriber is reset. Issue: #349
|
CI:
|
|
I carefully went through the failures on each CI job and I believe them all to be unrelated this time. So I'll merge this. Thanks @guillaumeautran for helping fix it up! |
Similar to the warnings when remapping to invalid namespaces, this better communicates failures to the user. Resolves ros2#449 Signed-off-by: Jacob Perron <jacob@openrobotics.org>
Reverts #448
@guillaumeautran and @deng02 FYI.