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

Prevent assertions from executing in release builds #1149

Open
wants to merge 1 commit into
base: release/3.0
Choose a base branch
from

Conversation

twobitlabs
Copy link

Currently assertions are enabled even when building for Release

@tonyarnold
Copy link
Contributor

This is on purpose. I've been shipping with assertions enabled for a while now — crashing is better than data corruption or inconsistent or irrecoverable state. Is this causing you trouble?

@twobitlabs
Copy link
Author

@tonyarnold I generally agree with that approach. However, here's what we're seeing:

  1. At the end of applicationWillTerminate:, we call [MagicalRecord cleanUp], per the documentation
  2. That nils out MagicalRecordStack.defaultStack
  3. Subsequent to that, but before the app exits, a background thread processing a network response retrieves a context using MagicalRecordStack.defaultStack()?.context. At this point the app crashes on the assertion that defaultStack is not nil

It's not a huge deal because the crash is not user-facing (the app is already being ejected), but it is noise in crashlytics (currently our most common crash). It's also possible that the crash prevents other libraries that listen for UIApplicationWillTerminateNotification are interrupted by the crash. It looks like in many cases Flurry is in the middle of ending its session when this occurs.

So perhaps the real question is whether it's really necessary to call [MagicalRecord cleanUp] when the app exits, since you can't be sure that a call to MagicalRecordStack.defaultStack() won't occur after the cleanUp.

Thoughts? 🍺🍺😀

@tonyarnold
Copy link
Contributor

Honestly, I think you can get away without calling cleanup. I will have a good look when I get home, but it's really a holdover from pre-ARC times in terms of function.

@twobitlabs
Copy link
Author

That's what I was thinking when I looked at what it does, but I thought I might be missing something. Thanks!

@Coeur Coeur added this to the 3.0.0 milestone May 31, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants