Skip to content

Commit

Permalink
Fix Core Data threading crashes (#569)
Browse files Browse the repository at this point in the history
[beta] [clowntown]

* Fix Core Data threading crashes

* WIP still

* More fixes

* Fix tests

* Some removals

* Fix the final boss of threading problems

* Revert status service changes

* Remove changes in safeMinAppVersion

* Fix TeamMedia setter crashes
  • Loading branch information
ZachOrr authored May 10, 2019
1 parent aedfd4a commit e4a4a51
Show file tree
Hide file tree
Showing 43 changed files with 410 additions and 301 deletions.
11 changes: 0 additions & 11 deletions tba-unit-tests/Core Data/Team/TeamMediaTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -127,17 +127,6 @@ class TeamMediaTestCase: CoreDataTestCase {
XCTAssertEqual(media.youtubeKey, foreignKey)
}

func test_viewImageURL() {
let media = TeamMedia.init(entity: TeamMedia.entity(), insertInto: persistentContainer.viewContext)

// No url - nil
XCTAssertNil(media.viewImageURL)

// cdPhotoThread
media.viewURL = "http://www.chiefdelphi.com/media/photos/foreign_key"
XCTAssertEqual(media.viewImageURL?.absoluteString, "http://www.chiefdelphi.com/media/photos/foreign_key")
}

func test_imageDirectURL() {
let media = TeamMedia.init(entity: TeamMedia.entity(), insertInto: persistentContainer.viewContext)

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -57,10 +57,13 @@
buildConfiguration = "Debug"
selectedDebuggerIdentifier = "Xcode.DebuggerFoundation.Debugger.LLDB"
selectedLauncherIdentifier = "Xcode.DebuggerFoundation.Launcher.LLDB"
enableThreadSanitizer = "YES"
launchStyle = "0"
useCustomWorkingDirectory = "NO"
ignoresPersistentStateOnLaunch = "NO"
debugDocumentVersioning = "YES"
stopOnEveryThreadSanitizerIssue = "YES"
stopOnEveryMainThreadCheckerIssue = "YES"
debugServiceExtension = "internal"
allowLocationSimulation = "YES">
<BuildableProductRunnable
Expand All @@ -76,6 +79,10 @@
<CommandLineArguments>
<CommandLineArgument
argument = "-com.apple.CoreData.ConcurrencyDebug 1"
isEnabled = "YES">
</CommandLineArgument>
<CommandLineArgument
argument = "-com.apple.CoreData.Logging.stderr"
isEnabled = "NO">
</CommandLineArgument>
<CommandLineArgument
Expand Down
23 changes: 11 additions & 12 deletions the-blue-alliance-ios/App Delegate/AppDelegate.swift
Original file line number Diff line number Diff line change
Expand Up @@ -182,15 +182,14 @@ class AppDelegate: UIResponder, UIApplicationDelegate {
// Register retries for our status service on the main thread
DispatchQueue.main.async {
self.statusService.registerRetryable(initiallyRetry: true)
}

// Check our minimum app version - abort app flow if necessary
if !self.isAppVersionSupported(minimumAppVersion: self.statusService.status.safeMinAppVersion) {
self.showMinimumAppVersionAlert(status: self.statusService.status)
return
}
// Check our minimum app version
let mininmumAppVersion = self.statusService.status.safeMinAppVersion
if !AppDelegate.isAppVersionSupported(minimumAppVersion: mininmumAppVersion) {
self.showMinimumAppVersionAlert(currentAppVersion: self.statusService.status.latestAppVersion!.intValue)
return
}

DispatchQueue.main.async {
guard let window = self.window else {
fatalError("Window not setup when setting root vc")
}
Expand Down Expand Up @@ -355,21 +354,21 @@ extension AppDelegate: GIDSignInDelegate {
}
}

func isAppVersionSupported(minimumAppVersion: Int) -> Bool {
static func isAppVersionSupported(minimumAppVersion: Int) -> Bool {
if ProcessInfo.processInfo.arguments.contains("-testUnsupportedVersion") {
return true
}

return Bundle.main.buildVersionNumber >= minimumAppVersion
}

func showMinimumAppVersionAlert(status: Status) {
func showMinimumAppVersionAlert(currentAppVersion: Int) {
guard let window = window else {
return
}

DispatchQueue.main.async {
AppDelegate.showMinimumAppAlert(appStoreID: "1441973916", currentAppVersion: status.safeMinAppVersion, in: window)
AppDelegate.showMinimumAppAlert(appStoreID: "1441973916", currentAppVersion: currentAppVersion, in: window)
}
}

Expand All @@ -378,8 +377,8 @@ extension AppDelegate: GIDSignInDelegate {
extension AppDelegate: StatusSubscribable {

func statusChanged(status: Status) {
if !isAppVersionSupported(minimumAppVersion: status.safeMinAppVersion) {
showMinimumAppVersionAlert(status: status)
if !AppDelegate.isAppVersionSupported(minimumAppVersion: status.safeMinAppVersion) {
showMinimumAppVersionAlert(currentAppVersion: self.statusService.status.latestAppVersion!.intValue)
}
}

Expand Down
11 changes: 6 additions & 5 deletions the-blue-alliance-ios/Core Data/District/District.swift
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ extension District {
The district championship for a district. A nil value means the DCMP hasn't been fetched yet.
*/
private var districtChampionship: Event? {
guard let events = events?.allObjects as? [Event] else {
guard let eventsSet = getValue(\District.events), let events = eventsSet.allObjects as? [Event] else {
return nil
}
return events.first(where: { (event) -> Bool in
Expand All @@ -27,22 +27,23 @@ extension District {
If the district is currently "in season", meaning it's after stop build day, but before the district CMP is over
*/
var isHappeningNow: Bool {
if year!.intValue != Calendar.current.year {
let year = getValue(\District.year!.intValue)
if year != Calendar.current.year {
return false
}
// If we can't find the district championship, we don't know if we're in season or not
guard let districtChampionship = districtChampionship else {
guard let dcmpEndDate = endDate else {
return false
}
let startOfEvents = Calendar.current.stopBuildDay()
return Date().isBetween(date: startOfEvents, andDate: districtChampionship.endDate!.endOfDay())
return Date().isBetween(date: startOfEvents, andDate: dcmpEndDate.endOfDay())
}

/**
The 'end date' for the district - the end date of the district championship
*/
var endDate: Date? {
return districtChampionship?.endDate
return districtChampionship?.getValue(\Event.endDate)
}

}
Expand Down
13 changes: 10 additions & 3 deletions the-blue-alliance-ios/Core Data/District/DistrictRanking.swift
Original file line number Diff line number Diff line change
Expand Up @@ -6,11 +6,18 @@ extension DistrictRanking {

var sortedEventPoints: [DistrictEventPoints] {
// TODO: This sort is going to be problematic if we don't have Event objects
return (eventPoints?.allObjects as? [DistrictEventPoints])?.sorted(by: { (lhs, rhs) -> Bool in
guard let lhsStartDate = lhs.eventKey?.event?.startDate else {
let eventPointsSet = getValue(\DistrictRanking.eventPoints)
return (eventPointsSet?.allObjects as? [DistrictEventPoints])?.sorted(by: { (lhs, rhs) -> Bool in
guard let lhsEventKey = lhs.getValue(\DistrictEventPoints.eventKey) else {
return false
}
guard let rhsStartDate = rhs.eventKey?.event?.startDate else {
guard let lhsStartDate = lhsEventKey.event?.getValue(\Event.startDate) else {
return false
}
guard let rhsEventKey = rhs.getValue(\DistrictEventPoints.eventKey) else {
return false
}
guard let rhsStartDate = rhsEventKey.event?.getValue(\Event.startDate) else {
return false
}
return rhsStartDate > lhsStartDate
Expand Down
49 changes: 28 additions & 21 deletions the-blue-alliance-ios/Core Data/Event/Event.swift
Original file line number Diff line number Diff line change
Expand Up @@ -403,29 +403,30 @@ extension Event: Locatable, Surfable, Managed {
}

public var weekString: String {
var weekString = "nil"
let eventType = self.eventType!.intValue
let year = self.year!.intValue

if eventType == EventType.championshipDivision.rawValue || eventType == EventType.championshipFinals.rawValue {
if year!.intValue >= 2017, let city = city {
weekString = "Championship - \(city)"
if year >= 2017, let city = city {
return "Championship - \(city)"
} else {
weekString = "Championship"
return "Championship"
}
} else {
switch eventType {
case EventType.unlabeled.rawValue:
weekString = "Other"
return "Other"
case EventType.preseason.rawValue:
weekString = "Preseason"
return "Preseason"
case EventType.offseason.rawValue:
guard let month = month else {
return "Offseason"
}
return "\(month) Offseason"
case EventType.festivalOfChampions.rawValue:
weekString = "Festival of Champions"
return "Festival of Champions"
default:
guard let week = week else {
guard let week = week?.intValue else {
return "Other"
}

Expand All @@ -436,16 +437,15 @@ extension Event: Locatable, Surfable, Managed {
*/
if year == 2016 {
if week == 0 {
weekString = "Week 0.5"
return "Week 0.5"
} else {
weekString = "Week \(week.intValue)"
return "Week \(week)"
}
} else {
weekString = "Week \(week.intValue + 1)"
return "Week \(week + 1)"
}
}
}
return weekString
}

public var safeShortName: String {
Expand All @@ -459,50 +459,57 @@ extension Event: Locatable, Surfable, Managed {
return "\(year!.stringValue) \(safeShortName) \(eventTypeString ?? "Event")"
}

/**
If the event is a CMP division or a CMP finals field.
*/
public var isChampionship: Bool {
let type = eventType!.intValue
let type = getValue(\Event.eventType!.intValue)
return type == EventType.championshipDivision.rawValue || type == EventType.championshipFinals.rawValue
}

/**
If the event is a district championship or a district championship division.
*/
public var isDistrictChampionshipEvent: Bool {
let type = eventType!.intValue
let type = getValue(\Event.eventType!.intValue)
return type == EventType.districtChampionshipDivision.rawValue || type == EventType.districtChampionship.rawValue
}

/**
If the event is a district championship.
*/
public var isDistrictChampionship: Bool {
return eventType!.intValue == EventType.districtChampionship.rawValue
let type = getValue(\Event.eventType!.intValue)
return type == EventType.districtChampionship.rawValue
}

public var isFoC: Bool {
return eventType!.intValue == EventType.festivalOfChampions.rawValue;
let type = getValue(\Event.eventType!.intValue)
return type == EventType.festivalOfChampions.rawValue;
}

public var isPreseason: Bool {
return eventType!.intValue == EventType.preseason.rawValue;
let type = getValue(\Event.eventType!.intValue)
return type == EventType.preseason.rawValue;
}

public var isOffseason: Bool {
return eventType!.intValue == EventType.offseason.rawValue;
let type = getValue(\Event.eventType!.intValue)
return type == EventType.offseason.rawValue;
}

/**
If the event is currently going, based on it's start and end dates.
*/
public var isHappeningNow: Bool {
guard let startDate = startDate, let endDate = endDate else {
guard let startDate = getValue(\Event.startDate), let endDate = getValue(\Event.endDate) else {
return false
}
return Date().isBetween(date: startDate, andDate: endDate.endOfDay())
}

public var month: String? {
guard let startDate = startDate else {
guard let startDate = getValue(\Event.startDate) else {
return nil
}
let dateFormatter = DateFormatter()
Expand Down Expand Up @@ -645,7 +652,7 @@ extension Event: Comparable {
extension Event: MyTBASubscribable {

public var modelKey: String {
return key!
return getValue(\Event.key!)
}

public var modelType: MyTBAModelType {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,18 @@ import Foundation
import CoreData
import Crashlytics

extension NSManagedObject {

/** Syncronous, thread-safe method to get a typed value from a NSManagedObject. **/
func getValue<T, J>(_ keyPath: KeyPath<T, J>) -> J {
guard let context = managedObjectContext else {
fatalError("No managedObjectContext for object.")
}
return context.getKeyPathAndWait(obj: self, keyPath: keyPath)!
}

}

extension NSManagedObjectContext {

func insertObject<A: NSManagedObject>() -> A where A: Managed {
Expand Down Expand Up @@ -43,6 +55,41 @@ extension NSManagedObjectContext {
}
}

func performAndWait<T>(_ block: () -> T?) -> T? {
var result: T? = nil
performAndWait {
result = block()
}
return result
}

fileprivate func getKeyPathAndWait<T, J>(obj: NSManagedObject, keyPath: KeyPath<T, J>) -> J? {
guard let keyPathString = keyPath._kvcKeyPathString else {
fatalError("Unable to get key path string for \(keyPath)")
}
return performAndWait {
return obj.value(forKeyPath: keyPathString) as? J
}
}

fileprivate func setKeyPathAndWait<T, J>(obj: NSManagedObject, value: J, keyPath: KeyPath<T, J>) {
guard let keyPathString = keyPath._kvcKeyPathString else {
fatalError("Unable to get key path string for \(keyPath)")
}
performAndWait {
obj.setValue(value, forKeyPath: keyPathString)
}
}

fileprivate func setNilKeyPathAndWait<T, J>(obj: NSManagedObject, keyPath: KeyPath<T, J>) {
guard let keyPathString = keyPath._kvcKeyPathString else {
fatalError("Unable to get key path string for \(keyPath)")
}
performAndWait {
obj.setNilValueForKey(keyPathString)
}
}

func deleteAllObjects() {
for entity in persistentStoreCoordinator?.managedObjectModel.entities ?? [] {
deleteAllObjectsForEntity(entity: entity)
Expand Down
2 changes: 1 addition & 1 deletion the-blue-alliance-ios/Core Data/Match/Match.swift
Original file line number Diff line number Diff line change
Expand Up @@ -289,7 +289,7 @@ extension Match: Managed {
extension Match: MyTBASubscribable {

public var modelKey: String {
return key!
return getValue(\Match.key!)
}

public var modelType: MyTBAModelType {
Expand Down
1 change: 1 addition & 0 deletions the-blue-alliance-ios/Core Data/Protocols/Locatable.swift
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import Foundation

// Does the model have enough information to be found by a travel agent.
protocol Locatable {
var city: String? { get }
var stateProv: String? { get }
Expand Down
5 changes: 3 additions & 2 deletions the-blue-alliance-ios/Core Data/Team/Team.swift
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,8 @@ extension Team: Locatable, Surfable, Managed {
This may seem backwards, providing a TeamKey from a Team, but since TeamAtEventViewController doesn't handle either a Team *or* a TeamKey, we need to pull a TeamKey for controllers that have full Team objects (ex: TeamsViewController)
*/
var teamKey: TeamKey {
return TeamKey.insert(withKey: key!, in: managedObjectContext!)
let key = getValue(\Team.key!)
return TeamKey.insert(withKey: key, in: managedObjectContext!)
}

/**
Expand Down Expand Up @@ -168,7 +169,7 @@ extension Team: Locatable, Surfable, Managed {
extension Team: MyTBASubscribable {

public var modelKey: String {
return key!
return getValue(\Team.key!)
}

public var modelType: MyTBAModelType {
Expand Down
Loading

0 comments on commit e4a4a51

Please sign in to comment.