rpng / open_vins

An open source platform for visual-inertial navigation research.
https://docs.openvins.com
GNU General Public License v3.0
2.16k stars 636 forks source link

Possible threading bug with dynamic initializer #415

Open nfriendlybrinc opened 8 months ago

nfriendlybrinc commented 8 months ago

I think there is a thread safety issue during initialization when the use_multi_threading_subs parameter is set to true. The dynamic initializer twice calls _db->get_internal_data() and then iterates over the contents of the data (lines 51 and 87). Best I can tell, this becomes an issue if another thread causes the data to get modified. While the feature database methods have a lock to prevent simultaneous modification to the database itself, there's nothing to prevent simultaneous modification of the features themselves. You can get unprotected access to the features via the following database methods get_internal_data(), features_not_containing_newer(), features_containing_older(), and features_containing().

As a quick check, I modified those methods to return const pointers to the features. This showed there are multiple places in the code base that modify features via these methods.

https://github.com/rpng/open_vins/blob/17b73cfe4b870ade0a65f9eb217d8aab58deae19/ov_init/src/dynamic/DynamicInitializer.cpp#L51