Deadlock issue with forwardSelector:arguments:options: method

[bbernberg]

We are seeing a repeatable deadlock issue due to the @synchronized(self) lock in SEGIntegrationsManager.m's forwardSelector:arguments:options: method. We are seeing the issue when the app becomes active and the handleAppStateNotification path is executed.

[bbernberg]

FYI, here is an example snapshot of the deadlock. As can be seen, Thread 26 (which has the lock) is attempting to access the main thread, which is attempting to obtain lock:

[raylillywhite]

Pretty sure this is the same issue, but the stack trace where we're seeing this is a bit different, so in case it's helpful for debugging:

[f2prateek]

Thanks for the report! @tonyxiao is looking into this.

[bbernberg]

FYI I tried a different approach using serial dispatch queues & a serial NSOperationQueue but both approaches ran into the same problem. Is there a reason the App State Notifications need to be handled synchronously from the main thread? I don't have enough background knowledge of the full system to understand if this is truly a requirement.

[tonyxiao]

@bbernberg I'm trying to investigate this right now. Do you happen to have a minimal example that you can put on github reproducing this issue?

[raylillywhite]

This regularly reproduces it on my iPhone 7 Plus from backgrounding and foregrounding the app a couple times: https://gist.github.com/raylillywhite/cb74d4d1b9325bfcde94119170be55ec

In the app where I'm experiencing it, we're obviously not sending anywhere near that many events, and it happens every time we turn on airplane mode, launch the app, background it, turn off airplane mode, foreground the app. I wasn't able to successfully reproduce that with a minimal example though.

[bbernberg]

@raylillywhite 's example looks perfect. It definitely seems to be the App State Notifications deadlocking with other events fired from the main thread.

[tonyxiao]

@bbernberg @raylillywhite we've got a fix on deadlock branch (#684) that works in our test setup, I'd love to get some confirmation that it does indeed resolve the issue you're seeing. Can you try out the branch with pod 'Analytics', :git => 'https://github.com/segmentio/analytics-ios.git', :branch => 'deadlock'?. We can make a new release with this fix right after.

[raylillywhite]

That branch does fix the issue in our main app as well 🎉 Thanks for making this fix a priority!

[bbernberg]

@tonyxiao this unfortunately did not fix the issue for us. Here's a stack trace from that branch:

[tonyxiao]

@bbernberg my best guess right now is that Taplytics tries to dispatch to the main thread synchronously within their logEvent:value:metadata method. However their SDK is closed source so it's not easy to tell. Will try to contact them to get more info and report back here.

cc @TeresaNesteby @ladanazita

[bbernberg]

@tonyxiao any update on this? We are still seeing this issue in production unfortunately.

[tonyxiao]

@bbernberg We reached out to Taplytics but have not heard back yet. In the meantime I'm going to add a workaround in the core library.

[f2prateek]

@tonyxiao did we add a workaround here?

[tonyxiao]

on the roadmap, will circle back to this next week.

[bbernberg]

Any more updates on this? We are about to launch a feature that is seeing this issue at a very high rate. My previous attempts at workarounds are no longer working.

[ladanazita]

@bbernberg Sorry for the delay here. I'm going to reach out to Taplytics again and see if they can jump in and help out. Will update as soon as I have more.

[tonyxiao]

@bbernberg we got in touch with the Taplytics team and are sorting out the right solution together.

[ladanazita]

@bbernberg I apologize for the delay again.

We decided to change our Taplytics integration to call Taplytics asynchronously rather than have Taplytics/Segment modify the core SDK.

Since this issue is outside the scope of our core SDK, I will close this here and track this issue internally in addition to the Taplytics repo for you to follow:
segment-integrations/analytics-ios-integration-taplytics#17