Repository navigation
Conversation
Refactor context locking and error handling in core.cpp Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Updated the explanation of client IDs to clarify the use of negative tags for migrated services and their uniqueness. Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Co-authored-by: amd-vkale <291155109+amd-vkale@users.noreply.github.com>
…nd spm stop_context The was_enabled guard added on top of the stacked fixes left the alignment of the surrounding declarations and assignments untouched, which clang-format 11 rejects, and reflowed a return that fits on one line. The formatting CI job runs clang-format over source/ and fails on any diff. Co-authored-by: Vivek Kale <vivek.kale@amd.com>
… regex
Widening SKIP_REGULAR_EXPRESSION to also match "SKIP: HIP device"
dropped the closing parenthesis of the enclosing
rocprofiler_add_integration_execute_test() call, so CMake never finds
the end of the function:
CMake Error at tests/counter-collection-per-agent/CMakeLists.txt:47:
Parse error. Function missing ending ")". End of file reached.
Configure fails, which is why every Core job on this branch died before
reaching a build or a test.
Co-authored-by: Vivek Kale <vivek.kale@amd.com>
Restoring the closing paren pushed the argument past the column limit, so cmake-format wants the value on its own line. Co-authored-by: Vivek Kale <vivek.kale@amd.com>
…ive registry get_mutable_registered_context() walked the registry without get_contexts_mutex(), which the file's own header comment already rules out: the registry is a stable_vector, and while it keeps element addresses stable, emplace_back() grows the std::vector of chunk pointers that at() indexes. A lookup racing allocate_context() therefore reads a reallocated buffer. Under the previous std::optional storage that misread was survivable -- the caller only asked has_value() and took an address, so garbage usually read as "empty" and returned nullptr. Copying a std::shared_ptr out of the same garbage is not: it increments a reference count through whatever the control-block pointer happens to be, which faults. That is why the anytime-tool-config tests segfault: a second tool configuring while the first is already tracing is the only point where allocate_context() runs concurrently with dispatch threads resolving context ids, and the registry chunk size of 8 means two tools' contexts are enough to trigger the growth. Resolve ids off the published snapshot instead. It is immutable once published and owns the contexts it names, so indexing it needs no lock and touches no reference count. The snapshot gains a positional slots array for this, since the existing view is compacted and no longer carries the slot numbering ids are derived from. Also take the registry size() in start_context() under the lock, for the same reason: size() walks the same chunk index. Co-authored-by: Vivek Kale <vivek.kale@amd.com>
…ing them deregister_client_contexts() dropped the registry's last reference to a context, destroying it. get_registered_contexts() and get_active_contexts() hand out raw context pointers that outlive the snapshot they were read from, and those pointers are used across every tracing service, so a tool finalizing while another tool's kernels were still in flight freed a context out from under the completion path. That is what segfaulted the anytime-tool-config tests, which are the only ones that register a second tool late enough for the two lifetimes to overlap. Before the registry stored contexts by shared_ptr it stored them by value and never destroyed them, so every one of those callers was written against a lifetime that ran to static teardown. Retire the context into a list owned by the same static as the registry to give it that lifetime back: it leaves the registry and the published snapshot as before, so it stops being reachable, but the object stays valid for pointers already handed out. Co-authored-by: Vivek Kale <vivek.kale@amd.com>
…me test TEMPORARY, to be reverted. The anytime-tool-config tests segfault in CI between the first tool's finalizer returning and the second tool's starting. The failure needs a GPU, so it cannot be reproduced locally, and ctest does not capture a backtrace. Enabling the SDK's own info log on one of the failing tests shows which internal teardown step runs last. Co-authored-by: Vivek Kale <vivek.kale@amd.com>
TEMPORARY, to be reverted, replacing the log-level diagnostic from the previous commit. Raising the log level made the instrumented test start passing, which is expected for a teardown race but leaves no crash to inspect, and the info log was verbose enough that CDash truncated it before finalization anyway. logging.cpp already knows how to install abseil's failure signal handler, but nothing turns it on. Gate it on an environment variable and enable it for the anytime tests, which prints a symbolized stack trace on SIGSEGV without perturbing the timing the crash depends on. Enabled across all the anytime tests rather than one, since which of them crash varies between runs. Co-authored-by: Vivek Kale <vivek.kale@amd.com>
…fetime stop_context() runs from a tool's finalizer, and it marks the context in get_stopping_contexts() and signals get_contexts_cv(). Both were plain function-local statics, so the C++ runtime destroyed them on its own schedule during teardown while the rest of the registry state is torn down deliberately by common::destroy_static_objects(). When a second tool finalized after those statics were gone, stop_context() hashed into the freed buckets of a destroyed unordered_set and segfaulted. That is the anytime-tool-config crash: the fault handler put the PC in stop_context() on two of the failures and in _M_find_before_node() on two more, and the stopping set is the only hash table stop_context() touches. It needs two tools because the first tool's teardown is what lets the statics go while a second finalizer still has contexts to stop, which is why no single-tool test ever reproduced it. Move both into registry_state, which already exists to keep the registry and its published snapshot on a teardown order the SDK controls, and have the accessors return null afterwards so a late stop degrades instead of crashing. Co-authored-by: Vivek Kale <vivek.kale@amd.com>
.cursor/rules/communication-style.mdc is local Cursor agent configuration and was committed by accident alongside the context-retirement fix. It is not part of the SDK and does not belong in this change. Co-authored-by: Vivek Kale <vivek.kale@amd.com>
amd-vkale
marked this pull request as ready for review
September 23, 2026 06:17
github-actions
Bot
added
documentation
Contributor
|
This pull request has been inactive for 25 days and will be marked as stale. If you would like to keep this PR open, please:
This PR will be automatically closed in 5 days if no further activity occurs. |
Contributor
|
This pull request has been automatically closed due to inactivity (30 days with no updates). If you'd like to continue working on this, feel free to reopen the PR or create a new one. |
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.
12 commits cherry-picked from #11970 onto a mirror of #8891 head e31df6b. Tree matches 78d6079 except .cursor/rules/communication-style.mdc (agent config, committed by accident). Two temporary debug commits must be reverted before merge.