Skip to content

Watchdog: the adapter aborts if destroyed before the watchdog thread starts, and an expired watchdog is never re-registered #116

Description

@bigsamich

RedisAdapter version or revision

main at ff2d5b8; unchanged through the #108 → #109 → #111 stack (0c3b0e6).

Redis version and deployment mode

Redis 7.4.11 standalone (TCP).

Observed behavior

There are two defects in the RA_Options::dogname watchdog.

1. std::terminate when the adapter is destroyed shortly after construction.

The usual start-up check is enough to trigger it:

RedisAdapter adapter(base, options);
if (!adapter.connected()) return 1;

The watchdog thread sets _watchdog_run = true itself, in the for initialiser, and only after addWatchdog() has made its HSET and HEXPIRE round trips. ~RedisAdapter joins the thread only if (_watchdog_run.load()). If the destructor runs first, the join is skipped, and member destruction then destroys a joinable std::thread, which calls std::terminate.

If the thread has meanwhile reached wait_for, the process can instead hang in ~condition_variable. This was observed with Redis stopped, where connected() takes about 100 ms (gdb: main thread in pthread_cond_destroy inside ~RedisAdapter, watchdog thread in condition_variable::wait_for).

2. The watchdog field is never re-created once it expires.

The thread calls addWatchdog() (HSET and HEXPIRE) once, and from then on only petWatchdog() (HEXPIRE). HEXPIRE cannot create a field: for a missing field or key it returns -2. petWatchdog treats that as success (reconnect(… != -1)), so nothing is logged and nothing re-registers. The TTL is 1 s and the pet period is 900 ms plus a round trip, so a single failed pet lets the field expire for good. From then on the process is missing from getWatchdogs(), and from any monitor that reads {base}:watchdog, for the rest of its life, even though it is healthy and connected. docs/api.md promises that dogname will "maintain a one-second field-TTL watchdog".

Expected:

  • destroying the adapter at any time joins the watchdog thread;
  • the watchdog re-appears within one period once Redis is reachable again.

Reproduction

For (1), with Redis reachable:

#include "RedisAdapter.hpp"
#include <cstdio>

int main() {
  RA_Options options;
  options.dogname = "dog";
  RedisAdapter adapter("reproC2", options);
  if (!adapter.connected()) { std::fprintf(stderr, "unreachable\n"); return 1; }
  return 0;
}

This aborts in 10 of 10 runs on main (terminate called without an active exception, exit 134). The stack head behaves the same.

For (2), identical on main and the stack head, with getWatchdogs() / HKEYS {base}:watchdog sampled every 0.5 s:

trigger result
Redis started 1 s after the adapter connected() is 1 from t=2.0 s, but getWatchdogs().size() stays 0 through t=5.0 s
CLIENT KILL TYPE normal at 2.2 s, adapter otherwise idle the adapter reconnects, but HKEYS stays [] from t=3.0 s to t=6.1 s
Redis restarted without persistence connected() is 1 from t=3.5 s, but there are 0 watchdogs through t=6.0 s

Build and runtime environment

  • Ubuntu 24.04, gcc 13.3
  • pinned redis-plus-plus 1.3.15 and hiredis 1.3.0
  • a private Redis 7.4.11 in a container

Suggested fix

Validated: the (1) reproducer exits cleanly 20/20, the watchdog is restored in all three (2) scenarios, and the stock gtest suite passes, including RedisAdapter.Watchdog.

  1. Set _watchdog_run = true in the constructor, before starting the thread, and loop with for (; _watchdog_run && _watchdog_cv.wait_for(lk, 900ms) == cv_status::timeout; …).
  2. In ~RedisAdapter, set the flag false, notify, and join whenever _watchdog_thd.joinable().
  3. In the loop, re-register when HEXPIRE reports -2. For example: auto r = _redis.hexpire(key, dog, 1); if (r == -2) addWatchdog(dog, 1); else reconnect(r != -1);. Log when the field had to be re-created.
  4. Consider more slack between the pet period and the TTL, so that one slow pet can't expire the field.
  5. Optionally, give the condition variable a real member mutex and a predicate wait, so a notify can't be lost. Today a lost notify can add up to 900 ms to the join.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    bugSomething isn't workingmust fix

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions