testRebotHeartbeatCount hangs indefinitely

[smarsching]

When running the test suite, testRebotHeartbeatCount hangs indefinitely. Through a debugger, I found that this happens because testable_rebot_sleep::waitForClientTestableMode() (implemented in testableRebotSleep_testingImpl.h) never returns.

I think that this happens because this function waits for RebotSleepSynchroniser::_clientHasReachedTestableMode to be set, and the only function that does this is testable_rebot_sleep::sleep_until(…), which apparently is never called. At least I couldn’t find any place in the code where it is called.

I noticed this problem on macOS 12.6, but from looking at the code, this should happen on all platforms.

I am not quite sure what this test is supposed to do, so I don’t no where sleep_until(…) should be called.
@mhier, @killenb: It looks like the two of you wrote the code for this test. Maybe you can provide some inside into what is supposed to be happening there.

[phako]

Odd. This is fine on our Jenkins and I also just tested it locally. testable_rebot_sleep::sleep_until() is called in RebotBacked::heartbeatLoop(). There is some trickery going on to use the testable sleep only in the test, maybe it picks up the wrong version on macOS.

[killenb]

I suspected a race condition and ran the thread sanitizer. It does not complain, the test is running fine for me. It's been some time since this code was written. I will have to dig back in and see how it's done, and why.

[smarsching]

Thanks for checking. I had another look and it seems like testable_rebot_sleep::sleep_until() is called from code inside device_backends/Rebot/src/RebotBackend.cc. It just was a bit unexpected that code in device_backends has a dependency on code in tests, so I didn’t look there at first.

The problem seems to be that the implementation of testable_rebot_sleep::sleep_until() in device_backends/Rebot/src/testableRebotSleep.cc instead of the one in tests/include/testableRebotSleep_testingImpl.h is used.

So, this looks less like a race condition and more like a compilation or linking problem. I had a look at the CMakeLists.txt files, and it seems like testRebotHeartbeatCount is linked against libChimeraTK-DeviceAccess (tests/CMakeLists.txt line 28). However, this library includes device_backends/Rebot/src/testableRebotSleep.cc, so with tests/include/testableRebotSleep_testingImpl.h being included in tests/executables_src/testRebotHeartbeatCount.cpp, there now are two definitions of testable_rebot_sleep::sleep_until.

Actually, I find it strange that the linker does not complain about these duplicate symbols (neither on Linux / GCC nor on macOS / LLVM). So, I guess the difference between Linux and macOS is which of the two implementations gets choosen, but I think this really is a bug in the code because clearly there should only be a single definition.

I guess that the correct fix would be to not link testRebotHeartbeatCount with libChimeraTK-DeviceAccess. However, this would mean explicitly including all the sources that are needed, which might be a bit bothersome.

A less bothersome alternative might be implementing a registry mechanism for testable_rebot_sleep where the default implementation is used, but it can be overriden by making a function call and passing pointer to other implementations of the relevant functions before using RebotBackend.