Skip to content

Commit

Permalink
Fix crash "cannot form weak reference to instance of viewcontroller" (#…
Browse files Browse the repository at this point in the history 
…159)
  • Loading branch information
@ArielDemarco
ArielDemarco authored Jan 21, 2025
1 parent 3f48e0a commit 94accb7
Show file tree
Hide file tree
Showing 3 changed files with 84 additions and 37 deletions.
115 changes: 81 additions & 34 deletions Sources/EmbraceCore/Capture/UX/View/UIViewControllerHandler.swift
View file
Original file line number Diff line number Diff line change
Expand Up @@ -90,34 +90,40 @@ class UIViewControllerHandler {
// We generate the id here, outside of the `queue`, to ensure we're doing it while the ViewController is still alive (renerding process).
// There could be a race condition and it's possible that the controller was released or is in the process of deallocation,
// which could cause a crash (as this feature relies on objc_setAssociatedObject).
// This kind of operation should be done _for anything_ that accesses the UIViewController.
let id = UUID().uuidString
let state = ViewInstrumentationState()
state.viewDidLoadSpanCreated = true
state.identifier = id
vc.emb_instrumentation_state = state

let className = vc.className
let viewName = vc.emb_viewName

// check if with need to measure time-to-render or time-to-interactive
let nameFormat = vc is InteractableViewController ?
SpanSemantics.View.timeToInteractiveName :
SpanSemantics.View.timeToFirstRenderName

queue.async {
// generate parent span
let className = vc.className

// check if with need to measure time-to-render or time-to-interactive
let nameFormat = vc is InteractableViewController ?
SpanSemantics.View.timeToInteractiveName :
SpanSemantics.View.timeToFirstRenderName

let spanName = nameFormat.replacingOccurrences(of: "NAME", with: className)

let parentSpan = self.createSpan(
with: otel,
vc: vc,
viewName: viewName,
className: className,
name: spanName,
startTime: now
)

// generate view did load span
let viewDidLoadSpan = self.createSpan(
with: otel,
vc: vc,
viewName: viewName,
className: className,
name: SpanSemantics.View.viewDidLoadName,
startTime: now,
parent: parentSpan
Expand All @@ -129,9 +135,11 @@ class UIViewControllerHandler {
}

func onViewDidLoadEnd(_ vc: UIViewController, now: Date = Date()) {
guard let id = vc.emb_instrumentation_state?.identifier else {
return
}
queue.async {
guard let id = vc.emb_instrumentation_state?.identifier,
let span = self.viewDidLoadSpans.removeValue(forKey: id) else {
guard let span = self.viewDidLoadSpans.removeValue(forKey: id) else {
return
}

Expand All @@ -140,18 +148,26 @@ class UIViewControllerHandler {
}

func onViewWillAppearStart(_ vc: UIViewController, now: Date = Date()) {
guard let id = vc.emb_instrumentation_state?.identifier else {
return
}

vc.emb_instrumentation_state?.viewWillAppearSpanCreated = true

let className = vc.className
let viewName = vc.emb_viewName

queue.async {
guard let otel = self.dataSource?.otel,
let id = vc.emb_instrumentation_state?.identifier,
let parentSpan = self.parentSpans[id] else {
return
}

// generate view will appear span
let span = self.createSpan(
with: otel,
vc: vc,
viewName: viewName,
className: className,
name: SpanSemantics.View.viewWillAppearName,
startTime: now,
parent: parentSpan
Expand All @@ -162,9 +178,11 @@ class UIViewControllerHandler {
}

func onViewWillAppearEnd(_ vc: UIViewController, now: Date = Date()) {
guard let id = vc.emb_instrumentation_state?.identifier else {
return
}
queue.async {
guard let id = vc.emb_instrumentation_state?.identifier,
let span = self.viewWillAppearSpans.removeValue(forKey: id) else {
guard let span = self.viewWillAppearSpans.removeValue(forKey: id) else {
return
}

Expand All @@ -173,18 +191,26 @@ class UIViewControllerHandler {
}

func onViewIsAppearingStart(_ vc: UIViewController, now: Date = Date()) {
guard let id = vc.emb_instrumentation_state?.identifier else {
return
}

vc.emb_instrumentation_state?.viewIsAppearingSpanCreated = true

let className = vc.className
let viewName = vc.emb_viewName

queue.async {
guard let otel = self.dataSource?.otel,
let id = vc.emb_instrumentation_state?.identifier,
let parentSpan = self.parentSpans[id] else {
return
}

// generate view is appearing span
let span = self.createSpan(
with: otel,
vc: vc,
viewName: viewName,
className: className,
name: SpanSemantics.View.viewIsAppearingName,
startTime: now,
parent: parentSpan
Expand All @@ -206,18 +232,26 @@ class UIViewControllerHandler {
}

func onViewDidAppearStart(_ vc: UIViewController, now: Date = Date()) {
guard let id = vc.emb_instrumentation_state?.identifier else {
return
}

vc.emb_instrumentation_state?.viewDidAppearSpanCreated = true

let className = vc.className
let viewName = vc.emb_viewName

queue.async {
guard let otel = self.dataSource?.otel,
let id = vc.emb_instrumentation_state?.identifier,
let parentSpan = self.parentSpans[id] else {
return
}

// generate view did appear span
let span = self.createSpan(
with: otel,
vc: vc,
viewName: viewName,
className: className,
name: SpanSemantics.View.viewDidAppearName,
startTime: now,
parent: parentSpan
Expand All @@ -238,16 +272,25 @@ class UIViewControllerHandler {
}
}

guard let id = vc.emb_instrumentation_state?.identifier else {
// This should never happen
return
}

let className = vc.className
let viewName = vc.emb_viewName

queue.async {
guard let otel = self.dataSource?.otel, let id = vc.emb_instrumentation_state?.identifier else {
guard let otel = self.dataSource?.otel else {
return
}

// check if we need to create a visibility span
if self.dataSource?.instrumentVisibility == true {
let span = self.createSpan(
with: otel,
vc: vc,
viewName: viewName,
className: className,
name: SpanSemantics.View.screenName,
type: .view,
startTime: now
Expand All @@ -266,13 +309,14 @@ class UIViewControllerHandler {
// end time to first render span
if parentSpan.isTimeToFirstRender {
parentSpan.end(time: now)
self.clear(id: id, vc: vc)
self.clear(id: id)

// generate ui ready span
} else {
let span = self.createSpan(
with: otel,
vc: vc,
viewName: viewName,
className: className,
name: SpanSemantics.View.uiReadyName,
startTime: now,
parent: parentSpan
Expand All @@ -284,7 +328,7 @@ class UIViewControllerHandler {
span.end(time: now)
parentSpan.end(time: now)

self.clear(id: id, vc: vc)
self.clear(id: id)

// otherwise we save it to close it later
} else {
Expand All @@ -295,11 +339,11 @@ class UIViewControllerHandler {
}

func onViewDidDisappear(_ vc: UIViewController) {
queue.async {
guard let id = vc.emb_instrumentation_state?.identifier else {
return
}
guard let id = vc.emb_instrumentation_state?.identifier else {
return
}

queue.async {
let now = Date()

// end visibility span
Expand All @@ -314,9 +358,12 @@ class UIViewControllerHandler {
}

func onViewBecameInteractive(_ vc: UIViewController) {
guard let id = vc.emb_instrumentation_state?.identifier else {
return
}

queue.async {
guard let id = vc.emb_instrumentation_state?.identifier,
let parentSpan = self.parentSpans[id],
guard let parentSpan = self.parentSpans[id],
parentSpan.isTimeToInteractive else {
return
}
Expand All @@ -327,7 +374,7 @@ class UIViewControllerHandler {
let now = Date()
span.end(time: now)
parentSpan.end(time: now)
self.clear(id: id, vc: vc)
self.clear(id: id)

// otherwise it means the view is still loading, in this case we flag
// the view controller so we can close the spans as soon as
Expand Down Expand Up @@ -369,7 +416,8 @@ class UIViewControllerHandler {

private func createSpan(
with otel: EmbraceOpenTelemetry,
vc: UIViewController,
viewName: String,
className: String,
name: String,
type: SpanType = .viewLoad,
startTime: Date,
Expand All @@ -379,8 +427,8 @@ class UIViewControllerHandler {
name: name,
type: type,
attributes: [
SpanSemantics.View.keyViewTitle: vc.emb_viewName,
SpanSemantics.View.keyViewName: vc.className
SpanSemantics.View.keyViewTitle: viewName,
SpanSemantics.View.keyViewName: className
],
autoTerminationCode: nil
)
Expand All @@ -394,15 +442,14 @@ class UIViewControllerHandler {
return builder.startSpan()
}

private func clear(id: String, vc: UIViewController? = nil) {
private func clear(id: String) {
self.parentSpans[id] = nil
self.viewDidLoadSpans[id] = nil
self.viewWillAppearSpans[id] = nil
self.viewIsAppearingSpans[id] = nil
self.viewDidAppearSpans[id] = nil
self.uiReadySpans[id] = nil
self.alreadyFinishedUiReadyIds.remove(id)
vc?.emb_instrumentation_state = nil
}
}

Expand Down
2 changes: 1 addition & 1 deletion Sources/EmbraceCore/Capture/UX/View/ViewCaptureService.swift
View file
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ public final class ViewCaptureService: CaptureService, UIViewControllerHandlerDa
}

var instrumentFirstRender: Bool {
return options.instrumentFirstRender
return options.instrumentFirstRender && Embrace.client?.config?.isUiLoadInstrumentationEnabled == true
}

@objc public convenience init(options: ViewCaptureService.Options) {
Expand Down
4 changes: 2 additions & 2 deletions Tests/EmbraceCoreTests/Internal/Logs/Exporter/DefaultLogBatcherTests.swift
View file
Original file line number Diff line number Diff line change
Expand Up @@ -46,10 +46,10 @@ class DefaultLogBatcherTests: XCTestCase {
givenDefaultLogBatcher(limits: .init(maxBatchAge: 0.1, maxLogsPerBatch: 10))
givenRepositoryCreatesLogsSuccessfully()
whenInvokingAddLogRecord(withLogRecord: randomLogRecord())
thenDelegateShouldInvokeBatchFinishedAfterBatchLifespan(0.2)
thenDelegateShouldInvokeBatchFinishedAfterBatchLifespan(0.5)
self.delegate.didCallBatchFinished = false
whenInvokingAddLogRecord(withLogRecord: randomLogRecord())
thenDelegateShouldInvokeBatchFinishedAfterBatchLifespan(0.2)
thenDelegateShouldInvokeBatchFinishedAfterBatchLifespan(0.5)
}

func testAutoEndBatchAfterLifespanExpired_CancelWhenBatchEndedPrematurely() {
Expand Down

0 comments on commit 94accb7

Please sign in to comment.