ros
FrequencyStatusParam is not thread safe
The Struct is storing pointers to the min_freq_ and max_freq_ that are being accessed from the FrequencyStatus class using scoped_locks protecting only the object itself but not the data that can be altered using the struct. Also, the data are not checked prior being dereferenced, but I guess this is not as important since it will most likely cause problems at the start time compared to the thread safety issue that could cause problems at any point of the lifetime of the application.
Hrm; that does seem like it's a potential issue, but I think it would be incredibly difficult to actually hit it. Is it causing trouble for you? Do you have a reproducible test case that exploits it?
The API has been like this for about 6 or 7 years, and at this point it is very widely used, so we should think carefully about compatibility before introducing changes.
I'm happy to accept a fix for this, particularly if it includes a test cases that exercises the issue.
I wouldn't really expect many scenarios where this can be exploited, since I doubt many users have actually changed the value of min_freq_ and max_freq_ dynamically at runtime.
However, I am currently developing a nodelet that would actually need this functionality for a production environment and I am having second thoughts on using it. The possibility of a segmentation fault is there and could cause crashes on the whole system.
I could propose a fix, but that would have to break the API, would this be acceptable?
An alternative API with a copy could be added to not break backwards compatibility.
I will develop this functionality and make a PR to further discuss on this.
Probably outdated. If still present, please open a new issue or raise a PR.
