TimeoutService: keep TimeoutWatchdog state per art schedule

[oksuzian]

Intent

TimeoutWatchdog is declared SHARED, but all of its per-event state lived in a single member, State tls_. The name is left over from the thread_local version in #1777; that storage was removed when the service moved from LEGACY to SHARED to pass the MT test. In a job with more than one schedule, the sPreProcessEvent callback runs on every schedule at once. Each call rewrites that one member, including a move-assignment of the same std::stop_source. That is a data race, and it happens in any multi-schedule job that loads Services.Reco or Services.SimAndReco, even with every budget at 0. Mu2eG4/fcl/g4test_03MT.fcl (5 schedules, Services.SimAndReco) is one such job. Once a budget is enabled, it gets worse: one schedule starting a new event would replace the deadline and stop token of a module still running on another schedule.

Change

• The state now lives in an art::PerScheduleContainer<State>. It is sized with expand_to_num_schedules() in the constructor and never resized, so each schedule reads and writes only its own element and no lock is needed.
• Every entry point (startEvent, startModule, endModule, check, stopToken, eventDeadline, moduleDeadline) takes the art::ScheduleID it acts on, and access goes through the bounds-checked at().
  • The pre-event callback passes ScheduleContext::id().
  • ModuleGuard takes the ID once at construction and stores it, so code calling guard.check() / guard.stopToken() does not change.
• The two in-tree consumers, TZClusterFinder and AgnosticHelixFinder, now pass their legacy-module scheduleID() to ModuleGuard. A GitHub code search finds no other user of the service in Offline, Production or mu2e-trig-config.
• art::Utilities is added to the library's link list, since the library now uses it directly.
• Two small changes on lines the patch already touches:
  • The startModule comment said each module starts with a fresh stop token. The code does the opposite: a stop request is sticky for the rest of the event. The comment now says so, and the existing FIXME is unchanged.
  • The debugLevel > 1 event print now includes the schedule number.

Single-schedule jobs behave exactly as before.

Validation

• Compiled TimeoutWatchdog.cc, TimeoutWatchdog_service.cc, TZClusterFinder_module.cc and AgnosticHelixFinder_module.cc with the buildtest flags (-std=c++20 -Wall -Werror -pedantic -O3 ..., envset p106). Linked the library and service plugin with -Wl,--no-undefined.
• Ran a job with the patched service and no modules: EmptyEvent, 200 events, @table::Services.Reco, eventTimeoutMs : 1000, debugLevel : 2.
  • num_schedules : 5 gives exit 0, with events on all five schedules (41/38/43/37/41). No at() threw, so the container is sized correctly when the service is constructed.
  • num_schedules : 1 gives exit 0.
  • The same 5-schedule job with the service built under AddressSanitizer gives exit 0 and no reports.
• Not run: a ThreadSanitizer build of art. The race is inferred from the code, not observed. I also did not exercise the two consumer modules with real input, only compiled them.

Deliberately not in this PR

These came out of the same package review, but each one needs a decision from the trigger group, not a mechanical fix:

• A timeout makes the consumers put() a truncated collection, and nothing in the event records that it happened.
• A module-level timeout is sticky for the rest of the event (the FIXME), so a later module in the same event also sees check() == true.
• The service-level moduleTimeoutMs has no effect, because both consumers create the guard only when their own timeoutMs > 0 and always pass it. eventTimeoutMs is checked only by modules that have a guard.
• Moving the diagnostics from printf to message-facility, and removing the vestigial SConscript.

@michaelmackenzie, this changes the ModuleGuard constructor signature. It is worth a look if you have trigger-side code outside Offline that constructs one.

🤖 Generated with Claude Code

https://claude.ai/code/session_014DZynGvjEZXCiW6Ag6y5vG

[FNALbuild]

Hi @oksuzian,
You have proposed changes to files in these packages:

• TimeoutService
• CalPatRec

which require these tests: build.

@Mu2e/write, @Mu2e/fnalbuild-users have access to CI actions on main.

⌛ The following tests have been triggered for 1dc8bab: build (Build queue - API unavailable)

About FNALbuild. Code review on Mu2e/Offline.

[oksuzian]

@FNALbuild run build test

[FNALbuild]

❌ Those tests have already run or are running for 1dc8bab (build)

[michaelmackenzie]

This looks good to me, thanks! I think it's worth noting that we do not intend to run with multiple threads Online, but it's probably best to be prepared for it in case it ever happens.

[FNALbuild]

☀️ The build tests passed at 1dc8bab.

Test	Result	Details
test with	✅	Command did not list any other PRs to include
merge	✅	Merged 1dc8bab at 329c100
build (prof)	✅	Log file. Build time: 04 min 25 sec
ceSimReco	✅	Log file.
g4test_03MT	✅	Log file.
transportOnly	✅	Log file.
POT	✅	Log file.
g4study	✅	Log file.
cosmicSimReco	✅	Log file.
cosmicOffSpill	✅	Log file.
ceSteps	✅	Log file.
ceDigi	✅	Log file.
muDauSteps	✅	Log file.
ceMix	✅	Log file.
rootOverlaps	✅	Log file.
g4surfaceCheck	✅	Log file.
trigger	❓	Log file. Return Code 1.
check_cmake	✅	Log file.
FIXME, TODO	➡️	TODO (0) FIXME (1) in 4 files
clang-tidy	➡️	3 errors 212 warnings
whitespace check	✅	no whitespace errors found

N.B. These results were obtained from a build of this Pull Request at 1dc8bab after being merged into the base branch at 329c100.

For more information, please check the job page here.
Build artifacts are deleted after 5 days. If this is not desired, select Keep this build forever on the job page.

[rlcee]

What does "pass the MT test" mean - what test? What is the project and goal?

[michaelmackenzie]

What does "pass the MT test" mean - what test? What is the project and goal?

Originally the code failed the CI test that used g4MT since I set it to be a LEGACY service without multi-threading support. I had to update to SHARED to pass that CI test.

[rlcee]

Why does the g4MT test use the timeout service? How does this test manage the other services, which should all be legacy? It has been years since we discussed MT, so I may have forgot something, but my recollection is that nothing is validated to run MT except Mu2eG4, and that runs MT internally, using geant mechanisms, not in art.

[oksuzian]

@rlcee The service is loaded by g4test_03MT through Services.SimAndReco, which includes TimeoutWatchdog. That job runs 5 art schedules, and art refuses to start a multi-schedule job if any loaded service is declared LEGACY. That is why Michael had to change it to SHARED. The other Offline services are not legacy either: all seven DECLARE_ART_SERVICE uses in Offline are SHARED. SHARED promises the service is safe to call from several threads at once, and the old single-state code did not keep that promise. This PR keeps separate state per schedule so that it does. Mu2e production still runs with one schedule, and single-schedule behaviour is unchanged.