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 17 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.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
4 changes: 2 additions & 2 deletions .github/workflows/codeql-analysis.yml
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ jobs:
uses: actions/checkout@v3

- name: Initialize CodeQL
uses: github/codeql-action/init@f3feb00acb00f31a6f60280e6ace9ca31d91c76a # pin@v2
uses: github/codeql-action/init@29b1f65c5e92e24fe6b6647da1eaabe529cec70f # pin@v2
with:
languages: ${{ matrix.language }}

Expand All @@ -37,4 +37,4 @@ jobs:
-destination platform="iOS Simulator,OS=latest,name=iPhone 11 Pro" | xcpretty && exit ${PIPESTATUS[0]}

- name: Perform CodeQL Analysis
uses: github/codeql-action/analyze@f3feb00acb00f31a6f60280e6ace9ca31d91c76a # pin@v2
uses: github/codeql-action/analyze@29b1f65c5e92e24fe6b6647da1eaabe529cec70f # pin@v2
5 changes: 3 additions & 2 deletions .github/workflows/test.yml
Original file line number Diff line number Diff line change
Expand Up @@ -194,6 +194,7 @@ jobs:

# We split building and running tests in two steps so we know how long running the tests takes.
- name: Build tests
id: build_tests
run: ./scripts/xcode-test.sh ${{matrix.platform}} ${{matrix.test-destination-os}} $GITHUB_REF_NAME ci build-for-testing

- name: Run tests
Expand All @@ -209,7 +210,7 @@ jobs:

- name: Archiving DerivedData Logs
uses: actions/upload-artifact@v3
if: failure()
if: steps.build_tests.outcome == 'failure'
with:
name: derived-data-${{matrix.platform}}-xcode-${{matrix.xcode}}-os-${{matrix.test-destination-os}}
path: |
Expand Down Expand Up @@ -251,7 +252,7 @@ jobs:
- name: Push code coverage to codecov
id: codecov_2
uses: codecov/codecov-action@894ff025c7b54547a9a2a1e9f228beae737ad3c2 # pin@v3.1.3
if: steps.codecov_1.outcome == 'failure' && ${{ contains(matrix.platform, 'iOS') && !contains(github.ref, 'release') && github.event.schedule == '' }}
if: ${{ steps.codecov_1.outcome == 'failure' && contains(matrix.platform, 'iOS') && !contains(github.ref, 'release') && github.event.schedule == '' }}
with:
token: ${{ secrets.CODECOV_TOKEN }}
fail_ci_if_error: true
Expand Down
6 changes: 6 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,12 @@

## Unreleased

### Fixed

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

## 8.7.0

### Features

- Allow starting the SDK with an initial scope (#2982)
Expand Down
8 changes: 4 additions & 4 deletions Samples/iOS-Swift/iOS-Swift.xcodeproj/project.pbxproj
Original file line number Diff line number Diff line change
Expand Up @@ -1190,7 +1190,7 @@
"$(inherited)",
"@executable_path/Frameworks",
);
MARKETING_VERSION = 8.6.0;
MARKETING_VERSION = 8.7.0;
PRODUCT_BUNDLE_IDENTIFIER = "io.sentry.sample.iOS-Swift";
PRODUCT_NAME = "$(TARGET_NAME)";
PROVISIONING_PROFILE_SPECIFIER = "match Development io.sentry.sample.iOS-Swift";
Expand Down Expand Up @@ -1219,7 +1219,7 @@
"$(inherited)",
"@executable_path/Frameworks",
);
MARKETING_VERSION = 8.6.0;
MARKETING_VERSION = 8.7.0;
PRODUCT_BUNDLE_IDENTIFIER = "io.sentry.sample.iOS-Swift";
PRODUCT_NAME = "$(TARGET_NAME)";
PROVISIONING_PROFILE_SPECIFIER = "match AppStore io.sentry.sample.iOS-Swift";
Expand Down Expand Up @@ -1864,7 +1864,7 @@
"$(inherited)",
"@executable_path/Frameworks",
);
MARKETING_VERSION = 8.6.0;
MARKETING_VERSION = 8.7.0;
PRODUCT_BUNDLE_IDENTIFIER = "io.sentry.sample.iOS-Swift.Clip";
PRODUCT_NAME = "$(TARGET_NAME)";
PROVISIONING_PROFILE_SPECIFIER = "match Development io.sentry.sample.iOS-Swift.Clip";
Expand Down Expand Up @@ -1899,7 +1899,7 @@
"$(inherited)",
"@executable_path/Frameworks",
);
MARKETING_VERSION = 8.6.0;
MARKETING_VERSION = 8.7.0;
PRODUCT_BUNDLE_IDENTIFIER = "io.sentry.sample.iOS-Swift.Clip";
PRODUCT_NAME = "$(TARGET_NAME)";
PROVISIONING_PROFILE_SPECIFIER = "match AppStore io.sentry.sample.iOS-Swift.Clip";
Expand Down
4 changes: 2 additions & 2 deletions Sentry.podspec
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
Pod::Spec.new do |s|
s.name = "Sentry"
s.version = "8.6.0"
s.version = "8.7.0"
s.summary = "Sentry client for cocoa"
s.homepage = "https://github.com/getsentry/sentry-cocoa"
s.license = "mit"
Expand All @@ -27,7 +27,7 @@ Pod::Spec.new do |s|
}

s.default_subspecs = ['Core']
s.dependency "SentryPrivate", "8.6.0"
s.dependency "SentryPrivate", "8.7.0"

s.subspec 'Core' do |sp|
sp.source_files = "Sources/Sentry/**/*.{h,hpp,m,mm,c,cpp}",
Expand Down
2 changes: 1 addition & 1 deletion SentryPrivate.podspec
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
Pod::Spec.new do |s|
s.name = "SentryPrivate"
s.version = "8.6.0"
s.version = "8.7.0"
s.summary = "Sentry Private Library."
s.homepage = "https://github.com/getsentry/sentry-cocoa"
s.license = "mit"
Expand Down
4 changes: 2 additions & 2 deletions SentrySwiftUI.podspec
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
Pod::Spec.new do |s|
s.name = "SentrySwiftUI"
s.version = "8.6.0"
s.version = "8.7.0"
s.summary = "Sentry client for SwiftUI"
s.homepage = "https://github.com/getsentry/sentry-cocoa"
s.license = "mit"
Expand All @@ -19,5 +19,5 @@ Pod::Spec.new do |s|
s.watchos.framework = 'WatchKit'

s.source_files = "Sources/SentrySwiftUI/**/*.{swift,h,m}"
s.dependency 'Sentry', "8.6.0"
s.dependency 'Sentry', "8.7.0"
end
2 changes: 1 addition & 1 deletion Sources/Configuration/Sentry.xcconfig
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,6 @@ PRODUCT_NAME = Sentry
INFOPLIST_FILE = Sources/Sentry/Info.plist
PRODUCT_BUNDLE_IDENTIFIER = io.sentry.Sentry

CURRENT_PROJECT_VERSION = 8.6.0
CURRENT_PROJECT_VERSION = 8.7.0

MODULEMAP_FILE = $(SRCROOT)/Sources/Sentry/Sentry.modulemap
2 changes: 1 addition & 1 deletion Sources/Configuration/SentryPrivate.xcconfig
Original file line number Diff line number Diff line change
@@ -1,3 +1,3 @@
PRODUCT_NAME = SentryPrivate
MACH_O_TYPE = staticlib
CURRENT_PROJECT_VERSION = 8.6.0
CURRENT_PROJECT_VERSION = 8.7.0
2 changes: 1 addition & 1 deletion Sources/Sentry/SentryMeta.m
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ @implementation SentryMeta
// Don't remove the static keyword. If you do the compiler adds the constant name to the global
// symbol table and it might clash with other constants. When keeping the static keyword the
// compiler replaces all occurrences with the value.
static NSString *versionString = @"8.6.0";
static NSString *versionString = @"8.7.0";
static NSString *sdkName = @"sentry.cocoa";

+ (NSString *)versionString
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);

Check warning on line 59 in Sources/Sentry/SentryProfileTimeseries.mm

View check run for this annotation

Codecov / codecov/patch

Sources/Sentry/SentryProfileTimeseries.mm#L59

Added line #L59 was not covered by tests
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);

Check warning on line 75 in Sources/Sentry/SentryProfileTimeseries.mm

View check run for this annotation

Codecov / codecov/patch

Sources/Sentry/SentryProfileTimeseries.mm#L75

Added line #L75 was not covered by tests
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