Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: synchronize and copy global profiling data structures #3018

Merged
merged 21 commits into from
May 15, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
21 commits
Select commit Hold shift + click to select a range
104d534
ref: extract serialization logic into testable function, with test to…
armcknight May 9, 2023
3557f57
copy global data structures in a synchronized context
armcknight May 10, 2023
a3c542c
changelog
armcknight May 10, 2023
d8da8a5
Update Tests/SentryProfilerTests/SentryProfilerSwiftTests.swift
armcknight May 10, 2023
73c63c1
Update Tests/SentryProfilerTests/SentryProfilerSwiftTests.swift
armcknight May 10, 2023
63525ab
ci: Fix skipping codecov2 upload for scheduled (#3015)
philipphofmann May 9, 2023
3594ea8
ci: only archive derived data logs if the build step fails (#3011)
armcknight May 9, 2023
a040e99
release: 8.7.0
getsentry-bot May 9, 2023
ecf9400
build(deps): bump github/codeql-action from 2.3.2 to 2.3.3 (#3014)
dependabot[bot] May 9, 2023
971805f
fix changelog entry and fix test
armcknight May 10, 2023
a337836
pr feedback
armcknight May 11, 2023
1ef06b3
reorganize to colocate queue metadata work
armcknight May 11, 2023
96494b4
relocate to common conditionally compiled code sector
armcknight May 11, 2023
a51cf97
update tests to guard against modifying thread metadata copy after se…
armcknight May 11, 2023
b3d2d3b
copy the mutable subdictionaries for thread metadata
armcknight May 11, 2023
c92fc91
fixup! reorganize to colocate queue metadata work
armcknight May 11, 2023
85209f0
add headerdoc explaining the test case, and explanation for another a…
armcknight May 12, 2023
ec9945a
also modify stacks/frames in critical section, with extra test checks
armcknight May 15, 2023
363d58b
also update queue metadata in critical section
armcknight May 15, 2023
f96c567
Merge remote-tracking branch 'origin/main' into armcknight/fix/crashes
armcknight May 15, 2023
7254787
fix changelog
armcknight May 15, 2023
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,5 +1,11 @@
# Changelog

## Unreleased

### Fixed

- Fix crashes in profiling serialization race condition (#3018)

## 8.7.1

### Features
Expand Down
40 changes: 16 additions & 24 deletions Sources/Sentry/SentryProfileTimeseries.mm
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,6 @@
# import "SentryLog.h"
# import "SentryTransaction.h"

std::mutex _gSamplesArrayLock;
armcknight marked this conversation as resolved.
Show resolved Hide resolved

/**
* Print a debug log to help diagnose slicing errors.
* @param start @c YES if this is an attempt to find the start of the sliced data based on the
Expand Down Expand Up @@ -44,51 +42,45 @@
NSArray<SentrySample *> *_Nullable slicedProfileSamples(
NSArray<SentrySample *> *samples, SentryTransaction *transaction)
{
NSArray<SentrySample *> *samplesCopy;
{
std::lock_guard<std::mutex> l(_gSamplesArrayLock);
samplesCopy = [samples copy];
}

if (samplesCopy.count == 0) {
if (samples.count == 0) {
return nil;
}

const auto transactionStart = transaction.startSystemTime;
const auto firstIndex =
[samplesCopy indexOfObjectWithOptions:NSEnumerationConcurrent
passingTest:^BOOL(SentrySample *_Nonnull sample, NSUInteger idx,
BOOL *_Nonnull stop) {
*stop = sample.absoluteTimestamp >= transactionStart;
return *stop;
}];
[samples indexOfObjectWithOptions:NSEnumerationConcurrent
passingTest:^BOOL(SentrySample *_Nonnull sample, NSUInteger idx,
BOOL *_Nonnull stop) {
*stop = sample.absoluteTimestamp >= transactionStart;
return *stop;
}];

if (firstIndex == NSNotFound) {
logSlicingFailureWithArray(samplesCopy, transaction, /*start*/ YES);
logSlicingFailureWithArray(samples, transaction, /*start*/ YES);
return nil;
} else {
SENTRY_LOG_DEBUG(@"Found first slice sample at index %lu", firstIndex);
}

const auto transactionEnd = transaction.endSystemTime;
const auto lastIndex =
[samplesCopy indexOfObjectWithOptions:NSEnumerationConcurrent | NSEnumerationReverse
passingTest:^BOOL(SentrySample *_Nonnull sample, NSUInteger idx,
BOOL *_Nonnull stop) {
*stop = sample.absoluteTimestamp <= transactionEnd;
return *stop;
}];
[samples indexOfObjectWithOptions:NSEnumerationConcurrent | NSEnumerationReverse
passingTest:^BOOL(SentrySample *_Nonnull sample, NSUInteger idx,
BOOL *_Nonnull stop) {
*stop = sample.absoluteTimestamp <= transactionEnd;
return *stop;
}];

if (lastIndex == NSNotFound) {
logSlicingFailureWithArray(samplesCopy, transaction, /*start*/ NO);
logSlicingFailureWithArray(samples, transaction, /*start*/ NO);
return nil;
} else {
SENTRY_LOG_DEBUG(@"Found last slice sample at index %lu", lastIndex);
}

const auto range = NSMakeRange(firstIndex, (lastIndex - firstIndex) + 1);
const auto indices = [NSIndexSet indexSetWithIndexesInRange:range];
return [samplesCopy objectsAtIndexes:indices];
return [samples objectsAtIndexes:indices];
}

@implementation SentrySample
Expand Down
Loading