[JonathanLichtnerAMD]

JIRA ID: ROCM-27477

Motivation

GetFdeepModel and GetModel in projects/miopen/src/conv/heuristics/ai_heuristics.cpp cache
loaded AI heuristic models in static std::map containers with no synchronization. When
convolutions run concurrently from multiple threads on the same device (e.g. per-thread MIOpen
handles), two callers can race on the same cache key: both find the key missing, both build a
model, and both insert. In GetFdeepModel the second insert (models[key] = std::move(model))
move-assigns over the first thread's unique_ptr<fdeep::model>, destroying the model object while
the first thread is still using the reference it was handed — a use-after-free that segfaults deep
inside fdeep's prediction code.

This was surfaced by the PyTorch test test_cuda.py::TestCuda::test_cudnn_multiple_threads_same_device
on ROCm 7.14.0-RC1 / gfx942 (MI300 series), with an observed crash rate of roughly 3-17% per
triggering run. The exposing commit (#7109, which enabled the 2D AI heuristic default path for
gfx942/gfx950) predates the last known-good nightly, but the underlying missing-lock defect itself
is older and not a regression introduced within the RC window. The race is warm-up-bounded: it can
only occur during first concurrent population of a given cache key, since entries are never
overwritten afterward.

A runtime workaround exists without rebuilding:
MIOPEN_DEBUG_GROUP_CONV_IMPLICIT_GEMM_HIP_FWD_XDLOPS_AI_HEUR=0 disables the AI heuristic path
entirely, but that is not a substitute for fixing the underlying race.

Technical Details

The fix mirrors the already-correct sibling GetCandidateSelectionModel in
ai_candidate_selection.cpp, which guards its structurally identical cache with a
static std::mutex + std::lock_guard.

• GetModel(arch, solver): added a static std::mutex models_mutex and moved the
  models.find(solver) lookup inside a std::lock_guard that also covers the insert-on-miss
  path, so find+insert is atomic with respect to other callers. The function now returns the
  cached std::shared_ptr<Model> by value while still holding the lock, which is safe since
  shared_ptr copies are cheap and the returned value keeps the model alive independently of the
  map.
• GetFdeepModel(path, key): added a static std::mutex models_mutex guarding find+insert.
  Critically, the insertion path was changed from models[key] = std::move(model); return ref;
  (which risks a move-assign clobbering an existing entry on a race, and previously took the
  returned reference before the map write) to it = models.emplace(key, std::move(model)).first;
  followed by return *(it->second);. Because entries are only ever inserted once and never
  reassigned, a reference into the map obtained after the lock is released remains valid for the
  lifetime of the process — the lock only needs to serialize the population itself, not every
  subsequent read.

No public API, call signatures, or heuristic selection logic changed; this is purely a
thread-safety fix around existing caching behavior.

Risk Assessment

Risk Level: 🟢 Low

The change is a single-file, 27-line diff that adds standard mutex-guarded caching to two
functions, following the exact pattern already used correctly elsewhere in the same file. It does
not alter model selection, loading logic, or any public interface.

Impacted Components

• projects/miopen/src/conv/heuristics/ai_heuristics.cpp: GetModel (used by the general AI
  heuristic model cache) and candidate_selection::GetFdeepModel (used by the AI candidate
  selection path). Any convolution solver selection path that goes through the AI heuristics
  (currently defaulted on for gfx942/gfx950 2D convolutions) is indirectly affected.

Potential Side Effects

• Serializing model cache population introduces a mutex acquisition on every call to GetModel/
  GetFdeepModel, not only on cache misses. Since these are in-process, uncontended locks after
  warm-up (steady-state calls hit the fast find path while holding a brief, uncontended lock),
  the performance impact is expected to be negligible relative to the cost of running the model
  itself, but reviewers should watch for any contention under very high thread-count concurrent
  convolution workloads.
• No behavioral change to which model is loaded or which heuristic decision is made — only the
  concurrency safety of caching is affected.

Mitigation Steps

• The fix follows an existing, proven pattern (GetCandidateSelectionModel) already in production
  in the same file, minimizing the chance of a novel defect.
• The runtime env var workaround (MIOPEN_DEBUG_GROUP_CONV_IMPLICIT_GEMM_HIP_FWD_XDLOPS_AI_HEUR=0)
  remains available as an immediate mitigation if an unexpected issue surfaces post-merge, without
  requiring a revert or rebuild.
• Revert plan: the diff is small and self-contained to one file, making a straightforward revert
  possible if needed.

Test Plan

This will be validated manually on gfx942 hardware using the PyTorch repro test
(test_cuda.py::TestCuda::test_cudnn_multiple_threads_same_device) that originally surfaced the
crash, running it repeatedly to build confidence that the segfault no longer reproduces.

In addition, unit tests have been added that reproduce the race condition. These have been run manually, and will also run as part of the standard MIOpen test category.

Test Result

• manual validation on gfx942 via the PyTorch repro
• CI passes

[therock-pr-bot]

❌ PR Check — Action Required

Check	Status	Details
🌿 Branch Name	✅ Pass	—
📝 PR Title/Description	✅ Pass	—
⛔ Forbidden Files	✅ Pass	—
🧪 Unit Test	❌ Fail	Error: Source/code files changed without an accompanying unit test. Expected: add at least one test file named like test_<name>.py / test_<name>.cpp (or <name>_test.*). Current: code file(s) changed: projects/miopen/src/conv/heuristics/ai_heuristics.cpp, projects/miopen/test/gtest/conv_ai_heuristics_threading.cpp; no test file found
🔎 pre-commit	✅ Pass	—
🚫 Draft PR	🔜 To Be Enabled	—
🚩 Feature Flag	🔜 To Be Enabled	—
📊 Code Coverage	🔜 To Be Enabled	—

⚠️ 1 policy check(s) failed. Please address the issues above before this PR can be Reviewed.

🚫 Please fix the failed policies

• ❌ Unit Test

The Not ready to Review label was added to this PR. Once all policies pass, the label is removed automatically.

📖 Need help? See the Policy FAQ for details on every check and how to fix failures.

[therock-pr-bot]

🚫 Please fix the failed policies before requesting reviews.

The following policy checks failed:

• ❌ Unit Test

The Not ready to Review label has been added to this PR.
Once all policies pass, the label will be removed automatically.

[JonathanLichtnerAMD]

gfx942 (MI300X) validation — fixes the test_cudnn_multiple_threads_same_device SIGSEGV

Retested the JIRA repro (ROCM-27477) on a gfx942 host with a matched before/after comparison. Result: fixed.

Setup

• Built MIOpen from this branch (fix commit b0b874b03ad) in rocm/miopen-dev (ROCm 7.13); verified the mutex is in the binary (GetFdeepModel(...)::models_mutex).
• Ran under rocm/pytorch:rocm7.2.4_ubuntu24.04_py3.12_pytorch_release_2.9.1 (torch 2.9.1), swapping in the built libMIOpen.so.1 + gfx942 AI models + CK grouped-conv plugin. Confirmed torch loads it and the AI-heuristic path is actually exercised (TunaNetNDModel: Loaded fdeep model from gfx942.tn.model).
• Built an unpatched baseline (reverting only ai_heuristics.cpp) to prove the harness reproduces the bug.
• Repro = the exact test body (2 threads, threading.Barrier, conv2d with weight=ones(1,1,2,2), 1000 iters × 3 trials), one fresh process per run; a crash surfaces as a signal exit code.

Results (identical test, same host, matched N)

Build	Runs	Crashes
Unpatched (develop − fix)	220	3 — 2× SIGSEGV(11), 1× SIGABRT(6)
Patched (this PR)	220	0

• Crash signals (SIGSEGV + SIGABRT/heap-corruption) match the RCA. Core dumps confirm the fault is inside libMIOpen.so.1 with a corrupted stack — the use-after-free signature (the Release build strips the internal fdeep/GetFdeepModel symbols, so exact frame names don't resolve, but the crash region and corruption pattern match).
• Observed crash rate here was ~1.4%, lower than the originally reported 3–17% — expected, since these runs were spread across 4 GPUs in parallel on a busy shared host, which shifts the timing of the warm-up race. The key point is the clean separation: the crash never occurred with the fix (0/220), and reproduced only without it.

[BrianHarrisonAMD]

Changes LGTM.

I think adding some tests for model retrieval is worth it.