Intermittent t/round_robin test failure during 2.0.0 release testing

[esabol]

t/round_robin failed on first attempt of my release testing (in an ubuntu:24.04 container) with the following:

tests/round_robin.cc:339: in _job_retry_TEST() pid(44277) Assertion 'uint32_t(limit.expected())' != 'uint32_t(limit.count())'
round_robin.--job-retries=10.GEARMAN_FATAL                                      [ failed ]

But when I ran make test again, it passed. 2.0.0 release is paused for the moment.

@p-alik : Any ideas? Release anyway?

[p-alik]

Root-caused this — it's a bug in the test itself, not in gearmand.

_job_retry_TEST() in tests/round_robin.cc starts the worker on a background thread but reads limit.count()/limit.expected() for its assertions right after gearman_client_do() returns, without ever joining that thread. The worker thread can still be mid-flight (reconnecting and running job_retry_WORKER(), which increments limit->_count) at the exact moment the main thread reads the same counter — a genuine, unsynchronized data race between the two threads.

Confirmed with ThreadSanitizer: building with -fsanitize=thread and looping the --job-retries=10 collection reproduced the race in 29/30 runs, at exactly this pair of locations:

Read of size 4 by main thread:
  Limit::count() tests/round_robin.cc:272
  _job_retry_TEST tests/round_robin.cc:339
Previous write of size 4 by thread T2:
  Limit::increment() tests/round_robin.cc:262
  job_retry_WORKER tests/round_robin.cc:315

That's a plausible explanation for the intermittent expected() != count() failure — no memory barrier guarantees the main thread sees the worker thread's last write.

Fix: join the worker thread (handle->shutdown()) before checking the counters — the same pattern already used a few tests up in this file (round_robin_epoch_does_not_block_regular_TEST). 50/50 clean runs under TSan after the fix (was 29/30 failing before), 10/10 clean in a normal build.

PR: #470. Should be safe to resume the 2.0.0 release once this lands — the flakiness is confined to this one test, not a real server bug.

[esabol]

Thanks again for the analysis. I noticed that t/protocol test FAILed initially here: https://github.com/gearman/gearmand/actions/runs/29280197849/job/86919231598?pr=470

Could that be a bug in the test as well, @p-alik ?

[esabol]

Oddly, these tests appear in the first attempt, but not the second attempt:

  PASS: libhostile/t/accept
  PASS: libhostile/t/close
  PASS: libhostile/t/pipe
  PASS: libhostile/t/pipe2

I'm not sure what's going on there!

Never mind. Ignore this. I just copied and pasted wrong when I was comparing the test results.

[p-alik]

Dug into the t/protocol failure @esabol flagged, plus the t/worker failure on the post-merge run — different root cause, unrelated to the round_robin fix in #470.

It's a pre-existing race in the shared libtest harness (libtest/server.cc:232): Server::start() releases a test's reserved port before actually forking/exec'ing the gearmand process, leaving a window where a different, concurrently-running test process (make check -j) can grab the same port first. That explains why it hits a different, unrelated test binary each time (saw t/vector fail the same way in a scheduled run from before #469 was even filed) and why it's intermittent/platform-dependent.

Filed separately as #471 with the full analysis, since it's a repo-wide test-infra issue, not specific to round_robin/#469.

[esabol]

Filed separately as #471 with the full analysis, since it's a repo-wide test-infra issue, not specific to round_robin/#469.

OK, re-closing this issue then. Thanks again!