Skip to content

Thread aware live-report capture #158

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

Merged
merged 3 commits into from
Jun 16, 2025
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
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
8 changes: 8 additions & 0 deletions Backtrace.xcodeproj/project.pbxproj
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,9 @@
2062D9C92C457B4500E4CE3C /* Crash+CoreDataProperties.swift in Sources */ = {isa = PBXBuildFile; fileRef = 2062D9C52C457B4500E4CE3C /* Crash+CoreDataProperties.swift */; };
2062D9CA2C457B4500E4CE3C /* Crash+CoreDataClass.swift in Sources */ = {isa = PBXBuildFile; fileRef = 2062D9C42C457B4500E4CE3C /* Crash+CoreDataClass.swift */; };
2062D9CB2C457B4500E4CE3C /* Crash+CoreDataProperties.swift in Sources */ = {isa = PBXBuildFile; fileRef = 2062D9C52C457B4500E4CE3C /* Crash+CoreDataProperties.swift */; };
20B01A542DEE4DCA00FED98F /* BacktraceReporterThreadSpec.swift in Sources */ = {isa = PBXBuildFile; fileRef = 20B01A532DEE4DC600FED98F /* BacktraceReporterThreadSpec.swift */; };
20B01A552DEE4DCA00FED98F /* BacktraceReporterThreadSpec.swift in Sources */ = {isa = PBXBuildFile; fileRef = 20B01A532DEE4DC600FED98F /* BacktraceReporterThreadSpec.swift */; };
20B01A562DEE4DCA00FED98F /* BacktraceReporterThreadSpec.swift in Sources */ = {isa = PBXBuildFile; fileRef = 20B01A532DEE4DC600FED98F /* BacktraceReporterThreadSpec.swift */; };
20DE4B342D4830D80076B3F6 /* NSManagedObjectContext+Extensions.swift in Sources */ = {isa = PBXBuildFile; fileRef = 20DE4B332D4830D00076B3F6 /* NSManagedObjectContext+Extensions.swift */; };
20DE4B352D4830D80076B3F6 /* NSManagedObjectContext+Extensions.swift in Sources */ = {isa = PBXBuildFile; fileRef = 20DE4B332D4830D00076B3F6 /* NSManagedObjectContext+Extensions.swift */; };
20DE4B362D4830D80076B3F6 /* NSManagedObjectContext+Extensions.swift in Sources */ = {isa = PBXBuildFile; fileRef = 20DE4B332D4830D00076B3F6 /* NSManagedObjectContext+Extensions.swift */; };
Expand Down Expand Up @@ -425,6 +428,7 @@
2050DBBE2C66D98500C6CCA9 /* PrivacyInfo.xcprivacy */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = text.xml; path = PrivacyInfo.xcprivacy; sourceTree = "<group>"; };
2062D9C42C457B4500E4CE3C /* Crash+CoreDataClass.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = "Crash+CoreDataClass.swift"; sourceTree = "<group>"; };
2062D9C52C457B4500E4CE3C /* Crash+CoreDataProperties.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = "Crash+CoreDataProperties.swift"; sourceTree = "<group>"; };
20B01A532DEE4DC600FED98F /* BacktraceReporterThreadSpec.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = BacktraceReporterThreadSpec.swift; sourceTree = "<group>"; };
20D4E5F32CB46A41000C92BF /* BacktraceResources-Info.plist */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = text.plist.xml; path = "BacktraceResources-Info.plist"; sourceTree = "<group>"; };
20DE4B332D4830D00076B3F6 /* NSManagedObjectContext+Extensions.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = "NSManagedObjectContext+Extensions.swift"; sourceTree = "<group>"; };
20DE4B372D48615C0076B3F6 /* NSManagedObjectContextExtensionTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = NSManagedObjectContextExtensionTests.swift; sourceTree = "<group>"; };
Expand Down Expand Up @@ -884,6 +888,7 @@
F266B85321C77D5D00D14417 /* Tests */ = {
isa = PBXGroup;
children = (
20B01A532DEE4DC600FED98F /* BacktraceReporterThreadSpec.swift */,
20DE4B372D48615C0076B3F6 /* NSManagedObjectContextExtensionTests.swift */,
A24A4B5428B595D8004F5052 /* AttachmentStorageTests.swift */,
A24A4B4F28B595D8004F5052 /* AttachmentTests.swift */,
Expand Down Expand Up @@ -2116,6 +2121,7 @@
A24A4B6228B595D9004F5052 /* BacktraceFileManagerTests.swift in Sources */,
AFCCCE252625392300B83A28 /* ReportMetadataStorageMock.swift in Sources */,
A24A4B7128B595D9004F5052 /* BacktraceCredentialsTests.swift in Sources */,
20B01A562DEE4DCA00FED98F /* BacktraceReporterThreadSpec.swift in Sources */,
A24A4B5F28B595D9004F5052 /* BacktraceDatabaseTests.swift in Sources */,
A24A4B9328B59653004F5052 /* BacktraceNotificationObserverMock.swift in Sources */,
A24A4B6B28B595D9004F5052 /* BacktraceRateLimiterTests.swift in Sources */,
Expand Down Expand Up @@ -2209,6 +2215,7 @@
A24A4B8228B595D9004F5052 /* BacktraceBreadcrumbTests.swift in Sources */,
A24A4B7628B595D9004F5052 /* DispatcherTests.swift in Sources */,
A24A4B6728B595D9004F5052 /* BacktraceOomWatcherTests.swift in Sources */,
20B01A542DEE4DCA00FED98F /* BacktraceReporterThreadSpec.swift in Sources */,
F2AB636B2244243500939BC9 /* Quick+Throws.swift in Sources */,
A24A4B5B28B595D9004F5052 /* BacktraceWatcherTests.swift in Sources */,
A24A4B7928B595D9004F5052 /* AttributesTests.swift in Sources */,
Expand Down Expand Up @@ -2324,6 +2331,7 @@
A24A4B8128B595D9004F5052 /* BacktraceBreadcrumbTests.swift in Sources */,
A24A4B7528B595D9004F5052 /* DispatcherTests.swift in Sources */,
A24A4B6628B595D9004F5052 /* BacktraceOomWatcherTests.swift in Sources */,
20B01A552DEE4DCA00FED98F /* BacktraceReporterThreadSpec.swift in Sources */,
F2AB636A2244243500939BC9 /* Quick+Throws.swift in Sources */,
A24A4B5A28B595D9004F5052 /* BacktraceWatcherTests.swift in Sources */,
A24A4B7828B595D9004F5052 /* AttributesTests.swift in Sources */,
Expand Down
4 changes: 2 additions & 2 deletions Podfile.lock
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
PODS:
- Backtrace (2.0.6):
- Backtrace (2.0.8):
- PLCrashReporter (= 1.11.1)
- Nimble (10.0.0)
- PLCrashReporter (1.11.1)
Expand All @@ -22,7 +22,7 @@ EXTERNAL SOURCES:
:path: "./Backtrace.podspec"

SPEC CHECKSUMS:
Backtrace: 72bea7c55570b42ab18dd7218ce41ceef3edca14
Backtrace: 5bcc05d7ef650d354f3ec3bdba76462d5edf5530
Nimble: 5316ef81a170ce87baf72dd961f22f89a602ff84
PLCrashReporter: 5d2d3967afe0efad61b3048d617e2199a5d1b787
Quick: 749aa754fd1e7d984f2000fe051e18a3a9809179
Expand Down
1 change: 1 addition & 0 deletions Sources/Features/Client/BacktraceReporter.swift
Original file line number Diff line number Diff line change
Expand Up @@ -118,6 +118,7 @@ extension BacktraceReporter {
faultMessage: String? = nil) throws -> BacktraceReport {
attributesProvider.set(faultMessage: faultMessage)
let resource = try reporter.generateLiveReport(exception: exception,
thread: mach_thread_self(),
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we always use the main thread id instead?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, we want to capture the specific currently executing thread, the main thread would be missing context and/or reporting wrong informations if the error occurs on a background thread for example.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  • callers who want main thread can still pass it explicitly.

attributes: attributesProvider.allAttributes,
attachmentPaths: attachmentPaths + attributesProvider.attachmentPaths)

Expand Down
19 changes: 19 additions & 0 deletions Sources/Public/BacktraceCrashReporter.swift
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,25 @@ extension BacktraceCrashReporter: CrashReporting {
let reportData = try reporter.generateLiveReport(with: exception)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it would be better if you will stick to one implementation for everything. The generateLiveReport (no thread argument) can call generateLiveReport with a thread argument by using current thread. You don't need to have a duplicated logic. If you start extending it at some point, you would need to double your code

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

sure thing, motivation was offering both methods without introducing a breaking change.

return try BacktraceReport(report: reportData, attributes: attributes, attachmentPaths: attachmentPaths)
}

func generateLiveReport(exception: NSException? = nil,
thread: mach_port_t = mach_thread_self(),
attributes: Attributes,
attachmentPaths: [String] = []) throws -> BacktraceReport {

defer { mach_port_deallocate(mach_task_self_, thread) }
let reportData = try reporter.generateLiveReport(withThread: thread, exception: exception)

return try BacktraceReport(report: reportData, attributes: attributes, attachmentPaths: attachmentPaths)
}

// Returns the thread ID
// can be used for logging/testing
static func currentThreadIdentifier() -> thread_t {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

shouldn't this be a helper? We already had a lot of code in Common. This class should be as clean as possible since this is a business logic of our SDK

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Common is for example app, removing this method since it's useless after consolidating generateLiveReport methods.

let tid = mach_thread_self()
mach_port_deallocate(mach_task_self_, tid)
return tid
}

func enableCrashReporting() throws {
try reporter.enableAndReturnError()
Expand Down
7 changes: 6 additions & 1 deletion Sources/Public/Internal/CrashReporting.swift
Original file line number Diff line number Diff line change
@@ -1,7 +1,12 @@
import Foundation

protocol CrashReporting {
func generateLiveReport(exception: NSException?, attributes: Attributes,
func generateLiveReport(exception: NSException?,
attributes: Attributes,
attachmentPaths: [String]) throws -> BacktraceReport
func generateLiveReport(exception: NSException?,
thread: mach_port_t,
attributes: Attributes,
attachmentPaths: [String]) throws -> BacktraceReport
func pendingCrashReport() throws -> BacktraceReport
func purgePendingCrashReport() throws
Expand Down
87 changes: 87 additions & 0 deletions Tests/BacktraceReporterThreadSpec.swift
Original file line number Diff line number Diff line change
@@ -0,0 +1,87 @@
import Quick
import Nimble
import CrashReporter
@testable import Backtrace

final class BacktraceReporterThreadSpec: QuickSpec {

override func spec() {

describe("Thread-aware live-report capture") {

var backtraceReporter: BacktraceReporter!
var mockPLCR: PLCrashReporterMock!
var urlSession: URLSessionMock!
var credentials: BacktraceCredentials!

beforeEach {
credentials = BacktraceCredentials(
endpoint: URL(string: "https://example.backtrace.io")!,
token: ""
)
urlSession = URLSessionMock()
mockPLCR = PLCrashReporterMock()

let crashReporter = BacktraceCrashReporter(reporter: mockPLCR)

let api = BacktraceApi(
credentials: credentials,
session: urlSession,
reportsPerMin: 30
)

backtraceReporter = try! BacktraceReporter(
reporter: crashReporter,
api: api,
dbSettings: BacktraceDatabaseSettings(),
credentials: credentials,
urlSession: urlSession
)
}

it("passes the calling thread to PLCrashReporter") {
let callThread = mach_thread_self()
_ = try? backtraceReporter.generate()
expect(mockPLCR.lastThread).to(equal(callThread))
mach_port_deallocate(mach_task_self_, callThread)
}

it("does not leak Mach SEND rights (> 2 refs to cover noise from Quick/Nimble or XCTest)") {
let before = sendRefCount()
_ = try? backtraceReporter.generate()
let after = sendRefCount()
expect(after).to(beLessThanOrEqualTo(before + 2))
}
}
}
}

/// Mock PLCrashReporter stubs that records the thread parameter
private final class PLCrashReporterMock: PLCrashReporter {

private(set) var lastThread: thread_t = mach_thread_self()

override func generateLiveReport(
withThread thread: thread_t,
exception: NSException?
) throws -> Data {
lastThread = thread
return Data([0xCA, 0xFE])
}

override func generateLiveReport(with exception: NSException?) throws -> Data {
return Data([0xBE, 0xEF])
}
}

/// Current ref-count for thread
private func sendRefCount() -> mach_port_urefs_t {
var refs: mach_port_urefs_t = 0
mach_port_get_refs(
mach_task_self_,
mach_thread_self(),
MACH_PORT_RIGHT_SEND,
&refs
)
return refs
}