Repository navigation
fix(runtimes): harden unsafe and tool lifecycle boundaries - #14373
Merged
Merged
Conversation
❌ PR Check — Action Required
📖 Need help? See the Policy FAQ for details on every check and how to fix failures. |
|
🚫 Please fix the failed policies before requesting reviews. The following policy checks failed:
The |
Contributor
There was a problem hiding this comment.
🟡 Changes recommended
Unresolved shutdown deadlock, rollback, propagation-error handling, and cross-generation resource ownership issues block approval.
4 open issues
What changed in this PR
This PR hardens lifecycle and ownership boundaries across libhsa, rocddi, the profiler SDK, and the registrar, aiming to keep tool callbacks and native operations within valid storage lifetimes.
Changes:
- Adds serialized table-update callbacks, generation-aware registration, and documented table-borrowing contracts.
- Coordinates shutdown admission, callback quiescence, SDK monitor retirement, and late configuration.
- Replaces exposed allocator guards with scoped operations and adds registrar lifecycle regression tests.
| File | Description |
|---|---|
| runtimes/ddi/rocddi/src/memory/storage/tests.rs | Tests scoped allocator restoration and ownership. |
| runtimes/ddi/rocddi/src/memory/storage/owners.rs | Reuses validated allocation layouts during release. |
| runtimes/ddi/rocddi/src/memory/storage/allocator.rs | Adds closure-based allocator scopes. |
| runtimes/ddi/rocddi/src/memory/storage.rs | Removes the public scope-guard export. |
| runtimes/ddi/rocddi/src/driver/linux_kfd/tests/shutdown.rs | Migrates shutdown tests to scoped allocation. |
| runtimes/ddi/rocddi/src/driver/linux_kfd/tests/memory.rs | Scopes allocation-failure injection. |
| runtimes/ddi/rocddi/src/driver/linux_kfd/sysfs.rs | Updates allocator use in sysfs tests. |
| runtimes/ddi/rocddi/src/driver/linux_kfd/event.rs | Updates allocator use in event tests. |
| runtimes/ddi/libhsa/src/signal/wait.rs | Gates public waits during shutdown. |
| runtimes/ddi/libhsa/src/signal/asynchronous.rs | Detaches workers and rejects late callbacks. |
| runtimes/ddi/libhsa/src/signal.rs | Adds admission for public signal access. |
| runtimes/ddi/libhsa/src/runtime/logging.rs | Exposes logging shutdown internally. |
| runtimes/ddi/libhsa/src/runtime/lifecycle.rs | Adds preparation and raw-handle admission states. |
| runtimes/ddi/libhsa/src/runtime/events.rs | Rejects event registration after stopping. |
| runtimes/ddi/libhsa/src/runtime.rs | Collects and quiesces tool callback workers. |
| runtimes/ddi/libhsa/src/queue/index.rs | Counts public queue-index operations. |
| runtimes/ddi/libhsa/src/queue/create.rs | Rejects queue creation during stopping. |
| runtimes/ddi/libhsa/src/queue.rs | Detaches queue callback workers. |
| runtimes/ddi/libhsa/src/queue_intercept.rs | Tracks and drains interception callbacks. |
| runtimes/ddi/libhsa/src/pc_sampling/session.rs | Quiesces sampling before tool unload. |
| runtimes/ddi/libhsa/src/pc_sampling.rs | Snapshots sessions and gates creation. |
| runtimes/ddi/libhsa/src/memory.rs | Detaches memory workers for external joins. |
| runtimes/ddi/libhsa/src/api/signal/atomic.rs | Routes signal operations through admission. |
| runtimes/ddi/libhsa/src/api/queue/core.rs | Tracks intercepted queue destruction. |
| runtimes/ddi/libhsa/src/api/loader/executable/state.rs | Rejects destruction from SDK record callbacks. |
| runtimes/ddi/libhsa/src/api/lifecycle.rs | Coordinates prepared shutdown and SDK contracts. |
| runtimes/ddi/libhsa/src/api_trace.rs | Publishes table snapshots and manages tool lifetimes. |
| runtimes/ddi/libhsa/README.md | Documents table and shutdown contracts. |
| runtimes/ddi/libhsa/build.rs | Generates atomic dispatch and profiler bridges. |
| runtimes/ddi/libamdf/src/queue.rs | Uses scoped mapping allocation. |
| runtimes/ddi/libamdf/src/memory.rs | Checks pending ownership and scopes allocation. |
| runtimes/ddi/libamdf/src/instance.rs | Scopes frontend owner allocation. |
| runtimes/api-headers/include/hsa/hsa_api_trace.h | Documents borrowed legacy-tool tables. |
| projects/rocprofiler-sdk/source/lib/rocprofiler-sdk/thread_trace/core.hpp | Declares resource-presence queries. |
| projects/rocprofiler-sdk/source/lib/rocprofiler-sdk/thread_trace/core.cpp | Reuses resources and narrows initialization rollback. |
| projects/rocprofiler-sdk/source/lib/rocprofiler-sdk/thread_trace/code_object.hpp | Declares table-hook reapplication. |
| projects/rocprofiler-sdk/source/lib/rocprofiler-sdk/thread_trace/code_object.cpp | Reapplies existing thread-trace hooks. |
| projects/rocprofiler-sdk/source/lib/rocprofiler-sdk/registration.cpp | Coordinates setters, monitor shutdown, and rollback. |
| projects/rocprofiler-sdk/source/lib/rocprofiler-sdk/pc_sampling/types.hpp | Tracks session configuration readiness. |
| projects/rocprofiler-sdk/source/lib/rocprofiler-sdk/pc_sampling/service.cpp | Serializes incremental sampling configuration. |
| projects/rocprofiler-sdk/source/lib/rocprofiler-sdk/pc_sampling/hsa_adapter.cpp | Tracks initialization and throws configuration failures. |
| projects/rocprofiler-sdk/source/lib/rocprofiler-sdk/pc_sampling/code_object.hpp | Declares sampling-hook reapplication. |
| projects/rocprofiler-sdk/source/lib/rocprofiler-sdk/pc_sampling/code_object.cpp | Reapplies existing sampling hooks. |
| projects/rocprofiler-sdk/source/lib/rocprofiler-sdk/kernel_replay/memory_tracker.cpp | Avoids rewriting unchanged saved functions. |
| projects/rocprofiler-sdk/source/lib/rocprofiler-sdk/internal_threading.cpp | Repairs task counts on submission failure. |
| projects/rocprofiler-sdk/source/lib/rocprofiler-sdk/hsa/queue.cpp | Installs the queue write interceptor unconditionally. |
| projects/rocprofiler-sdk/source/lib/rocprofiler-sdk/hsa/queue_interposition.hpp | Declares queue admission and callback-state checks. |
| projects/rocprofiler-sdk/source/lib/rocprofiler-sdk/hsa/queue_info_session.hpp | Adds packet retirement flags. |
| projects/rocprofiler-sdk/source/lib/rocprofiler-sdk/hsa/queue_controller.hpp | Adds explicit queue mode and table updates. |
| projects/rocprofiler-sdk/source/lib/rocprofiler-sdk/hsa/queue_controller.cpp | Coordinates queue publication and pointer invalidation. |
| projects/rocprofiler-sdk/source/lib/rocprofiler-sdk/hsa/hsa.hpp | Declares native generation queries. |
| projects/rocprofiler-sdk/source/lib/rocprofiler-sdk/hsa/hsa.cpp | Adds lifecycle serialization and native-contract detection. |
| projects/rocprofiler-sdk/source/lib/rocprofiler-sdk/context/context.hpp | Tracks sampling-service configuration. |
| projects/rocprofiler-sdk/source/lib/rocprofiler-sdk/context/context.cpp | Gates lifecycle operations and retires client contexts. |
| projects/rocprofiler-sdk/source/lib/rocprofiler-sdk/code_object/code_object.hpp | Declares code-object hook reapplication. |
| projects/rocprofiler-sdk/source/lib/rocprofiler-sdk/code_object/code_object.cpp | Reapplies hooks and guards callback destruction. |
| projects/rocprofiler-sdk/source/lib/rocprofiler-sdk/buffer.hpp | Declares buffer-callback activity detection. |
| projects/rocprofiler-sdk/source/lib/rocprofiler-sdk/buffer.cpp | Tracks callback nesting per thread. |
| projects/rocprofiler-sdk/source/include/rocprofiler-sdk/registration.h | Documents configuration and shutdown coordination APIs. |
| projects/rocprofiler-sdk/source/include/rocprofiler-sdk/intercept_table.h | Documents late propagation and borrowed tables. |
| projects/rocprofiler-register/tests/CMakeLists.txt | Registers callback lifecycle and race tests. |
| projects/rocprofiler-register/tests/callback-suspend-race.cpp | Tests suspension against active setters. |
| projects/rocprofiler-register/tests/callback-lifecycle.cpp | Tests callback registration, failures, and reactivation. |
| projects/rocprofiler-register/tests/callback-attach.cpp | Provides a test attachment setter. |
| projects/rocprofiler-register/source/lib/rocprofiler-register/rocprofiler_register.cpp | Implements callback registration and serialized lifecycle controls. |
| projects/rocprofiler-register/source/lib/rocprofiler-register/details/checked_lock.hpp | Adds nonblocking acquisition tracking. |
| projects/rocprofiler-register/source/include/rocprofiler-register/rocprofiler-register.h | Exposes callback and suspension contracts. |
Review effort: Balanced
atgutier
force-pushed
the
users/agutierr/unsafe-boundary-hardening
branch
from
October 27, 2026 20:18
23bdb4c to
fb8f964
Compare
Contributor
There was a problem hiding this comment.
🟡 Changes recommended
Two critical lifecycle races can allow tool callbacks after OnUnload and let deferred shutdown return before publishing the stop request, risking process aborts or further handler execution.
2 open issues
4 resolved since last review
Review effort: Balanced
Comment on lines
+210
to
+211
| { | ||
| return SUCCESS; |
| /// the final event run after that lock is released, before tool `OnUnload`. | ||
| pub(crate) fn begin_tool_shutdown(&mut self) -> ToolShutdownWork { | ||
| self.request_stop(); | ||
| let (queue_workers, intercept_queues) = self.queue_state.take_callback_workers(); |
Raw HSA calls and callback workers could outlive generation storage, while legacy tools could retain API table pointers past unload. The earlier tool fix depended on registrar and SDK lifecycle hooks absent from the existing ABI. Deferred shutdown could also return before its stop request, and a proxy worker could be published after shutdown's callback snapshot. Count native and raw handle work through shutdown, quiesce callback producers before OnUnload, and retain per-registration and legacy tool tables. Publish legacy edits through atomic dispatch slots and bridge to retained registrar fields so existing SDK late attachment remains available. Publish the deferred stop request before returning to callbacks and fence proxy queue creation under the lifecycle lock. Tighten rocddi allocator, sysfs, and event ownership at the remaining unsafe boundaries. The registrar still has no lock or notification for a concurrent late table rewrite, nor a fence against replay during final HSA shutdown. Preserve its existing behavior and document those concurrency limits. Repeated HSA generations still inherit the registrar and SDK resource limits. Verified with Cargo workspace tests, strict Clippy, rustfmt, a CMake runtime build, six CTests, symbol inspection, and git diff --check. GPU-dependent paths could not run on this host.
atgutier
force-pushed
the
users/agutierr/unsafe-boundary-hardening
branch
from
October 27, 2026 20:37
fb8f964 to
9c3db9e
Compare
Contributor
TheRock Submodule Bump ActivityNewest first
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
HSA table updates, raw handle calls, and callback workers could outlive their storage or tool state. Late SDK configuration could race with queue creation, and failures could leave clients or completion work active.
Publish HSA table changes through serialized registrar callbacks and retain borrowed tables by generation. Count raw and native work through shutdown, quiesce callbacks before OnUnload, and coordinate SDK queue mode, monitor retirement, and forced client rollback. Tighten rocddi allocator, sysfs, and event ownership at the remaining unsafe boundaries.
Verified with serial and parallel Cargo workspace tests, strict Clippy, rustfmt, six runtime CTests, 16 registrar callback and soname CTests, and git diff --check. The SDK CMake build is unavailable because this checkout lacks its external PTL and fmt submodules.