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

Various crashes due to UI updates made in a secondary thread #364

Closed
herzbube opened this issue Mar 12, 2021 · 0 comments
Closed

Various crashes due to UI updates made in a secondary thread #364

herzbube opened this issue Mar 12, 2021 · 0 comments
Assignees
Milestone

Comments

@herzbube
Copy link
Owner

Because it is not thread-safe, UIKit in general wants updates to be performed on the main thread ([NSThread mainThread]). After releasing version 1.6.0 to the App Store, however, a number of new crashes were reported that all had the common cause that UI updates were performed in a secondary thread. Version 1.6.0 of the app was built with the iOS base SDK 14.3. Apparently iOS 14.3 has more checks that detect if UI updates are performed in a secondary thread, and these additional checks now let the app crash unconditionally more often. So while the crashes themselves are new, the underlying causes are not - older iOS versions simply did not detect the thread mismatches.

In the past thread mismatches were usually resolved by modifying the source of the UI update (e.g. a command) to perform the action that causes the UI update (e.g. sending a notification) on the main thread instead of on a secondary thread. The newest batch of crashes demonstrates that this approach is bound to fail:

  • It's not future proof: When a new command is introduced it is easy to forget that it may be executed in a secondary thread and thus needs to take special measures to ensure that it triggers UI updates on the main thread.
  • It introduces undesired coupling: A command should not need to know who reacts to a notification, or that the reacton needs to take place in a specific thread.

A new approach is therefore to make sure that the receiver of a notification triggers the UI update on the main thread. Something along this line:

- (void) notificationHandler:(NSNotification*)notification
{
  if ([NSThread currentThread] == [NSThread mainThread])
    [self updateUI];
  else
    [self performSelectorOnMainThread:@selector(updateUI) withObject:nil waitUntilDone:YES];
}

- (void) updateUI
{
  [...]
}
@herzbube herzbube added this to the 1.6.1 milestone Mar 12, 2021
@herzbube herzbube self-assigned this Mar 12, 2021
@herzbube herzbube changed the title Various crashes due to UI updates made in secondary threads Various crashes due to UI updates made in a secondary thread Mar 12, 2021
herzbube added a commit that referenced this issue Mar 12, 2021
linked crash reports
- Xcode Organizer
  - Crash report batch #1, consisting of 4 crash reports from a single
    device (an iPhone 8) running iOS 14.4.
- Crashlytics
  - Crash report batch #2, consisting of 1 crash report from an iPhone 8
    device running iOS 14.4.

all crashes occurred in method updateNavigationBarWidths of
NavigationBarControllerPhonePortraitOnly, on this line:
  [self.view removeConstraints:constraintsToRemove];
herzbube added a commit that referenced this issue Mar 12, 2021
linked crash reports
- Xcode Organizer
  - None
- Crashlytics
  - Crash report batch #3, consisting of 4 crash reports from a single
    device (a 6th generation iPad) running iOS 14.4.

all crashes occurred in method updateAutoLayoutConstraints of
StatusViewController, on this line:
  self.activityIndicatorSpacingConstraint.constant = 0.0f;
herzbube added a commit that referenced this issue Mar 12, 2021
…main thread (#364)

linked crash reports
- Xcode Organizer
  - None
- Crashlytics
  - Crash report batch #4, consisting of 4 crash reports from two
    devices (bothh 5th generation iPad) running iOS 14.4.0 and 14.4.1.

all crashes occurred in method updateCurrentBoardPosition of
BoardPositionTableListViewController, on this line:
  [self updateCurrentBoardPositionInBoardPositionListTableView];
herzbube added a commit that referenced this issue Mar 12, 2021
unlike in the previous commits, there are no crash reports indicating
that these changes are actually necessary. the changes are made to
implement the strategy change outlined in GitHub issue #364 (receivers
of notifications must make sure that UI updates occur on the main
thread).
herzbube added a commit that referenced this issue Sep 16, 2022
…ible (#364)

the main issue fixed in this commit is that the game info screen was
popped in a secondary thread.

also, the delay when popping ViewGameController from the navigation
stack is no longer necessary due to the changes made during UI layout
unification.

finally, the sequence of operations in ViewGameController was adjusted
to avoid a debug warning
  "Unbalanced calls to begin/end appearance transitions"
in iOS 13 and newer. for iOS 12 the debug warning is still emitted for
unknown reasons.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

1 participant