Thread and Address Sanitizer CI

[tylerjw]

Signed-off-by: Tyler Weaver [email protected]

This adds a github actions CI job for tsan and asan.

This can be tested by building locally with the new cmake variables added. For example building and testing with colcon can be done like this:

tsan:

colcon build --cmake-args -DENABLE_SANITIZER_THREAD=ON -DCMAKE_BUILD_TYPE=Debug
colcon test --event-handlers console_direct+

asan:

colcon build --cmake-args -DENABLE_SANITIZER_ADDRESS=ON -DCMAKE_BUILD_TYPE=Debug
colcon test --event-handlers console_direct+

I'm creating this with the hope that we can fix the errors found by ASAN and TSAN here so that projects that depend on this library can use ASAN and TSAN themselves.

Locally I've applied this change over #186 and was able to observe it reduced the number of errors found by ASAN.

[tylerjw]

Note that we should probably figure out how to run these changes in CI from this PR before merging this (to test the CI config works as expected). Also, this probably shouldn't be merged until asan and tsan tests pass (they don't locally).

The reason I created this was to make it easier to test this project locally with asan and tsan to begin fixing memory errors and race conditions.

[gbiggs]

I think merging it while the asan and tsan tests don't pass is OK, but I agree with the need to have it running from CI.

[tylerjw]

I'll see if I can get it to run in my fork to test if it runs.

[tylerjw]

@gbiggs I created this PR within my own fork to test that these work as expected: tylerjw#1

[tylerjw]

I spent today trying to figure out these memory leaks and while I don't have a PR yet I'm getting closer using a combination of asan and valgrind. I hope to have a PR soon that cleans up all manual memory management.

[tylerjw]

I'm a little confused by the heavy usage of recursive mutex, does anyone know if there are specific use cases that explain why this repo uses recursive mutexes and if hopefully those use cases are reflected in the tests?

[tylerjw]

I pushed a rebase of this to see if the latest merged changes fix the bug found by asan.