Skip to content

Commit

Permalink
Merge pull request #879 from kiwix/single-background-context
Browse files Browse the repository at this point in the history 
Fix Crash with Single DB background context
  • Loading branch information
@kelson42
kelson42 committed Jul 25, 2024
2 parents ef8d6b5 + 71d38cf commit 2b1f49d
Show file tree
Hide file tree
Showing 18 changed files with 126 additions and 173 deletions.
4 changes: 2 additions & 2 deletions App/App_iOS.swift
View file
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ struct Kiwix: App {
WindowGroup {
RootView()
.ignoresSafeArea()
.environment(\.managedObjectContext, Database.viewContext)
.environment(\.managedObjectContext, Database.shared.viewContext)
.environmentObject(library)
.environmentObject(navigation)
.modifier(AlertHandler())
Expand All @@ -45,7 +45,7 @@ struct Kiwix: App {
.modifier(SaveContentHandler())
.onChange(of: scenePhase) { newValue in
guard newValue == .inactive else { return }
try? Database.viewContext.save()
try? Database.shared.viewContext.save()
}
.onOpenURL { url in
if url.isFileURL {
Expand Down
8 changes: 3 additions & 5 deletions App/App_macOS.swift
View file
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ struct Kiwix: App {
var body: some Scene {
WindowGroup {
RootView()
.environment(\.managedObjectContext, Database.shared.container.viewContext)
.environment(\.managedObjectContext, Database.shared.viewContext)
.environmentObject(libraryRefreshViewModel)
}.commands {
SidebarCommands()
Expand Down Expand Up @@ -214,10 +214,8 @@ struct RootView: View {
DownloadService.shared.restartHeartbeatIfNeeded()
case let .custom(zimFileURL):
LibraryOperations.open(url: zimFileURL) {
Task {
await ZimMigration.forCustomApps()
navigation.currentItem = .reading
}
ZimMigration.forCustomApps()
navigation.currentItem = .reading
}
}
}
Expand Down
5 changes: 3 additions & 2 deletions App/SidebarViewController.swift
View file
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ class SidebarViewController: UICollectionViewController, NSFetchedResultsControl
}()
private let fetchedResultController = NSFetchedResultsController(
fetchRequest: Tab.fetchRequest(sortDescriptors: [NSSortDescriptor(key: "created", ascending: false)]),
managedObjectContext: Database.viewContext,
managedObjectContext: Database.shared.viewContext,
sectionNameKeyPath: nil,
cacheName: nil
)
Expand Down Expand Up @@ -187,7 +187,8 @@ class SidebarViewController: UICollectionViewController, NSFetchedResultsControl
// MARK: - Collection View Configuration

private func configureCell(cell: UICollectionViewListCell, indexPath: IndexPath, item: NavigationItem) {
if case let .tab(objectID) = item, let tab = try? Database.viewContext.existingObject(with: objectID) as? Tab {
if case let .tab(objectID) = item,
let tab = try? Database.shared.viewContext.existingObject(with: objectID) as? Tab {
var config = cell.defaultContentConfiguration()
config.text = tab.title ?? "common.tab.menu.new_tab".localized
if let zimFile = tab.zimFile, let category = Category(rawValue: zimFile.category) {
Expand Down
5 changes: 4 additions & 1 deletion App/SplitViewController.swift
View file
Original file line number Diff line number Diff line change
Expand Up @@ -111,7 +111,10 @@ final class SplitViewController: UISplitViewController {
let controller = UIHostingController(rootView: ZimFilesCategories(dismiss: nil))
setViewController(UINavigationController(rootViewController: controller), for: .secondary)
case .downloads:
let controller = UIHostingController(rootView: ZimFilesDownloads(dismiss: nil))
let controller = UIHostingController(
rootView: ZimFilesDownloads(dismiss: nil)
.environment(\.managedObjectContext, Database.shared.viewContext)
)
setViewController(UINavigationController(rootViewController: controller), for: .secondary)
case .new:
let controller = UIHostingController(rootView: ZimFilesNew(dismiss: nil))
Expand Down
72 changes: 37 additions & 35 deletions Model/DownloadService.swift
View file
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ private final class DownloadProgress {
final class DownloadService: NSObject, URLSessionDelegate, URLSessionTaskDelegate, URLSessionDownloadDelegate {
static let shared = DownloadService()

private let queue = DispatchQueue(label: "downloads")
private let queue = DispatchQueue(label: "downloads", qos: .background)
private let progress = DownloadProgress()
@MainActor private var heartbeat: Timer?
var backgroundCompletionHandler: (() -> Void)?
Expand All @@ -74,40 +74,38 @@ final class DownloadService: NSObject, URLSessionDelegate, URLSessionTaskDelegat
let zimFileID = UUID(uuidString: taskDescription) else { return }
self.progress.updateFor(uuid: zimFileID, totalBytes: task.countOfBytesReceived)
}
self.startHeartbeat()
Task { @MainActor in
self.startHeartbeat()
}
}
}

/// Start heartbeat, which will update database every 0.25 second
private func startHeartbeat() {
DispatchQueue.main.async {
guard self.heartbeat == nil else { return }
self.heartbeat = Timer.scheduledTimer(withTimeInterval: 0.25, repeats: true) { [weak self] _ in
Database.shared.container.performBackgroundTask { [weak self] context in
context.mergePolicy = NSMergePolicy.mergeByPropertyObjectTrump
if let progressValues = self?.progress.values() {
for (zimFileID, downloadedBytes) in progressValues {
let predicate = NSPredicate(format: "fileID == %@", zimFileID as CVarArg)
let request = DownloadTask.fetchRequest(predicate: predicate)
guard let downloadTask = try? context.fetch(request).first else { return }
downloadTask.downloadedBytes = downloadedBytes
}
try? context.save()
@MainActor private func startHeartbeat() {
guard self.heartbeat == nil else { return }
self.heartbeat = Timer.scheduledTimer(withTimeInterval: 0.25, repeats: true) { [weak self] _ in
Database.shared.performBackgroundTask { [weak self] context in
context.mergePolicy = NSMergePolicy.mergeByPropertyObjectTrump
if let progressValues = self?.progress.values() {
for (zimFileID, downloadedBytes) in progressValues {
let predicate = NSPredicate(format: "fileID == %@", zimFileID as CVarArg)
let request = DownloadTask.fetchRequest(predicate: predicate)
guard let downloadTask = try? context.fetch(request).first else { return }
downloadTask.downloadedBytes = downloadedBytes
}
try? context.save()
}
}
os_log("Heartbeat started.", log: Log.DownloadService, type: .info)
}
os_log("Heartbeat started.", log: Log.DownloadService, type: .info)
}

/// Stop heartbeat, which stops periodical database update
private func stopHeartbeat() {
DispatchQueue.main.async {
guard self.heartbeat != nil else { return }
self.heartbeat?.invalidate()
self.heartbeat = nil
os_log("Heartbeat stopped.", log: Log.DownloadService, type: .info)
}
@MainActor private func stopHeartbeat() {
guard self.heartbeat != nil else { return }
self.heartbeat?.invalidate()
self.heartbeat = nil
os_log("Heartbeat stopped.", log: Log.DownloadService, type: .info)
}

// MARK: - Download Actions
Expand All @@ -116,9 +114,9 @@ final class DownloadService: NSObject, URLSessionDelegate, URLSessionTaskDelegat
/// - Parameters:
/// - zimFile: the zim file to download
/// - allowsCellularAccess: if using cellular data is allowed
func start(zimFileID: UUID, allowsCellularAccess: Bool) {
@MainActor func start(zimFileID: UUID, allowsCellularAccess: Bool) {
requestNotificationAuthorization()
Database.shared.container.performBackgroundTask { context in
Database.shared.performBackgroundTask { context in
context.mergePolicy = NSMergePolicy.mergeByPropertyObjectTrump
let fetchRequest = ZimFile.fetchRequest(fileID: zimFileID)
guard let zimFile = try? context.fetch(fetchRequest).first,
Expand Down Expand Up @@ -160,7 +158,7 @@ final class DownloadService: NSObject, URLSessionDelegate, URLSessionTaskDelegat
session.getTasksWithCompletionHandler { _, _, downloadTasks in
guard let task = downloadTasks.filter({ $0.taskDescription == zimFileID.uuidString }).first else { return }
task.cancel { resumeData in
Database.shared.container.performBackgroundTask { context in
Database.shared.performBackgroundTask { context in
context.mergePolicy = NSMergePolicy.mergeByPropertyObjectTrump
let request = DownloadTask.fetchRequest(fileID: zimFileID)
guard let downloadTask = try? context.fetch(request).first else { return }
Expand All @@ -173,9 +171,9 @@ final class DownloadService: NSObject, URLSessionDelegate, URLSessionTaskDelegat

/// Resume a zim file download task and start heartbeat
/// - Parameter zimFileID: identifier of the zim file
func resume(zimFileID: UUID) {
@MainActor func resume(zimFileID: UUID) {
requestNotificationAuthorization()
Database.shared.container.performBackgroundTask { context in
Database.shared.performBackgroundTask { context in
context.mergePolicy = NSMergePolicy.mergeByPropertyObjectTrump

let request = DownloadTask.fetchRequest(fileID: zimFileID)
Expand All @@ -190,14 +188,16 @@ final class DownloadService: NSObject, URLSessionDelegate, URLSessionTaskDelegat
downloadTask.resumeData = nil
try? context.save()

self.startHeartbeat()
Task { @MainActor in
self.startHeartbeat()
}
}
}

// MARK: - Database

private func deleteDownloadTask(zimFileID: UUID) {
Database.shared.container.performBackgroundTask { context in
Database.shared.performBackgroundTask { context in
context.mergePolicy = NSMergePolicy.mergeByPropertyObjectTrump
do {
let request = DownloadTask.fetchRequest(fileID: zimFileID)
Expand All @@ -217,15 +217,15 @@ final class DownloadService: NSObject, URLSessionDelegate, URLSessionTaskDelegat

// MARK: - Notification

private func requestNotificationAuthorization() {
@MainActor private func requestNotificationAuthorization() {
UNUserNotificationCenter.current().requestAuthorization(options: [.alert, .sound]) { _, _ in }
}

private func scheduleDownloadCompleteNotification(zimFileID: UUID) {
let center = UNUserNotificationCenter.current()
center.getNotificationSettings { settings in
guard settings.authorizationStatus != .denied else { return }
Database.shared.container.performBackgroundTask { context in
Database.shared.performBackgroundTask { context in
// configure notification content
let content = UNMutableNotificationContent()
content.title = "download_service.complete.title".localized
Expand All @@ -248,7 +248,9 @@ final class DownloadService: NSObject, URLSessionDelegate, URLSessionTaskDelegat
let zimFileID = UUID(uuidString: taskDescription) else { return }
progress.resetFor(uuid: zimFileID)
if progress.isEmpty() {
stopHeartbeat()
Task { @MainActor in
stopHeartbeat()
}
}

// download finished successfully if there's no error
Expand All @@ -268,7 +270,7 @@ final class DownloadService: NSObject, URLSessionDelegate, URLSessionTaskDelegat
Note: The result data equality check is used as a trick to distinguish user pausing the download task vs
failure. When pausing, the same resume data would have already been saved when the delegate is called.
*/
Database.shared.container.performBackgroundTask { context in
Database.shared.performBackgroundTask { context in
context.mergePolicy = NSMergePolicy.mergeByPropertyObjectTrump
let request = DownloadTask.fetchRequest(fileID: zimFileID)
let resumeData = error.userInfo[NSURLSessionDownloadTaskResumeData] as? Data
Expand Down
98 changes: 24 additions & 74 deletions SwiftUI/Model/Database.swift
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,47 +18,36 @@ import os

final class Database {
static let shared = Database()
private var notificationToken: NSObjectProtocol?
private var token: NSPersistentHistoryToken?
private var tokenURL = NSPersistentContainer.defaultDirectoryURL().appendingPathComponent("token.data")
private let container: NSPersistentContainer
private let backgroundContext: NSManagedObjectContext
private let backgroundQueue = DispatchQueue(label: "database.background.queue",
qos: .utility)

private init() {
// due to objc++ interop, only the older notification value is working for downloads:
// https://developer.apple.com/documentation/coredata/nspersistentstoreremotechangenotification?language=objc
let storeChange: NSNotification.Name = .init(rawValue: "NSPersistentStoreRemoteChangeNotification")
notificationToken = NotificationCenter.default.addObserver(
forName: storeChange, object: nil, queue: nil) { _ in
try? self.mergeChanges()
}
token = {
guard let data = UserDefaults.standard.data(forKey: "PersistentHistoryToken") else { return nil }
return try? NSKeyedUnarchiver.unarchivedObject(ofClass: NSPersistentHistoryToken.self, from: data)
}()
container = Self.createContainer()
backgroundContext = container.newBackgroundContext()
backgroundContext.persistentStoreCoordinator = container.persistentStoreCoordinator
backgroundContext.automaticallyMergesChangesFromParent = true
backgroundContext.mergePolicy = NSMergePolicy.mergeByPropertyObjectTrump
backgroundContext.undoManager = nil
backgroundContext.shouldDeleteInaccessibleFaults = true
}

deinit {
if let token = notificationToken {
NotificationCenter.default.removeObserver(token)
}
var viewContext: NSManagedObjectContext {
container.viewContext
}

static var viewContext: NSManagedObjectContext {
Database.shared.container.viewContext
}

static func performBackgroundTask(_ block: @escaping (NSManagedObjectContext) -> Void) {
Database.shared.container.performBackgroundTask(block)
}

static func performBackgroundTask<T>(_ block: @escaping (NSManagedObjectContext) throws -> T) async rethrows -> T {
try await Database.shared.container.performBackgroundTask(block)
func performBackgroundTask(_ block: @escaping (NSManagedObjectContext) -> Void) {
backgroundQueue.sync { [self] in
backgroundContext.perform { [self] in
block(backgroundContext)
}
}
}

/// A persistent container to set up the Core Data stack.
lazy var container: NSPersistentContainer = {
/// - Tag: persistentContainer
private static func createContainer() -> NSPersistentContainer {
let container = NSPersistentContainer(name: "DataModel")

guard let description = container.persistentStoreDescriptions.first else {
fatalError("Failed to retrieve a persistent store description.")
}
Expand All @@ -75,27 +64,25 @@ final class Database {
}
}

// This sample refreshes UI by consuming store changes via persistent history tracking.
/// - Tag: viewContextMergeParentChanges
container.viewContext.automaticallyMergesChangesFromParent = false
container.viewContext.automaticallyMergesChangesFromParent = true
container.viewContext.name = "viewContext"
/// - Tag: viewContextMergePolicy
container.viewContext.mergePolicy = NSMergeByPropertyObjectTrumpMergePolicy
container.viewContext.undoManager = nil
container.viewContext.shouldDeleteInaccessibleFaults = true
return container
}()
}

/// Save image data to zim files.
func saveImageData(url: URL, completion: @escaping (Data) -> Void) {
URLSession.shared.dataTask(with: url) { data, response, _ in
URLSession.shared.dataTask(with: url) { [self] data, response, _ in
guard let response = response as? HTTPURLResponse,
response.statusCode == 200,
let mimeType = response.mimeType,
mimeType.contains("image"),
let data = data else { return }
let context = self.container.newBackgroundContext()
context.perform {
performBackgroundTask { [data] context in
let predicate = NSPredicate(format: "faviconURL == %@", url as CVarArg)
let request = ZimFile.fetchRequest(predicate: predicate)
guard let zimFile = try? context.fetch(request).first else { return }
Expand All @@ -105,41 +92,4 @@ final class Database {
completion(data)
}.resume()
}

/// Merge changes performed on batch requests to view context.
private func mergeChanges() throws {
let context = container.newBackgroundContext()
context.mergePolicy = NSMergePolicy.mergeByPropertyObjectTrump
context.undoManager = nil
context.perform {
// fetch and merge changes
let fetchRequest = NSPersistentHistoryChangeRequest.fetchHistory(after: self.token)
guard let result = try? context.execute(fetchRequest) as? NSPersistentHistoryResult else {
os_log("no persistent history found after token: \(self.token)")
self.token = nil
return
}
guard let transactions = result.result as? [NSPersistentHistoryTransaction] else {
os_log("no transactions in persistent history found after token: \(self.token)")
self.token = nil
return
}
self.container.viewContext.performAndWait {
transactions.forEach { transaction in
self.container.viewContext.mergeChanges(fromContextDidSave: transaction.objectIDNotification())
self.token = transaction.token
}
}

// update token
guard let token = transactions.last?.token else { return }
let data = try? NSKeyedArchiver.archivedData(withRootObject: token, requiringSecureCoding: true)
UserDefaults.standard.set(data, forKey: "PersistentHistoryToken")

// purge history
let sevenDaysAgo = Date(timeIntervalSinceNow: -3600 * 24 * 7)
let purgeRequest = NSPersistentHistoryChangeRequest.deleteHistory(before: sevenDaysAgo)
_ = try? context.execute(purgeRequest)
}
}
}
Loading

0 comments on commit 2b1f49d

Please sign in to comment.