Skip to content

Commit

Permalink
Fix Core Data deadlocks (#989)
Browse files Browse the repository at this point in the history
* Utilize the RunLoop as a way to avoid database deadlocks.

* Fix formatting in DateExtensionTests.
  • Loading branch information
samsymons authored Nov 17, 2021
1 parent 6096e00 commit cb4393c
Show file tree
Hide file tree
Showing 5 changed files with 243 additions and 10 deletions.
8 changes: 4 additions & 4 deletions Core/Database.swift
Original file line number Diff line number Diff line change
Expand Up @@ -30,8 +30,8 @@ public class Database {

public static let shared = Database()

private let semaphore = DispatchSemaphore(value: 0)
private let container: NSPersistentContainer
private let storeLoadedCondition = RunLoop.ResumeCondition()

public var isDatabaseFileInitialized: Bool {
var containerURL = DDGPersistentContainer.defaultDirectoryURL()
Expand Down Expand Up @@ -77,17 +77,17 @@ public class Database {
context.name = "Migration"
context.perform {
handler(context)
self.semaphore.signal()
self.storeLoadedCondition.resolve()
}
}
}

public func makeContext(concurrencyType: NSManagedObjectContextConcurrencyType, name: String? = nil) -> NSManagedObjectContext {
semaphore.wait()
RunLoop.current.run(until: storeLoadedCondition)

let context = NSManagedObjectContext(concurrencyType: concurrencyType)
context.persistentStoreCoordinator = container.persistentStoreCoordinator
context.name = name
semaphore.signal()

return context
}
Expand Down
8 changes: 8 additions & 0 deletions DuckDuckGo.xcodeproj/project.pbxproj
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,8 @@
4B62C4BA25B930DD008912C6 /* AppConfigurationFetchTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 4B62C4B925B930DD008912C6 /* AppConfigurationFetchTests.swift */; };
4B75EA9226A266CB00018634 /* PrintingUserScript.swift in Sources */ = {isa = PBXBuildFile; fileRef = 4B75EA9126A266CB00018634 /* PrintingUserScript.swift */; };
4B82E98025B634B800656FE7 /* TrackerRadarKit in Frameworks */ = {isa = PBXBuildFile; productRef = 4B82E97F25B634B800656FE7 /* TrackerRadarKit */; };
4BC21A2B2723862E00229F0E /* RunLoopExtension.swift in Sources */ = {isa = PBXBuildFile; fileRef = 4BC21A29272384F500229F0E /* RunLoopExtension.swift */; };
4BC21A2F27238B7500229F0E /* RunLoopExtensionTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 4BC21A2C272388BD00229F0E /* RunLoopExtensionTests.swift */; };
4BDCECB92680333100F5C244 /* EmailWaitlistWebViewController.swift in Sources */ = {isa = PBXBuildFile; fileRef = 4BDCECB82680333100F5C244 /* EmailWaitlistWebViewController.swift */; };
4BE86C5D2633DBD8001C0E77 /* BrowserServicesKit in Frameworks */ = {isa = PBXBuildFile; productRef = 4BE86C5C2633DBD8001C0E77 /* BrowserServicesKit */; };
83004E802193BB8200DA013C /* WKNavigationExtension.swift in Sources */ = {isa = PBXBuildFile; fileRef = 83004E7F2193BB8200DA013C /* WKNavigationExtension.swift */; };
Expand Down Expand Up @@ -734,6 +736,8 @@
4B60ACA0252EC0B100E8D219 /* FullScreenVideoUserScript.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = FullScreenVideoUserScript.swift; sourceTree = "<group>"; };
4B62C4B925B930DD008912C6 /* AppConfigurationFetchTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = AppConfigurationFetchTests.swift; sourceTree = "<group>"; };
4B75EA9126A266CB00018634 /* PrintingUserScript.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = PrintingUserScript.swift; sourceTree = "<group>"; };
4BC21A29272384F500229F0E /* RunLoopExtension.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; name = RunLoopExtension.swift; path = ../DuckDuckGo/RunLoopExtension.swift; sourceTree = "<group>"; };
4BC21A2C272388BD00229F0E /* RunLoopExtensionTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = RunLoopExtensionTests.swift; sourceTree = "<group>"; };
4BDCECB82680333100F5C244 /* EmailWaitlistWebViewController.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = EmailWaitlistWebViewController.swift; sourceTree = "<group>"; };
6FB030C7234331B400A10DB9 /* Configuration.xcconfig */ = {isa = PBXFileReference; lastKnownFileType = text.xcconfig; name = Configuration.xcconfig; path = Configuration/Configuration.xcconfig; sourceTree = "<group>"; };
83004E7F2193BB8200DA013C /* WKNavigationExtension.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = WKNavigationExtension.swift; sourceTree = "<group>"; };
Expand Down Expand Up @@ -3256,6 +3260,7 @@
98982B3322F8D8E400578AC9 /* Debounce.swift */,
1CB7B82023CEA1F800AA24EA /* DateExtension.swift */,
85449EFA23FDA0BC00512AAF /* UserDefaultsPropertyWrapper.swift */,
4BC21A29272384F500229F0E /* RunLoopExtension.swift */,
);
name = Utilities;
sourceTree = "<group>";
Expand Down Expand Up @@ -3470,6 +3475,7 @@
83C05D04212C74600068712A /* BloomFilterWrapperTest.swift */,
8341D804212D5DFB000514C2 /* HashExtensionTest.swift */,
1CB7B82223CEA28300AA24EA /* DateExtensionTests.swift */,
4BC21A2C272388BD00229F0E /* RunLoopExtensionTests.swift */,
);
name = Utilities;
sourceTree = "<group>";
Expand Down Expand Up @@ -4702,6 +4708,7 @@
8528AE7E212EF5FF00D0BD74 /* AppRatingPromptTests.swift in Sources */,
85C9E5F521B422D300460EBC /* BookmarksManagerTests.swift in Sources */,
981FED692201FE69008488D7 /* AutoClearSettingsScreenTests.swift in Sources */,
4BC21A2F27238B7500229F0E /* RunLoopExtensionTests.swift in Sources */,
851B1283221FE65E004781BC /* ImproveOnboardingExperiment1Tests.swift in Sources */,
F14513591F4664E900710C46 /* SiteRatingTests.swift in Sources */,
F194FAFB1F14E622009B4DF8 /* UIFontExtensionTests.swift in Sources */,
Expand Down Expand Up @@ -4833,6 +4840,7 @@
85C271DF1FD044D7007216B4 /* HTTPSUpgradeStore.swift in Sources */,
830381C01F850AAF00863075 /* WKWebViewConfigurationExtension.swift in Sources */,
85B718F51FD071E50031A14F /* HTTPSUpgrade.xcdatamodeld in Sources */,
4BC21A2B2723862E00229F0E /* RunLoopExtension.swift in Sources */,
85CA53AA24BB376800A6288C /* NotFoundCachingDownloader.swift in Sources */,
4B60ACA1252EC0B100E8D219 /* FullScreenVideoUserScript.swift in Sources */,
F1A886781F29394E0096251E /* WebCacheManager.swift in Sources */,
Expand Down
94 changes: 94 additions & 0 deletions DuckDuckGo/RunLoopExtension.swift
Original file line number Diff line number Diff line change
@@ -0,0 +1,94 @@
//
// RunLoopExtension.swift
// DuckDuckGo
//
// Copyright © 2021 DuckDuckGo. All rights reserved.
//
// Licensed under the Apache License, Version 2.0 (the "License");
// you may not use this file except in compliance with the License.
// You may obtain a copy of the License at
//
// http://www.apache.org/licenses/LICENSE-2.0
//
// Unless required by applicable law or agreed to in writing, software
// distributed under the License is distributed on an "AS IS" BASIS,
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
// See the License for the specific language governing permissions and
// limitations under the License.
//

import Foundation

extension RunLoop {

final class ResumeCondition {

private let lock = NSLock()
private var receivePorts = [Port]()

private var _isResolved = false
var isResolved: Bool {
lock.lock()
defer { lock.unlock() }
return _isResolved
}

init() {
}

convenience init(dispatchGroup: DispatchGroup) {
self.init()
dispatchGroup.notify(queue: .main) {
self.resolve()
}
}

func addPort(to runLoop: RunLoop, forMode mode: RunLoop.Mode) -> Port? {
lock.lock()
defer { lock.unlock() }
guard !_isResolved else { return nil }

let port = Port()
receivePorts.append(port)
runLoop.add(port, forMode: mode)

return port
}

func resolve(mode: RunLoop.Mode = .default) {
lock.lock()

assert(!_isResolved)
_isResolved = true

let ports = receivePorts

lock.unlock()

let sendPort = Port()
RunLoop.current.add(sendPort, forMode: mode)

// Send Wake message from current RunLoop port to each running RunLoop
for receivePort in ports {
receivePort.send(before: Date(), components: nil, from: sendPort, reserved: 0)
}

RunLoop.current.remove(sendPort, forMode: mode)
}

}

func run(mode: RunLoop.Mode = .default, until condition: ResumeCondition) {
// Add port to current RunLoop to receive Wake message
guard let port = condition.addPort(to: self, forMode: mode) else {
// already resolved
return
}

while !condition.isResolved {
self.run(mode: mode, before: .distantFuture)
}
self.remove(port, forMode: mode)
}

}
12 changes: 6 additions & 6 deletions DuckDuckGoTests/DateExtensionTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -19,24 +19,24 @@

import XCTest

class DateExtensionTests: XCTestCase {
class DateExtensionTests: XCTestCase {

func testWhenDatesAreSame() {
func testWhenDatesAreSame() {
let today: Date = Date()
let secondDate: Date = Date()
XCTAssertTrue(today.isSameDay(secondDate))
}

func testWhenDatesAreNotSame() {
func testWhenDatesAreNotSame() {
let today: Date = Date()
let secondDate: Date? = Calendar.current.date(byAdding: .day, value: -1, to: today)
XCTAssertFalse(today.isSameDay(secondDate))
}

func testWhenOtherDateIsNil() {
func testWhenOtherDateIsNil() {
let today: Date = Date()
let secondDate: Date? = nil
XCTAssertFalse(today.isSameDay(secondDate))
}

}
}
131 changes: 131 additions & 0 deletions DuckDuckGoTests/RunLoopExtensionTests.swift
Original file line number Diff line number Diff line change
@@ -0,0 +1,131 @@
//
// RunLoopExtensionTests.swift
// DuckDuckGo
//
// Copyright © 2021 DuckDuckGo. All rights reserved.
//
// Licensed under the Apache License, Version 2.0 (the "License");
// you may not use this file except in compliance with the License.
// You may obtain a copy of the License at
//
// http://www.apache.org/licenses/LICENSE-2.0
//
// Unless required by applicable law or agreed to in writing, software
// distributed under the License is distributed on an "AS IS" BASIS,
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
// See the License for the specific language governing permissions and
// limitations under the License.
//

import Foundation
import XCTest
@testable import Core

class RunLoopExtensionTests: XCTestCase {

func testWhenConditionResolvedThenNoWaitIsPerformed() {
let condition = RunLoop.ResumeCondition()
condition.resolve()

let e = expectation(description: "should execute after wait")
var isExecuted = false
RunLoop.current.perform {
isExecuted = true
e.fulfill()
}

RunLoop.current.run(until: condition)
XCTAssertFalse(isExecuted)

waitForExpectations(timeout: 1)
}

func testWhenConditionIsResolvedThenWaitIsFinished() {
let condition = RunLoop.ResumeCondition()

let e = expectation(description: "should execute")
RunLoop.current.perform {
condition.resolve()
e.fulfill()
}

RunLoop.current.run(until: condition)
waitForExpectations(timeout: 0)
}

func testWhenDispatchGroupIsEmptyThenNoWaitIsPerformed() {
let dispatchGroup = DispatchGroup()
let condition = RunLoop.ResumeCondition(dispatchGroup: dispatchGroup)

let e = expectation(description: "should execute")
RunLoop.current.perform {
e.fulfill()
}

RunLoop.current.run(until: condition)
waitForExpectations(timeout: 0)
}

func testWhenDispatchGroupIsCompleteThenWaitIsFinished() {
let dispatchGroup = DispatchGroup()
let condition = RunLoop.ResumeCondition(dispatchGroup: dispatchGroup)

let e = expectation(description: "should execute")
RunLoop.current.perform {
e.fulfill()
}

for _ in 0..<3 {
dispatchGroup.enter()
DispatchQueue.global().asyncAfter(deadline: .now() + 0.05) {
dispatchGroup.leave()
}
}

RunLoop.current.run(until: condition)
waitForExpectations(timeout: 0)
}

func testWhenNestedWaitIsCalledThenWaitIsPerformed() {
let condition = RunLoop.ResumeCondition()

let e = expectation(description: "should execute")
RunLoop.current.perform {
RunLoop.current.perform {
condition.resolve()
e.fulfill()
}
RunLoop.current.run(until: condition)
}

RunLoop.current.run(until: condition)
waitForExpectations(timeout: 0)
}

func testWhenResolveFromBackgroundThreadThenWaitIsFinished() {
let condition = RunLoop.ResumeCondition()

DispatchQueue.global().asyncAfter(deadline: .now() + 0.05) {
condition.resolve()
}

RunLoop.current.run(until: condition)
}

func testWhenWaitingInBackgroundThreadThenWaitIsFinishedWhenResolved() {
let condition = RunLoop.ResumeCondition()

let e = expectation(description: "should execute")
DispatchQueue.global().async {
RunLoop.current.run(until: condition)
e.fulfill()
}
DispatchQueue.global().asyncAfter(deadline: .now() + 0.05) {
condition.resolve()
}

RunLoop.current.run(until: condition)
waitForExpectations(timeout: 1)
}

}

0 comments on commit cb4393c

Please sign in to comment.