amd-vkale · Sep 21, 2026 · Sep 21, 2026 · Sep 21, 2026 · Sep 21, 2026 · Sep 21, 2026
@@ -107,10 +107,9 @@ keeps a plain name. Each remaining service mirrors this trio in its own `queue_h
 
 `client_ids.hpp` replaces the registry's auto-incrementing `ClientID` with fixed producer tags, so
 the id attached to an instrumentation packet no longer depends on the order in which services
-happened to register. The tags numerically overlap the registry's `ClientID`, which also starts at 1
-and remains in use by services that have not migrated yet. That is inert only because no consumer
-dispatches on the tag — each service identifies its own packets by pointer lookup or `dynamic_cast`
-— and `client_ids.hpp` records what has to change before anything routes on these values.
+register. The tags are negative, keeping them disjoint from the positive ClientIDs still used by
+services that have not migrated yet. Each migrated service identifies its own packets by its tag,
+so any tag added here must remain negative and unique.
 
 ## 3. Enqueue and completion use different context sets
 

@@ -93,6 +93,10 @@ init_logging(std::string_view env_prefix, logging_config cfg)
 
         cfg.logdir       = get_env(fmt::format("{}_LOG_DIR", env_prefix), cfg.logdir);
         cfg.vlog_modules = get_env(fmt::format("{}_vmodule", env_prefix), cfg.vlog_modules);
+        // Opt-in because it replaces the application's own fault handling: a tool that installs
+        // its own SIGSEGV handler, or a test that expects a bare crash, should not inherit ours.
+        cfg.install_failure_handler = get_env(fmt::format("{}_FAILURE_SIGNAL_HANDLER", env_prefix),
+                                              cfg.install_failure_handler);
 
         auto loglvl = to_lower(common::get_env(fmt::format("{}_LOG_LEVEL", env_prefix), ""));
         // default to warning

@@ -62,6 +62,10 @@ struct registered_context_storage
 {
     std::vector<std::shared_ptr<context>> owned = {};
     std::vector<const context*>           view  = {};
+    // Positional, unlike `view`: index i is registry slot i, holding nullptr where the slot is
+    // empty. Resolving a context id is an index operation, so it needs the slot numbering that
+    // allocate_context() assigns, which `view` loses by skipping empty slots.
+    std::vector<context*> slots = {};
 };
 
 namespace
@@ -151,6 +155,19 @@ struct registry_state
     stable_context_vec_t registry =
         stable_context_vec_t{reserve_size_t{stable_context_vec_t::chunk_size}};
     published_snapshot_t published = {};
+    // Contexts whose client has deregistered. They are unreachable through the registry and the
+    // snapshot, but they are not destroyed here: get_registered_contexts() and
+    // get_active_contexts() hand out raw pointers that outlive the snapshot they were read from,
+    // and those pointers are spread across every tracing service. Holding the last reference until
+    // static teardown gives them the lifetime they had when the registry stored the contexts
+    // by value, which is the lifetime all of those callers were written against.
+    std::vector<context_ptr_t> retired = {};
+    // Context ids whose stop is in progress. stop_context() releases get_contexts_mutex() across
+    // the GPU drain, so for that window the context is still in the active array with no lock
+    // held. This set is what keeps start_context() and a second stop_context() from acting on
+    // that half-stopped state. Guarded by get_contexts_mutex().
+    std::unordered_set<uint64_t> stopping = {};
+    std::condition_variable      cv       = {};
 };
 
 registry_state*
@@ -174,6 +191,13 @@ get_published_snapshot()
     return (_state) ? &_state->published : nullptr;
 }
 
+std::vector<context_ptr_t>*
+get_retired_contexts()
+{
+    auto* _state = get_registry_state();
+    return (_state) ? &_state->retired : nullptr;
+}
+
 context_snapshot_ptr_t
 current_registered_contexts()
 {
@@ -196,8 +220,10 @@ publish_registered_contexts()
     {
         _data->owned.reserve(_impl->size());
         _data->view.reserve(_impl->size());
+        _data->slots.reserve(_impl->size());
         for(const auto& itr : *_impl)
         {
+            _data->slots.emplace_back(itr.get());
             if(!itr) continue;
             _data->owned.emplace_back(itr);
             _data->view.emplace_back(itr.get());
@@ -209,6 +235,10 @@ publish_registered_contexts()
 
 // The registry is index-addressed: allocate_context() derives context_idx from the slot position,
 // so a context id maps straight back to its slot.
+//
+// Requires get_contexts_mutex(). The registry is a stable_vector, which keeps element addresses
+// stable but reallocates the chunk index that at() walks, so an unlocked reader can index a freed
+// buffer. Callers that cannot take the mutex must go through lookup_registered_context() instead.
 context_ptr_t
 get_registered_context_slot(uint64_t handle)
 {
@@ -221,22 +251,40 @@ get_registered_context_slot(uint64_t handle)
     return _impl->at(_idx);
 }
 
-// Context ids whose stop is in progress. stop_context() releases get_contexts_mutex() across the
-// GPU drain, so for that window the context is still in the active array with no lock held. This
-// set is what keeps start_context() and a second stop_context() from acting on that half-stopped
-// state. Guarded by get_contexts_mutex().
-std::unordered_set<uint64_t>&
+// Resolves a context id off the published snapshot, for the callers that hold no lock. The
+// snapshot is immutable once published and owns the contexts it names, so indexing it neither
+// races allocate_context() nor touches a reference count.
+context*
+lookup_registered_context(uint64_t handle)
+{
+    if(handle < get_contexts_offset()) return nullptr;
+
+    auto _snapshot = current_registered_contexts();
+    if(!_snapshot) return nullptr;
+
+    auto _idx = handle - get_contexts_offset();
+    if(_idx >= _snapshot->slots.size()) return nullptr;
+
+    return _snapshot->slots[_idx];
+}
+
+// Both live in registry_state rather than being function-local statics of their own. A tool's
+// finalizer runs stop_context() during teardown, by which point a plain function-local static may
+// already have been destroyed -- and reading a destroyed unordered_set means hashing into freed
+// bucket memory. registry_state is torn down by common::destroy_static_objects() instead, at a
+// point the SDK controls, and these accessors return null once that has happened.
+std::unordered_set<uint64_t>*
 get_stopping_contexts()
 {
-    static auto _v = std::unordered_set<uint64_t>{};
-    return _v;
+    auto* _state = get_registry_state();
+    return (_state) ? &_state->stopping : nullptr;
 }
 
-std::condition_variable&
+std::condition_variable*
 get_contexts_cv()
 {
-    static auto _v = std::condition_variable{};
-    return _v;
+    auto* _state = get_registry_state();
+    return (_state) ? &_state->cv : nullptr;
 }
 
 }  // namespace
@@ -247,7 +295,13 @@ get_contexts_cv()
 void
 wait_for_stopping_contexts(std::unique_lock<std::mutex>& _lk)
 {
-    get_contexts_cv().wait(_lk, []() { return get_stopping_contexts().empty(); });
+    auto* _cv = get_contexts_cv();
+    if(!_cv) return;
+
+    _cv->wait(_lk, []() {
+        const auto* _stopping = get_stopping_contexts();
+        return (!_stopping) ? true : _stopping->empty();
+    });
 }
 
 std::mutex&
@@ -416,7 +470,7 @@ allocate_context()
 context*
 get_mutable_registered_context(rocprofiler_context_id_t id)
 {
-    return get_registered_context_slot(id.handle).get();
+    return lookup_registered_context(id.handle);
 }
 
 const context*
@@ -451,13 +505,18 @@ start_context(rocprofiler_context_id_t context_id)
     if(validate_context(cfg) != ROCPROFILER_STATUS_SUCCESS)
         return ROCPROFILER_STATUS_ERROR_CONTEXT_INVALID;
 
-    uint64_t rocp_tot_contexts = get_registered_contexts_impl()->size();
+    uint64_t rocp_tot_contexts = 0;
     auto     idx               = rocp_tot_contexts;
     auto&    active_contexts   = get_active_contexts_impl();
     {
         // hold a lock here to prevent multiple threads from finding the same nullptr slot
         auto _lk = std::unique_lock<std::mutex>{get_contexts_mutex()};
 
+        // Sized under the lock: size() walks the registry's chunk index, which allocate_context()
+        // reallocates as it grows.
+        rocp_tot_contexts = get_registered_contexts_impl()->size();
+        idx               = rocp_tot_contexts;
+
         // A context that is mid-stop is still in the active array while its GPU drain runs, so
         // scanning against that state would either report a conflict against a context that is on
         // its way out, or hand back a slot that the stop is about to clear.
@@ -574,15 +633,15 @@ stop_context(rocprofiler_context_id_t idx)
         if(!slot) return ROCPROFILER_STATUS_ERROR_CONTEXT_NOT_FOUND;
 
         _owner = get_registered_context_slot(idx.handle);
-        get_stopping_contexts().emplace(idx.handle);
+        if(auto* _stopping = get_stopping_contexts()) _stopping->emplace(idx.handle);
     }
 
     auto _unclaim = common::scope_destructor{[&idx]() {
         {
             auto _lk = std::unique_lock<std::mutex>{get_contexts_mutex()};
-            get_stopping_contexts().erase(idx.handle);
+            if(auto* _stopping = get_stopping_contexts()) _stopping->erase(idx.handle);
         }
-        get_contexts_cv().notify_all();
+        if(auto* _cv = get_contexts_cv()) _cv->notify_all();
     }};
 
     // Phase two, unlocked: the service teardowns below call hsa::queue_controller_sync(), an
@@ -715,10 +774,6 @@ deregister_client_contexts(rocprofiler_client_id_t client_id)
 {
     if(!get_registered_contexts_impl()) return;
 
-    // Outlives the lock below so the contexts are destroyed without it held: a context destructor
-    // calls back into HSA, which is not something to do underneath the registry mutex.
-    auto _retired = std::vector<context_ptr_t>{};
-
     {
         // Mutates the registry, so it needs the same lock allocate_context() takes.
         auto _lk = std::unique_lock<std::mutex>{get_contexts_mutex()};
@@ -737,10 +792,14 @@ deregister_client_contexts(rocprofiler_client_id_t client_id)
                 {
                     if(bitr && bitr->context_id == itr->context_idx) bitr.reset();
                 }
-                // Moved rather than reset: dropping the registry's reference here would destroy
-                // the context under the lock, and a reader holding an older snapshot still needs
-                // it to stay alive until that snapshot is released.
-                _retired.emplace_back(std::move(itr));
+                // Moved to the retired list rather than reset: dropping the registry's reference
+                // here would destroy the context while dispatch and completion paths may still
+                // hold raw pointers to it, which is what made the anytime-tool-config tests
+                // segfault when one tool finalized while another tool's kernels were in flight.
+                if(auto* _retired = get_retired_contexts())
+                    _retired->emplace_back(std::move(itr));
+                else
+                    itr.reset();
             }
         }
 

@@ -262,10 +262,11 @@ using context_filter_t = bool (*)(const context*);
 inline bool
 default_context_filter(const context* val);
 
-// These read the published snapshot and then drop it, so the pointers they return outlive the
-// guarantee that the contexts are alive. That is fine for callers that cannot overlap with
-// deregister_client_contexts(); anything on a dispatch or completion path should hold a
-// registered_contexts_snapshot for as long as it uses the pointers.
+// These read the published snapshot and then drop it, so the pointers they return outlive it. The
+// contexts themselves stay alive regardless: deregister_client_contexts() retires a context out of
+// the registry without destroying it, so a pointer obtained here stays dereferenceable until
+// static teardown. Which contexts are reachable is still a snapshot-in-time answer, so a caller
+// that needs a stable set across a long operation should hold a registered_contexts_snapshot.
 context_array_t&
 get_registered_contexts(context_array_t& data, context_filter_t filter = default_context_filter);
 

@@ -176,14 +176,16 @@ stop_context(const context::context* ctx)
 {
     if(!ctx || !ctx->dispatch_counter_collection) return;
 
-    auto* controller = hsa::get_queue_controller();
+    auto* controller  = hsa::get_queue_controller();
+    bool  was_enabled = false;
 
     ctx->dispatch_counter_collection->enabled.wlock([&](auto& enabled) {
         if(!enabled) return;
-        enabled = false;
+        was_enabled = true;
+        enabled     = false;
     });
 
-    if(controller)
+    if(controller && was_enabled)
     {
         // Drain in-flight dispatches before anything else is torn down. The review of #8891
         // accepted provenance-based completion routing on the condition that the callback thread
@@ -215,22 +217,15 @@ set_dispatch_agents(rocprofiler_context_id_t      context_id,
 {
     if(num_agents > 0 && agents == nullptr) return ROCPROFILER_STATUS_ERROR_INVALID_ARGUMENT;
 
+    auto _lk = std::unique_lock<std::mutex>{context::get_contexts_mutex()};
+    context::wait_for_stopping_contexts(_lk);
+
     auto* ctx_p = context::get_mutable_registered_context(context_id);
     if(!ctx_p) return ROCPROFILER_STATUS_ERROR_CONTEXT_INVALID;
     if(!ctx_p->dispatch_counter_collection) return ROCPROFILER_STATUS_ERROR_CONTEXT_NOT_FOUND;
 
     // The agent set is read without a lock on the dispatch path and is what scopes
-    // serialization at start, so it may only change while the context is stopped. Hold the
-    // contexts mutex so this check and the assignment are atomic with start_context: a
-    // concurrent start could otherwise activate the context after the scan (using the old
-    // set for serialization) and then race with the unordered-set assignment below while
-    // dispatch hooks read it.
-    auto _lk = std::unique_lock<std::mutex>{context::get_contexts_mutex()};
-
-    // A context that is mid-stop is still in the active array while its GPU drain runs, and that
-    // drain happens with the mutex released, so the lock alone does not exclude it. Without this
-    // the scan below would read a context that is on its way out as a running one.
-    context::wait_for_stopping_contexts(_lk);
+    // serialization at start, so it may only change while the context is stopped.
 
     for(const auto* itr : context::get_active_contexts())
     {

@@ -323,14 +323,16 @@ stop_context(const context::context* ctx)
 {
     if(!ctx || !ctx->dispatch_spm) return;
 
-    auto* controller = hsa::get_queue_controller();
+    auto* controller  = hsa::get_queue_controller();
+    bool  was_enabled = false;
 
     ctx->dispatch_spm->enabled.wlock([&](auto& enabled) {
         if(!enabled) return;
-        enabled = false;
+        was_enabled = true;
+        enabled     = false;
     });
 
-    if(controller)
+    if(controller && was_enabled)
     {
         hsa::queue_controller_sync();
         controller->disable_serialization();

@@ -462,7 +462,7 @@ DispatchThreadTracer::resource_init()
 void
 DispatchThreadTracer::resource_deinit()
 {
-    enabled.store(false, std::memory_order_release);
+    auto was_enabled = enabled.exchange(false, std::memory_order_acq_rel);
 
     if(auto* controller = hsa::get_queue_controller())
     {
@@ -471,7 +471,7 @@ DispatchThreadTracer::resource_deinit()
             controller->remove_callback(*client_id);
             client_id = std::nullopt;
         });
-        controller->disable_serialization();
+        if(was_enabled) controller->disable_serialization();
     }
 
     ROCP_TRACE << "Clearing agents";

@@ -265,3 +265,32 @@ TEST(thread_trace, local_context_override_forced_on_still_invokes_dispatch_cb)
 
     tracer.resource_deinit();
 }
+
+TEST(thread_trace, resource_deinit_does_not_release_foreign_serialization_owner)
+{
+    ASSERT_EQ(hsa_init(), HSA_STATUS_SUCCESS);
+    test_init();
+
+    auto* controller = hsa::get_queue_controller();
+    ASSERT_NE(controller, nullptr);
+    ASSERT_FALSE(controller->get_supported_agents().empty());
+
+    const auto& agent = controller->get_supported_agents().begin()->second;
+    ASSERT_NE(agent.get_rocp_agent(), nullptr);
+
+    FakeQueue queue{agent, {.handle = 9}};
+    controller->serializer(&queue);
+
+    thread_trace::DispatchThreadTracer tracer{};
+
+    controller->enable_serialization();
+    tracer.start_context();
+    tracer.stop_context();
+    tracer.resource_deinit();
+
+    EXPECT_TRUE(controller->is_serialization_enabled(agent.get_rocp_agent()->id))
+        << "resource_deinit must not release another owner's serialization hold";
+
+    controller->disable_serialization();
+    EXPECT_FALSE(controller->is_serialization_enabled(agent.get_rocp_agent()->id));
+}
@@ -43,7 +43,10 @@ set(early-callback-late-buffered-env
     "ROCPROFILER_TOOL_CONTEXTS=KERNEL_DISPATCH_CALLBACK,CODE_OBJECT"
     "ROCPROFILER_TOOL_OUTPUT_FILE=${CMAKE_CURRENT_BINARY_DIR}/early-callback-late-buffered-tool-a.json"
     "ROCPROFILER_TOOL_LATE_OUTPUT_FILE=${CMAKE_CURRENT_BINARY_DIR}/early-callback-late-buffered-tool-b.json"
-    "ROCPROFILER_TOOL_DISABLE_PERFETTO=1")
+    "ROCPROFILER_TOOL_DISABLE_PERFETTO=1"
+    # TEMPORARY DIAGNOSTIC -- revert once the teardown segfault is root-caused. See
+    # early-hip-late-memcpy for the rationale.
+    "ROCPROFILER_FAILURE_SIGNAL_HANDLER=1")
 
 rocprofiler_configure_pytest_files(CONFIG pytest.ini COPY conftest.py)
 

@@ -47,7 +47,10 @@ set(early-hip-late-dispatch-env
     "ROCPROFILER_TOOL_CONTEXTS=HIP_API_BUFFERED"
     "ROCPROFILER_TOOL_OUTPUT_FILE=${CMAKE_CURRENT_BINARY_DIR}/early-hip-late-dispatch-tool-a.json"
     "ROCPROFILER_TOOL_LATE_OUTPUT_FILE=${CMAKE_CURRENT_BINARY_DIR}/early-hip-late-dispatch-tool-b.json"
-    "ROCPROFILER_TOOL_DISABLE_PERFETTO=1")
+    "ROCPROFILER_TOOL_DISABLE_PERFETTO=1"
+    # TEMPORARY DIAGNOSTIC -- revert once the teardown segfault is root-caused. See
+    # early-hip-late-memcpy for the rationale.
+    "ROCPROFILER_FAILURE_SIGNAL_HANDLER=1")
 
 rocprofiler_configure_pytest_files(CONFIG pytest.ini COPY conftest.py)