[hskang-amelia]

Description

IdentifierHash uses std::hash<std::string>/std::hash<std::string_view>, which is standard-library-implementation-defined — deterministic within one binary's execution, but not guaranteed stable across different compilers or libstdc++/libc++ versions. That's a problem specifically because IdentifierHash's primary real-world use is being sent as a raw numeric hash over ControlClient IPC to Launch Manager — no string travels with it, so the two sides must compute the identical value independently. If the ControlClient side and the Launch Manager daemon happen to be built by different toolchains, they don't.

Effect: Graph::startTransition() looks the incoming hash up in run_targets_, gets -1 (getRunTargetIndex), and that -1 slips past a SCORE_LANGUAGE_FUTURECPP_ASSERT_DBG_MESSAGE (compiled out in a release build) straight into TransitionBuilder::createTransition(), which then hits its own always-on assert (transition_.isValidNode(target)) and aborts the entire Launch Manager daemon — for what should just be an ordinary ControlClientCode::kSetStateInvalidArguments response to one bad request.

How this was found: building a real cross-toolchain end-to-end reproduction — an independent ControlClient implementation, built by a different toolchain than Launch Manager itself, called ActivateRunTarget() for the exact run target name Launch Manager had itself just activated as initial_run_target on boot. Instrumenting IdentifierHash::operator<< and Graph::getRunTargetIndex showed two different numeric hashes for that identical string, one per binary.

Fix: switch to FNV-1a (a small, fixed, self-contained algorithm — no dependency on the standard library's hash implementation), and add a regression test that pins specific expected hash values for known input strings, so any future change to the algorithm has to consciously revisit the cross-binary guarantee this class exists to provide. identifier_hash.cpp's own comment already flagged exactly this risk ("if this should work between different compilers, we may need to go for our own hash function") — this resolves that open question with evidence rather than leaving it as a caveat.

Also drops the now-unused <functional> include.

Related ticket

closes #528

Test plan

• New identifier_hash_UT.cpp tests: hardcoded expected FNV-1a values (computed independently of this codebase) for known strings, and cross-constructor-overload consistency (std::string/std::string_view/const char* must agree)
• bazel test //score/launch_manager/src/daemon/src/common:identifier_hash_UT — passes
• bazel test //score/launch_manager/... (28/28 targets) — passes, no regressions from the algorithm change
• End-to-end: rebuilt the reproducing ControlClient implementation against this fix (via a local Bazel module override) and re-ran the exact cross-toolchain repro that originally surfaced this — ActivateRunTarget() now returns Success for both the initial-transition-to-an-already-active-target case and an actual run target switch, where it previously crashed the daemon every time

Note

Independent of this fix, createTransition() aborting the entire daemon on any unrecognized run target name (not just this hash-mismatch case) is a separate robustness gap — filed as #541, kept this PR scoped to the actual root cause.

[github-actions]

License Check Results

🚀 The license check job ran with the Bazel command:

bazel run --lockfile_mode=error //:license-check

Status: ⚠️ Needs Review

Click to expand output

[License Check Output]
Extracting Bazel installation...
Starting local Bazel server (8.7.0) and connecting to it...
INFO: Invocation ID: f7475dbc-c75c-491b-a7b0-974b3d2e88cb
Computing main repo mapping: 
Computing main repo mapping: 
Loading: 
Loading: 0 packages loaded
Loading: 0 packages loaded
Loading: 0 packages loaded
    currently loading: 
Loading: 0 packages loaded
    currently loading: 
Loading: 0 packages loaded
    currently loading: 
Loading: 0 packages loaded
    currently loading: 
Analyzing: target //:license-check (1 packages loaded, 0 targets configured)
Analyzing: target //:license-check (1 packages loaded, 0 targets configured)

Analyzing: target //:license-check (93 packages loaded, 17 targets configured)

Analyzing: target //:license-check (156 packages loaded, 3581 targets configured)

Analyzing: target //:license-check (163 packages loaded, 9368 targets configured)

Analyzing: target //:license-check (163 packages loaded, 9368 targets configured)

INFO: Analyzed target //:license-check (173 packages loaded, 11555 targets configured).
[1 / 1] no actions running
INFO: Found 1 target...
Target //:license.check.license_check up-to-date:
  bazel-bin/license.check.license_check
  bazel-bin/license.check.license_check.jar
INFO: Elapsed time: 17.061s, Critical Path: 0.30s
INFO: 16 processes: 4 disk cache hit, 12 internal.
INFO: Build completed successfully, 16 total actions
INFO: Running command line: bazel-bin/license.check.license_check ./formatted.txt <args omitted>
usage: org.eclipse.dash.licenses.cli.Main [-batch <int>] [-cd <url>]
       [-confidence <int>] [-ef <url>] [-excludeSources <sources>] [-help] [-lic
       <url>] [-project <shortname>] [-repo <url>] [-review] [-summary <file>]
       [-timeout <seconds>] [-token <token>]

[hskang-amelia]

Hi @MaciejKaszynski,
Thanks for the review — addressed all points in 5222639:

• 32-bit support: the FNV-1a offset basis / prime constants now switch based on
  SIZE_MAX == UINT32_MAX, so they match the actual width of std::size_t instead of
  hardcoding the 64-bit variant.
• std::hash<T>: added a std::hash<IdentifierHash> specialization in
  identifier_hash.hpp, delegating to IdentifierHash::data() — so IdentifierHash can be
  used directly as an unordered_map/unordered_set key without a custom hasher.
• Dropped the in-code explanation paragraph.
• Applied the comment/description suggestions as-is.

Also re-verified end to end against a live Launch Manager (both the initial activation of initial_run_target and a subsequent run target switch round-trip successfully) — no regressions from the review changes.

[hskang-amelia]

Applied via the suggested commit (84838a6) — thanks!