open_vins icon indicating copy to clipboard operation
open_vins copied to clipboard

Possible threading bug with dynamic initializer

Open nfriendlybrinc opened this issue 5 months ago • 0 comments

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

nfriendlybrinc avatar Jan 26 '24 19:01 nfriendlybrinc