Skip to content

Commit 763e77a

Browse files
committed
test(PrimaryClient): add regression test for stop() teardown during stuck reconnect
Covers the second symptom of issue #368: PrimaryClient::stop() (the implementation behind UrDriver::stopPrimaryClientCommunication()) must return promptly when the producer thread is stuck in its reconnect loop against an unreachable robot, instead of blocking on the pipeline join. Also asserts the stop()/start() restart path reconnects, verifying the sticky cancellation flag is cleared via clearStop() on (re)start.
1 parent e9c46b3 commit 763e77a

1 file changed

Lines changed: 65 additions & 0 deletions

File tree

tests/test_primary_client_reconnect.cpp

Lines changed: 65 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -117,6 +117,71 @@ TEST(PrimaryClientReconnectTest, destructor_not_blocked_by_stuck_reconnect_threa
117117
<< " ms — the producer reconnect thread was not aborted by requestStop()";
118118
}
119119

120+
// Regression test for the SECOND symptom reported in issue #368: calling
121+
// PrimaryClient::stop() (the implementation of UrDriver::stopPrimaryClientCommunication())
122+
// hangs when the producer thread is stuck in its reconnect loop against an unreachable
123+
// robot.
124+
//
125+
// This is distinct from the destructor test above: stop() is a restartable operation, so
126+
// besides asserting that it returns promptly it must also leave the client in a state where
127+
// a subsequent start() can reconnect. That exercises the clearStop() reuse path in
128+
// URProducer::setupProducer()/PrimaryClient::reconnectStream() — a sticky-flag regression
129+
// there would not hang teardown but would silently prevent the client from ever reconnecting
130+
// after a stop().
131+
TEST(PrimaryClientReconnectTest, stop_not_blocked_by_stuck_reconnect_thread)
132+
{
133+
comm::INotifier notifier;
134+
135+
auto server = std::make_unique<FakePrimaryServer>(primary_interface::UR_PRIMARY_PORT);
136+
auto client = std::make_unique<primary_interface::PrimaryClient>("127.0.0.1", notifier);
137+
138+
// Unlimited reconnect attempts with a large reconnection time: if the fix is
139+
// absent, the producer's reconnect path keeps stop()'s pipeline join blocked.
140+
const std::chrono::milliseconds large_reconnect_timeout(5000);
141+
ASSERT_NO_THROW(client->start(/*max_num_tries=*/0, large_reconnect_timeout));
142+
ASSERT_TRUE(server->waitForClient()) << "PrimaryClient never connected to the fake server";
143+
144+
// Drop the server. The producer's read() fails, the socket transitions to
145+
// SocketState::Disconnected, and the producer enters its reconnect loop.
146+
server.reset();
147+
148+
// Give the producer time to detect the drop and reach its reconnect sleep
149+
// (initial backoff is 1 s, after which it sleeps inside TCPSocket::setup()).
150+
std::this_thread::sleep_for(std::chrono::milliseconds(1500));
151+
152+
// stop() must return quickly: requestStop() aborts the producer's connect attempt/back-off,
153+
// so the pipeline join completes well under 2 s. Without the fix this blocks for at least the
154+
// reconnect timeout (and indefinitely with unlimited retries against a dead port). Run it on a
155+
// worker with a watchdog so a regression fails fast with a clear message instead of hanging the
156+
// test binary (the CTest TIMEOUT then reaps it).
157+
std::packaged_task<void()> stop_task([&client]() { client->stop(); });
158+
auto stop_future = stop_task.get_future();
159+
std::thread stop_thread(std::move(stop_task));
160+
161+
const auto t0 = std::chrono::steady_clock::now();
162+
if (stop_future.wait_for(std::chrono::seconds(5)) == std::future_status::timeout)
163+
{
164+
stop_thread.detach();
165+
FAIL() << "PrimaryClient::stop() did not return within 5 s — the producer reconnect thread was not aborted by "
166+
"requestStop()";
167+
}
168+
stop_thread.join();
169+
const auto elapsed = std::chrono::steady_clock::now() - t0;
170+
171+
EXPECT_LT(elapsed, std::chrono::seconds(2))
172+
<< "PrimaryClient::stop() blocked for " << std::chrono::duration_cast<std::chrono::milliseconds>(elapsed).count()
173+
<< " ms — the producer reconnect thread was not aborted by requestStop()";
174+
175+
// Restart-reuse check: bring up a fresh server and start() again. This must reconnect,
176+
// proving that stop()'s sticky cancellation flag was cleared via clearStop() on restart.
177+
auto server2 = std::make_unique<FakePrimaryServer>(primary_interface::UR_PRIMARY_PORT);
178+
ASSERT_NO_THROW(client->start(/*max_num_tries=*/0, large_reconnect_timeout));
179+
EXPECT_TRUE(server2->waitForClient(std::chrono::seconds(3))) << "PrimaryClient did not reconnect after "
180+
"stop()/start() — the cancellation flag set by "
181+
"stop() was not cleared "
182+
"via clearStop() on restart";
183+
}
184+
120185
int main(int argc, char* argv[])
121186
{
122187
::testing::InitGoogleTest(&argc, argv);

0 commit comments

Comments
 (0)