Prevent instantiation of std::vectors in wait_for_work

[jacquelinekay]

For real-time performance completeness, there should be an alternative to instantiating and allocating dynamically sized data structures in Executor::wait_for_work, even if they contain small objects like shared pointers. This pull request abstracts out the vectors instantiated in wait_for_work and replaces them with a SharedPtrContainer interface. In the default memory strategy, this interface is implemented with a std::vector, and in the StaticMemoryStrategy, it is implemented with a std::array.

[jacquelinekay]

CI:

http://ci.ros2.org/job/ros2_batch_ci_linux/105/
http://ci.ros2.org/job/ros2_batch_ci_osx/46/
http://ci.ros2.org/job/ros2_batch_ci_windows/79/

[tfoote]

+1, though there is a merge conflict now

[dirk-thomas]

The referenced CI jobs also indicate new warnings.

[esteve]

+1

[jacquelinekay]

CI after rebase and warnings fix:

Linux and OSX green, no new warnings on Windows
http://ci.ros2.org/job/ros2_batch_ci_linux/119/
http://ci.ros2.org/job/ros2_batch_ci_osx/61/
http://ci.ros2.org/job/ros2_batch_ci_windows/88/

[dirk-thomas]

The two warning categories (C4996 and MSB3073) on Windows are currently also the case when building the master: see http://ci.ros2.org/job/ros2_batch_ci_windows/86/

[jacquelinekay]

Going to squash and merge if that's ok.

[dirk-thomas]

The SharedPtrContainer class seems to be essentially a std::vector and the StaticSharedPtrContainer a std::array. Instead of implementing our own base class as well as two subclasses would it be possible to just provide typedefs to the specific std storage classes and use them with there already existing identical API?

[jacquelinekay]

yes, a much "slimmer" solution is possible. I will investigate it.

[jacquelinekay]

The definitions for the different storage classes cannot be typedefs because typedefs cannot be templated (unless a different typedef is to be defined for each necessary template specialization of array, vector, etc). Perhaps a type alias is preferable.

However, the API for the two storage containers is not identical because std::array does not have push_back. The container class in this PR implements push_back for the static container, which causes the std::array to behave like a ring buffer. Because the APIs are not identical, I actually don't think a typedef or a type alias is a good idea.

Another detail I'm not clear on is how to override the definition made in MemoryStrategy (e.g. ContainerType -> std::vector) in StaticMemoryStrategy (ContainerType<T, Size> -> std::array<T, Size>), which is a subclass ofMemoryStrategy`.

Perhaps I should simply implement or find a ring buffer class with an identical (or similar) API to std::vector (since there is no std::ring_buffer) and then follow your original suggestion to provide a definition in Executor (ContainerType) that defaults to std::vector but can be overridden to the new ring buffer type.

That way, future changes to rclcpp that instantiate a vector-like container can be real-time safe.

[jacquelinekay]

After some discussion offline I've decided to close this PR and readdress the problem of implementing static/real-time safe alternatives to std::vector, std::map and std::string after further testing of rclcpp.