🧑‍🌾 Windows test_executors regression: SEH Exception

[Crola1702]

Description

This is a consistent regression in windows release and windows repeated

Reference build:

• https://ci.ros2.org/job/nightly_win_rel/3337/
• https://ci.ros2.org/job/nightly_win_rep/3643/

Test regressions:

• rclcpp.test_executor.gtest.missing_result
• projectroot.test.rclcpp.test_executors_busy_waiting

Log output:

Running main() from C:\ci\ws\install\src\gtest_vendor\src\gtest_main.cc
[==========] Running 28 tests from 1 test suite.
[----------] Global test environment set-up.
[----------] 28 tests from TestExecutor
[ RUN      ] TestExecutor.add_remove_node_thread_safe
unknown file: error: SEH exception with code 0xc0000005 thrown in the test body.
Stack trace:


[  FAILED  ] TestExecutor.add_remove_node_thread_safe (33 ms)

[jmachowinski]

When did this start ?

[jmachowinski]

I just had a look at this, I stumbled over this a few days back. The test is kind of broken as it deletes the node before the executor stopped using it. I would temporarily disable the test, until we agreed on a patch for rclcpp::Executor::remove_node().

[fujitatomoya]

i think that is okay to disable the test for now.

originally #1505 just adds the thread safety locks for the executor.
if that is so, how about changing the test to make sure that destroys nodes after executor stopped? and we can probably keep the current test (deleting the nodes before executor stops) as comment and enable once the rclcpp::Executor::remove_node() is fixed

[alsora]

We discussed this issue during the waffle meeting and we think that it would be good to disable the test temporarily.
However it's important that we follow-up with work to fix the root cause.

@Crola1702 do you know when this bug started and which changes could have caused it?