Skip to content

Commit

Permalink
fix: additional profiling data races (#3035)
Browse files Browse the repository at this point in the history
In #3018, there was a change to add locking where `processBacktrace` mutated data structures that could be accessed concurrently. It attempted to separate the code that did not access the shared data structures, and moved all mutating code to a section at the end of the function. However, it missed a few reads from those data structures that also needed to be synchronized (e.g. `frames.count` and `stacks.count`). Generally, separating the critical section from the rest of the logic in that function is not worth the pain because the taking the lock for the entire duration of the function is not expensive.

This refactor adds a new type, `SentryProfilingState`, that centralizes all logic for reading/writing the shared data structures. This has the additional benefit that you no longer need a `processBacktrace` function that takes many parameters where the lifetime of those parameters is unclear (this will be helpful for fixing the memory leaks in a subsequent PR).

---------

Co-authored-by: Sentry Github Bot <[email protected]>
  • Loading branch information
indragiek and getsentry-bot authored May 16, 2023
1 parent 2719ce6 commit cd39d58
Show file tree
Hide file tree
Showing 7 changed files with 273 additions and 287 deletions.
2 changes: 1 addition & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@

### Fixed

- Fix crashes in profiling serialization race condition (#3018)
- Fix crashes in profiling serialization race condition (#3018, #3035)
- Fix a crash for user interaction transactions (#3036)

## 8.7.1
Expand Down
4 changes: 4 additions & 0 deletions Sentry.xcodeproj/project.pbxproj
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
objects = {

/* Begin PBXBuildFile section */
0354A22B2A134D9C003C3A04 /* SentryProfiler+Private.h in Headers */ = {isa = PBXBuildFile; fileRef = 0354A22A2A134D9C003C3A04 /* SentryProfiler+Private.h */; };
0356A570288B4612008BF593 /* SentryProfilesSampler.h in Headers */ = {isa = PBXBuildFile; fileRef = 0356A56E288B4612008BF593 /* SentryProfilesSampler.h */; };
0356A571288B4612008BF593 /* SentryProfilesSampler.m in Sources */ = {isa = PBXBuildFile; fileRef = 0356A56F288B4612008BF593 /* SentryProfilesSampler.m */; };
03BCC38A27E1BF49003232C7 /* SentryTime.h in Headers */ = {isa = PBXBuildFile; fileRef = 03BCC38927E1BF49003232C7 /* SentryTime.h */; };
Expand Down Expand Up @@ -861,6 +862,7 @@
/* End PBXContainerItemProxy section */

/* Begin PBXFileReference section */
0354A22A2A134D9C003C3A04 /* SentryProfiler+Private.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; name = "SentryProfiler+Private.h"; path = "Sources/Sentry/include/SentryProfiler+Private.h"; sourceTree = SOURCE_ROOT; };
0356A56E288B4612008BF593 /* SentryProfilesSampler.h */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.h; name = SentryProfilesSampler.h; path = Sources/Sentry/include/SentryProfilesSampler.h; sourceTree = SOURCE_ROOT; };
0356A56F288B4612008BF593 /* SentryProfilesSampler.m */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.objc; name = SentryProfilesSampler.m; path = Sources/Sentry/SentryProfilesSampler.m; sourceTree = SOURCE_ROOT; };
035E73C727D56757005EEB11 /* SentryBacktraceTests.mm */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.cpp.objcpp; path = SentryBacktraceTests.mm; sourceTree = "<group>"; };
Expand Down Expand Up @@ -3064,6 +3066,7 @@
03F84D1127DD414C008FE43F /* SentryProfiler.h */,
844EDD6B2949387000C86F34 /* SentryMetricProfiler.h */,
8454CF8B293EAF9A006AC140 /* SentryMetricProfiler.mm */,
0354A22A2A134D9C003C3A04 /* SentryProfiler+Private.h */,
84A888FC28D9B11700C51DFD /* SentryProfiler+Test.h */,
844EDC7B2942843400C86F34 /* SentryProfiler+SwiftTest.h */,
03F84D2B27DD4191008FE43F /* SentryProfiler.mm */,
Expand Down Expand Up @@ -3515,6 +3518,7 @@
7BC8522F24581096005A70F0 /* SentryFileContents.h in Headers */,
7B30B67C26527886006B2752 /* SentryDisplayLinkWrapper.h in Headers */,
7BD86ECF264A7C77005439DB /* SentryAppStartMeasurement.h in Headers */,
0354A22B2A134D9C003C3A04 /* SentryProfiler+Private.h in Headers */,
7DC8310A2398283C0043DD9A /* SentryCrashIntegration.h in Headers */,
63FE718320DA4C1100CDBAE8 /* SentryCrashReportFixer.h in Headers */,
03F84D2027DD414C008FE43F /* SentryStackBounds.hpp in Headers */,
Expand Down
Loading

0 comments on commit cd39d58

Please sign in to comment.