Skip to content

Commit

Permalink
chore(profiling): prevent strings from GC'ed whose string_views are p…
Browse files Browse the repository at this point in the history
…assed to native side [backport 2.11] (#10826)

Backport 23a54ce from #10812 to 2.11.

## Checklist
- [x] PR author has checked that all the criteria below are met
- The PR description includes an overview of the change
- The PR description articulates the motivation for the change
- The change includes tests OR the PR description describes a testing
strategy
- The PR description notes risks associated with the change, if any
- Newly-added code is easy to change
- The change follows the [library release note
guidelines](https://ddtrace.readthedocs.io/en/stable/releasenotes.html)
- The change includes or references documentation updates if necessary
- Backport labels are set (if
[applicable](https://ddtrace.readthedocs.io/en/latest/contributing.html#backporting))

## Reviewer Checklist
- [x] Reviewer has checked that all the criteria below are met 
- Title is accurate
- All changes are related to the pull request's stated goal
- Avoids breaking
[API](https://ddtrace.readthedocs.io/en/stable/versioning.html#interfaces)
changes
- Testing strategy adequately addresses listed risks
- Newly-added code is easy to change
- Release note makes sense to a user of the library
- If necessary, author has acknowledged and discussed the performance
implications of this PR as reported in the benchmarks PR comment
- Backport labels are set in a manner that is consistent with the
[release branch maintenance
policy](https://ddtrace.readthedocs.io/en/latest/contributing.html#backporting)

Co-authored-by: Taegyun Kim <[email protected]>
  • Loading branch information
github-actions[bot] and taegyunkim committed Sep 26, 2024
1 parent d62344d commit 41e13b9
Showing 1 changed file with 8 additions and 0 deletions.
8 changes: 8 additions & 0 deletions ddtrace/internal/datadog/profiling/ddup/_ddup.pyx
Original file line number Diff line number Diff line change
Expand Up @@ -168,9 +168,16 @@ def upload() -> None:
processor = ddtrace.tracer._endpoint_call_counter_span_processor
endpoint_counts, endpoint_to_span_ids = processor.reset()

# We want to make sure that the endpoint_bytes strings outlive the for loops
# below and prevent them to be GC'ed. We do this by storing them in a list.
# This is necessary because we pass string_views to the C++ code, which is
# a view into the original string. If the original string is GC'ed, the view
# will point to garbage.
endpoint_bytes_list = []
cdef map[int64_t, string_view] span_ids_to_endpoints = map[int64_t, string_view]()
for endpoint, span_ids in endpoint_to_span_ids.items():
endpoint_bytes = ensure_binary_or_empty(endpoint)
endpoint_bytes_list.append(endpoint_bytes)
for span_id in span_ids:
span_ids_to_endpoints.insert(
pair[int64_t, string_view](
Expand All @@ -183,6 +190,7 @@ def upload() -> None:
cdef map[string_view, int64_t] trace_endpoints_to_counts = map[string_view, int64_t]()
for endpoint, cnt in endpoint_counts.items():
endpoint_bytes = ensure_binary_or_empty(endpoint)
endpoint_bytes_list.append(endpoint_bytes)
trace_endpoints_to_counts.insert(pair[string_view, int64_t](
string_view(<const char*>endpoint_bytes, len(endpoint_bytes)),
clamp_to_int64_unsigned(cnt)
Expand Down

0 comments on commit 41e13b9

Please sign in to comment.