Bugfix FXIOS-9159 - Fix Firefox Suggest operations to avoid blocking. (…

…#20401)

* Fix Firefox Suggest operations to avoid blocking.

This commit:

* Refactors `RustFirefoxSuggest` to use a pair of serial
  dispatch queues to execute blocking read and write operations on
  the underlying `SuggestStore`. Previously, these operations would
  block, because calling a blocking function from an actor method
  (whether isolated or not) does _not_ automatically execute that
  function on a "global concurrent executor"; it can starve the
  calling thread.
* Adopts the new `SuggestStore.interrupt(kind:)` API to implement
  `RustFirefoxSuggest.interruptEverything()`, which interrupts all
  ongoing operations.
* Adds an expiration handler for the background ingestion task that
  calls `interruptEverything()`.

* Make `RustFirefoxSuggest` a class, not an actor.

Isolated methods on an actor are serialized, to protect the actor's
mutable state. This is undesired for `RustFirefoxSuggest`, because
it doesn't have any mutable state, and uses GCD queues to run read and
write operations concurrently and off-main-thread. Using an actor here
would serialize these operations, because isolated methods can't run
concurrently.

Instead, we can make `RustFirefoxSuggest` a regular class, and use
`withCheckedThrowingContinuation` to expose an `await`-compatible API.

firefox-ios/Client/Application/BackgroundFirefoxSuggestIngestUtility.swift

@@ -13,11 +13,11 @@ import Storage
 class BackgroundFirefoxSuggestIngestUtility: BackgroundUtilityProtocol, FeatureFlaggable {
     static let taskIdentifier = "org.mozilla.ios.firefox.suggest.ingest"
 
-    let firefoxSuggest: RustFirefoxSuggestActor
+    let firefoxSuggest: RustFirefoxSuggestProtocol
     let logger: Logger
     private var didRegisterTaskHandler = false
 
-    init(firefoxSuggest: RustFirefoxSuggestActor, logger: Logger = DefaultLogger.shared) {
+    init(firefoxSuggest: RustFirefoxSuggestProtocol, logger: Logger = DefaultLogger.shared) {
         self.firefoxSuggest = firefoxSuggest
         self.logger = logger
 
@@ -72,6 +72,12 @@ class BackgroundFirefoxSuggestIngestUtility: BackgroundUtilityProtocol, FeatureF
     private func setUp() {
         guard featureFlags.isFeatureEnabled(.firefoxSuggestFeature, checking: .buildAndUser) else { return }
         BGTaskScheduler.shared.register(forTaskWithIdentifier: Self.taskIdentifier, using: nil) { task in
+            task.expirationHandler = {
+                // Interrupt all ongoing storage operations if our
+                // background time is about to expire.
+                self.firefoxSuggest.interruptEverything()
+                task.setTaskCompleted(success: false)
+            }
             Task {
                 let success = await self.ingest()
                 if !success {

firefox-ios/Providers/Profile.swift

@@ -82,7 +82,7 @@ protocol Profile: AnyObject {
     var files: FileAccessor { get }
     var pinnedSites: PinnedSites { get }
     var logins: RustLogins { get }
-    var firefoxSuggest: RustFirefoxSuggestActor? { get }
+    var firefoxSuggest: RustFirefoxSuggestProtocol? { get }
     var certStore: CertStore { get }
     var recentlyClosedTabs: ClosedTabsStore { get }
 
@@ -615,7 +615,7 @@ open class BrowserProfile: Profile {
         return RustLogins(databasePath: databasePath)
     }()
 
-    lazy var firefoxSuggest: RustFirefoxSuggestActor? = {
+    lazy var firefoxSuggest: RustFirefoxSuggestProtocol? = {
         do {
             let cacheFileURL = try FileManager.default.url(
                 for: .cachesDirectory,

firefox-ios/Storage/Rust/RustFirefoxSuggest.swift

@@ -5,7 +5,7 @@
 import Foundation
 import MozillaAppServices
 
-public protocol RustFirefoxSuggestActor: Actor {
+public protocol RustFirefoxSuggestProtocol {
     /// Downloads and stores new Firefox Suggest suggestions.
     func ingest() async throws
 
@@ -17,14 +17,22 @@ public protocol RustFirefoxSuggestActor: Actor {
     ) async throws -> [RustFirefoxSuggestion]
 
     /// Interrupts any ongoing queries for suggestions.
-    nonisolated func interruptReader()
+    func interruptReader()
+
+    /// Interrupts all ongoing operations.
+    func interruptEverything()
 }
 
-/// An actor that wraps the synchronous Rust `SuggestStore` binding to execute
-/// blocking operations on the default global concurrent executor.
-public actor RustFirefoxSuggest: RustFirefoxSuggestActor {
+/// Wraps the synchronous Rust `SuggestStore` binding to execute
+/// blocking operations on a dispatch queue.
+public class RustFirefoxSuggest: RustFirefoxSuggestProtocol {
     private let store: SuggestStore
 
+    // Using a pair of serial queues lets read and write operations run
+    // without blocking one another.
+    private let writerQueue = DispatchQueue(label: "RustFirefoxSuggest.writer")
+    private let readerQueue = DispatchQueue(label: "RustFirefoxSuggest.reader")
+
     public init(dataPath: String, cachePath: String, remoteSettingsConfig: RemoteSettingsConfig? = nil) throws {
         var builder = SuggestStoreBuilder()
             .dataPath(path: dataPath)
@@ -42,22 +50,44 @@ public actor RustFirefoxSuggest: RustFirefoxSuggestActor {
         // downloading new suggestions. This is safe to call multiple times.
         Viaduct.shared.useReqwestBackend()
 
-        try store.ingest(constraints: SuggestIngestionConstraints())
+        try await withCheckedThrowingContinuation { continuation in
+            writerQueue.async(qos: .utility) {
+                do {
+                    try self.store.ingest(constraints: SuggestIngestionConstraints())
+                    continuation.resume()
+                } catch {
+                    continuation.resume(throwing: error)
+                }
+            }
+        }
     }
 
     public func query(
         _ keyword: String,
         providers: [SuggestionProvider],
         limit: Int32
     ) async throws -> [RustFirefoxSuggestion] {
-        return try store.query(query: SuggestionQuery(
-            keyword: keyword,
-            providers: providers,
-            limit: limit
-        )).compactMap(RustFirefoxSuggestion.init)
+        return try await withCheckedThrowingContinuation { continuation in
+            readerQueue.async(qos: .userInitiated) {
+                do {
+                    let suggestions = try self.store.query(query: SuggestionQuery(
+                        keyword: keyword,
+                        providers: providers,
+                        limit: limit
+                    )).compactMap(RustFirefoxSuggestion.init) ?? []
+                    continuation.resume(returning: suggestions)
+                } catch {
+                    continuation.resume(throwing: error)
+                }
+            }
+        }
     }
 
-    public nonisolated func interruptReader() {
+    public func interruptReader() {
         store.interrupt()
     }
+
+    public func interruptEverything() {
+        store.interrupt(kind: .readWrite)
+    }
 }

firefox-ios/firefox-ios-tests/Tests/ClientTests/Mocks/MockProfile.swift

@@ -108,14 +108,14 @@ open class MockProfile: Client.Profile {
 
     public var files: FileAccessor
     public var syncManager: ClientSyncManager!
-    public var firefoxSuggest: RustFirefoxSuggestActor?
+    public var firefoxSuggest: RustFirefoxSuggestProtocol?
 
     fileprivate let name: String = "mockaccount"
 
     private let directory: String
     private let databasePrefix: String
 
-    init(databasePrefix: String = "mock", firefoxSuggest: RustFirefoxSuggestActor? = nil) {
+    init(databasePrefix: String = "mock", firefoxSuggest: RustFirefoxSuggestProtocol? = nil) {
         files = MockFiles()
         syncManager = ClientSyncManagerSpy()
         self.databasePrefix = databasePrefix

firefox-ios/firefox-ios-tests/Tests/ClientTests/SearchViewControllerTests.swift

@@ -9,7 +9,7 @@ import Shared
 
 @testable import Client
 
-actor MockRustFirefoxSuggest: RustFirefoxSuggestActor {
+class MockRustFirefoxSuggest: RustFirefoxSuggestProtocol {
     func ingest() async throws {
     }
     func query(
@@ -36,7 +36,9 @@ actor MockRustFirefoxSuggest: RustFirefoxSuggestActor {
         }
         return suggestions
     }
-    nonisolated func interruptReader() {
+    func interruptReader() {
+    }
+    func interruptEverything() {
     }
 }