Skip to content

Commit de05195

Browse files
committed
fix(sdks): provider reconnect across csharp/cpp/go/python/java demos
All six language demos stayed offline after the agent restarted (incident 2026-08-16 05:33 / 07:38 UTC); only the JS demo recovered. Root causes per SDK (all on the SDK->Agent local link): - csharp: on remote close the read loop failed pending requests with SetCanceled, which the heartbeat loop misread as graceful shutdown and exited, so ReconnectAsync never ran. Fail pendings with an exception instead, only break on real cancellation, and trigger reconnect when the transport is already disconnected. - cpp: heartbeat failure only logged and broke out of the loop; the reconnect_thread_ state existed but was never started from this path. Run a blocking reconnect loop on the heartbeat thread (self-join-safe via detached thread + thread-id guard). - go: TCPClient.Call has no deadline, so a heartbeat on a half-dead connection blocked forever in the pending channel. Add a 30s timeout on heartbeat calls so the failure counter can trigger the existing reconnect path. - python: _recover_connection held _state_lock across blocking dial/ register calls, and _send_heartbeat/_send_drain_complete performed network I/O under the same lock; a blocked write stalled every state user (observed as a permanent futex wait on the heartbeat thread, tid 7, via /proc wchan). Snapshot state under the lock, do network I/O outside it. - java: the demo image ships only slf4j-api, so the NOP logger silently swallowed every log line and the container looked dead. Add slf4j-simple to the demo runtime classpath.
1 parent 4403210 commit de05195

6 files changed

Lines changed: 84 additions & 11 deletions

File tree

sdks/cpp/src/croupier_client.cpp

Lines changed: 47 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@
2222
#include <sstream>
2323
#include <stdexcept>
2424
#include <thread>
25+
#include <optional>
2526
#include <unordered_map>
2627

2728
// Logging macros with configuration support
@@ -329,6 +330,7 @@ class CroupierClient::Impl {
329330
std::string session_id_;
330331
std::thread heartbeat_thread_;
331332
std::atomic<bool> should_stop_heartbeat_{false};
333+
std::optional<std::thread::id> heartbeat_thread_id_;
332334
std::string last_error_;
333335

334336
// Reconnection state
@@ -612,7 +614,11 @@ class CroupierClient::Impl {
612614
void startHeartbeatLoop() {
613615
stopHeartbeatLoop();
614616
should_stop_heartbeat_ = false;
617+
// detach: the heartbeat thread may run the reconnect loop, and
618+
// Connect() inside it calls startHeartbeatLoop() again — a join there
619+
// would self-deadlock. Ownership is tracked via heartbeat_thread_id_.
615620
heartbeat_thread_ = std::thread([this]() {
621+
heartbeat_thread_id_ = std::this_thread::get_id();
616622
const auto interval = std::max(1, config_.heartbeat_interval);
617623
while (!should_stop_heartbeat_) {
618624
for (int elapsed = 0; elapsed < interval * 10 && !should_stop_heartbeat_; ++elapsed) {
@@ -628,10 +634,51 @@ class CroupierClient::Impl {
628634
last_error_ = e.what();
629635
connected_ = false;
630636
SDK_LOG_WARN("Heartbeat failed: " + last_error_);
637+
if (!should_stop_heartbeat_ && running_) {
638+
reconnectLoop();
639+
}
631640
break;
632641
}
633642
}
634643
});
644+
heartbeat_thread_.detach();
645+
}
646+
647+
void stopHeartbeatLoop() {
648+
should_stop_heartbeat_ = true;
649+
// Cannot join from the heartbeat thread itself (reconnect path calls
650+
// Connect() -> startHeartbeatLoop() -> stopHeartbeatLoop()).
651+
if (heartbeat_thread_.joinable()) {
652+
if (heartbeat_thread_id_.has_value() &&
653+
*heartbeat_thread_id_ == std::this_thread::get_id()) {
654+
return; // self-stop: thread will exit via should_stop_heartbeat_
655+
}
656+
heartbeat_thread_.join();
657+
}
658+
}
659+
660+
// Blocking reconnect loop used after a heartbeat/connection failure.
661+
// Retries with a fixed interval until Stop() is called or the
662+
// connection is re-established. Runs on the heartbeat thread.
663+
void reconnectLoop() {
664+
std::lock_guard<std::mutex> lock(reconnect_mutex_);
665+
if (is_reconnecting_.exchange(true)) {
666+
return; // another thread is already reconnecting
667+
}
668+
int attempt = 0;
669+
while (!should_stop_heartbeat_ && running_) {
670+
++attempt;
671+
SDK_LOG_INFO("Reconnecting to agent (attempt " + std::to_string(attempt) + ")...");
672+
if (Connect()) {
673+
SDK_LOG_INFO("Reconnected and re-registered after " + std::to_string(attempt) + " attempt(s)");
674+
is_reconnecting_ = false;
675+
return;
676+
}
677+
for (int waited = 0; waited < 5 * 10 && !should_stop_heartbeat_ && running_; ++waited) {
678+
std::this_thread::sleep_for(std::chrono::milliseconds(100));
679+
}
680+
}
681+
is_reconnecting_ = false;
635682
}
636683

637684
void stopHeartbeatLoop() {

sdks/csharp/src/Croupier.Sdk/CroupierClient.cs

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -719,9 +719,17 @@ private async Task HeartbeatLoopAsync(CancellationToken cancellationToken)
719719
try
720720
{
721721
await Task.Delay(interval, cancellationToken);
722+
723+
// The read loop may have marked the transport disconnected
724+
// without this loop observing a failed heartbeat yet.
725+
if (_transport is not { IsConnected: true })
726+
{
727+
throw new InvalidOperationException("Not connected to Agent");
728+
}
729+
722730
await SendHeartbeatAsync(cancellationToken);
723731
}
724-
catch (OperationCanceledException)
732+
catch (OperationCanceledException) when (cancellationToken.IsCancellationRequested)
725733
{
726734
break;
727735
}

sdks/csharp/src/Croupier.Sdk/Transport/TCPTransport.cs

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -362,10 +362,12 @@ private async Task ReadLoop(CancellationToken cancellationToken)
362362
}
363363
}
364364

365-
// Fail all pending requests
365+
// Fail all pending requests with an error (NOT cancellation): callers
366+
// like the heartbeat loop treat OperationCanceledException as normal
367+
// shutdown and would silently exit instead of reconnecting.
366368
foreach (var (reqId, tcs) in _pending)
367369
{
368-
tcs.SetCanceled(cancellationToken);
370+
tcs.TrySetException(new InvalidOperationException("connection closed"));
369371
}
370372
_pending.Clear();
371373
}

sdks/go/pkg/croupier/tcp_manager.go

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -412,7 +412,9 @@ func (m *TCPManager) sendHeartbeat(ctx context.Context) error {
412412
return fmt.Errorf("marshal heartbeat: %w", err)
413413
}
414414

415-
_, _, err = client.Call(ctx, protocol.MsgProviderHeartbeatRequest, reqBody)
415+
hbCtx, hbCancel := context.WithTimeout(ctx, 30*time.Second)
416+
defer hbCancel()
417+
_, _, err = client.Call(hbCtx, protocol.MsgProviderHeartbeatRequest, reqBody)
416418
return err
417419
}
418420

sdks/java/examples/Dockerfile

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,11 @@ COPY --from=build /out/croupier-java-sdk.jar /app/lib/croupier-java-sdk.jar
2222
COPY --from=build /out/classes /app/classes
2323
COPY --from=build /out/lib /app/lib
2424

25+
# The demo only ships slf4j-api; without a binding every log line is silently
26+
# swallowed (NOP logger), making the container look dead. Add slf4j-simple so
27+
# demo logs (connect/register/reconnect) are visible on stdout.
28+
ADD --chmod=644 https://repo1.maven.org/maven2/org/slf4j/slf4j-simple/2.0.16/slf4j-simple-2.0.16.jar /app/lib/slf4j-simple.jar
29+
2530
# The game-demo build occasionally bundles the SDK's own classes (io/github/.../wire)
2631
# into target/classes. Since the classpath puts /app/classes before the SDK jar,
2732
# those shadow copies would override the jar and pin stale wire encoding. Strip

sdks/python/croupier/__init__.py

Lines changed: 16 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -646,7 +646,7 @@ def _send_drain_complete(self) -> None:
646646
return
647647
transport = self._transport
648648

649-
# ProviderDrainCompleteRequest body is empty (no proto definition needed).
649+
# Network call outside the state lock.
650650
transport.call(protocol.MSG_PROVIDER_DRAIN_COMPLETE_REQUEST, b"")
651651
LOG.info("DrainComplete sent to Agent")
652652

@@ -673,23 +673,32 @@ def _heartbeat_loop(self) -> None:
673673
self._recover_connection()
674674

675675
def _send_heartbeat(self) -> None:
676+
# Snapshot state under the lock, then perform the network call
677+
# WITHOUT holding _state_lock: a blocked socket write must never
678+
# stall every other state user (observed as a permanent futex wait
679+
# on the heartbeat thread after the agent restarts).
676680
with self._state_lock:
677681
if not self._transport or not self._session_id:
678682
raise RuntimeError("Client is not registered")
679683
transport = self._transport
680-
request = provider_pb2.ProviderHeartbeatRequest(
681-
service_id=self._config.service_id,
682-
session_id=self._session_id,
683-
)
684-
req_data = request.SerializeToString()
684+
session_id = self._session_id
685+
service_id = self._config.service_id
686+
687+
request = provider_pb2.ProviderHeartbeatRequest(
688+
service_id=service_id,
689+
session_id=session_id,
690+
)
691+
req_data = request.SerializeToString()
685692

686693
transport.call(protocol.MSG_PROVIDER_HEARTBEAT_REQUEST, req_data)
687694

688695
def _recover_connection(self) -> None:
689696
while not self._heartbeat_stop.is_set():
690697
try:
698+
# Do NOT hold _state_lock across the (blocking) dial/register
699+
# network calls; only take it to publish the new state.
700+
self._connect_and_register()
691701
with self._state_lock:
692-
self._connect_and_register()
693702
self._connected = True
694703
LOG.info("Reconnected and re-registered service %s", self._config.service_id)
695704
return

0 commit comments

Comments
 (0)