Repository navigation
Don't use a destroyed ClassLoader when deleting its instances - #240
Conversation
Deleters for managed instances captured the ClassLoader's `this`, so releasing an instance after its ClassLoader was destroyed was a heap-use-after-free. Have the deleters share a small context with the loader instead; the loader clears it on destruction, after which a deleter just deletes the object (the library is not unloaded while instances exist). Fixes the lifetime half of #199 without changing the API. Signed-off-by: Michael Carroll <mjcarroll.oss@gmail.com>
|
Pulls: #240
|
|
the fix looks good to me, but i do not have power to merge this. |
|
Pulls: #240
|
…oader-state Signed-off-by: Michael Carroll <mjcarroll.oss@gmail.com> # Conflicts: # src/class_loader.cpp
|
stupid 10 hr timeout on the linux job, but rpr covers about the same. |
Deleters for managed instances captured the
ClassLoader'sthis. Releasing an instance after itsClassLoaderwas destroyed was therefore a heap-use-after-free inonPluginDeletion().Now the deleters share a small context with the loader, which the loader clears when it is destroyed. After that, a deleter just deletes the object. The library is never unloaded while instances exist, so the object's code is still loaded.
This fixes the lifetime half of #199 without the API change (no
ClassLoader::Make()), so the rclcpp and demos patches aren't needed. The leak half of #199 was already fixed by #201 and #205. Thanks to @tylerjw for the original analysis.Adds
ClassLoaderLifetimeTest.instancesOutliveLoader. Without the fix, it fails under AddressSanitizer.