Fix memory leaks in the navigators and publication server (#49)

Author: mickael-menu

CHANGELOG.md

@@ -4,7 +4,18 @@ All notable changes to this project will be documented in this file. Take a look
 
 **Warning:** Features marked as *alpha* may change or be removed in a future release without notice. Use with caution.
 
-<!--## [Unreleased]-->
+## [Unreleased]
+
+### Fixed
+
+#### Navigator
+
+* Fixed memory leaks in the EPUB and PDF navigators.
+
+#### Streamer
+
+* Fixed memory leak in the `PublicationServer`.
+
 
 ## [2.3.0]
 

Sources/Navigator/PDF/PDFTapGestureController.swift

@@ -22,7 +22,7 @@ final class PDFTapGestureController: NSObject {
     private let tapAction: TargetAction
     private var tapRecognizer: UITapGestureRecognizer!
 
-    init(pdfView: PDFView, target: Any, action: Selector) {
+    init(pdfView: PDFView, target: AnyObject, action: Selector) {
         assert(pdfView.superview != nil, "The PDFView must be in the view hierarchy")
 
         self.pdfView = pdfView

Sources/Navigator/Toolkit/PaginationView.swift

@@ -211,9 +211,11 @@ final class PaginationView: UIView, Loggable {
             }
         }
 
-        loadNextPage {
-            self.delegate?.paginationViewDidUpdateViews(self)
-            completion()
+        loadNextPage { [weak self] in
+            if let self = self {
+                self.delegate?.paginationViewDidUpdateViews(self)
+                completion()
+            }
         }
     }
 

Sources/Navigator/Toolkit/TargetAction.swift

@@ -15,16 +15,18 @@ import UIKit
 /// Represents a couple (`target`, `action`) which can be invoked from a `sender`.
 final class TargetAction {
 
-    private let target: Any
+    private weak var target: AnyObject?
     private let action: Selector
 
-    init(target: Any, action: Selector) {
+    init(target: AnyObject, action: Selector) {
         self.target = target
         self.action = action
     }
 
     func invoke(from sender: Any?) {
-        UIApplication.shared.sendAction(action, to: target, from: sender, for: nil)
+        if let target = target {
+            UIApplication.shared.sendAction(action, to: target, from: sender, for: nil)
+        }
     }
 
 }

Sources/Streamer/Server/PublicationServer.swift

@@ -211,52 +211,50 @@ public class PublicationServer: ResourcesServer {
     }
 
     fileprivate func addResourcesHandler(for publication: Publication, at endpoint: String) throws {
-        /// Webserver HTTP GET ressources request handler.
-        func resourcesHandler(request: GCDWebServerRequest?) -> GCDWebServerResponse? {
-            guard let request = request else {
-                log(.error, "The request received is nil.")
-                return GCDWebServerErrorResponse(statusCode: 500)
-            }
-            
-            // Remove the prefix from the URI.
-            var href = request.url.absoluteString
-            if let range = href.range(of: endpoint) {
-                href = String(href[range.upperBound...])
-                href = href.removingPercentEncoding ?? href
-            }
-
-            let resource = publication.get(href.removingPercentEncoding ?? href)
-            let range = request.hasByteRange() ? request.byteRange : nil
-            return WebServerResourceResponse(
-                resource: resource,
-                range: range,
-                contentType: resource.link.type ?? MediaType.binary.string
-            )
-        }
-        
         webServer.addHandler(
             forMethod: "GET",
             pathRegex: "/\(endpoint)/.*",
             request: GCDWebServerRequest.self,
-            processBlock: resourcesHandler
+            processBlock: { [weak self, weak publication] request in
+                guard let publication = publication else {
+                    self?.log(.error, "The publication was deallocated.")
+                    return GCDWebServerErrorResponse(statusCode: 500)
+                }
+                
+                // Remove the prefix from the URI.
+                var href = request.url.absoluteString
+                if let range = href.range(of: endpoint) {
+                    href = String(href[range.upperBound...])
+                    href = href.removingPercentEncoding ?? href
+                }
+
+                let resource = publication.get(href.removingPercentEncoding ?? href)
+                let range = request.hasByteRange() ? request.byteRange : nil
+                return WebServerResourceResponse(
+                    resource: resource,
+                    range: range,
+                    contentType: resource.link.type ?? MediaType.binary.string
+                )
+            }
         )
     }
 
     fileprivate func addManifestHandler(for publication: Publication, at endpoint: String) {
-        /// The webserver handler to process the HTTP GET
-        func manifestHandler(request: GCDWebServerRequest?) -> GCDWebServerResponse? {
-            guard let manifestData = publication.jsonManifest?.data(using: .utf8) else {
-                return GCDWebServerResponse(statusCode: 404)
-            }
-            let type = "\(MediaType.readiumWebPubManifest.string); charset=utf-8"
-            return GCDWebServerDataResponse(data: manifestData, contentType: type)
-        }
-        
         webServer.addHandler(
             forMethod: "GET",
             pathRegex: "/\(endpoint)/manifest.json",
             request: GCDWebServerRequest.self,
-            processBlock: manifestHandler
+            processBlock: { [weak self, weak publication] request in
+                guard let publication = publication else {
+                    self?.log(.error, "The publication was deallocated.")
+                    return GCDWebServerErrorResponse(statusCode: 500)
+                }
+                guard let manifestData = publication.jsonManifest?.data(using: .utf8) else {
+                    return GCDWebServerResponse(statusCode: 404)
+                }
+                let type = "\(MediaType.readiumWebPubManifest.string); charset=utf-8"
+                return GCDWebServerDataResponse(data: manifestData, contentType: type)
+            }
         )
     }
 

TestApp/Sources/Reader/Common/ReaderViewController.swift

@@ -101,7 +101,7 @@ class ReaderViewController: UIViewController, Loggable {
         navigator.didMove(toParent: self)
 
         stackView.addArrangedSubview(accessibilityToolbar)
-        
+
         positionLabel.translatesAutoresizingMaskIntoConstraints = false
         positionLabel.font = .systemFont(ofSize: 12)
         positionLabel.textColor = .darkGray
@@ -181,9 +181,9 @@ class ReaderViewController: UIViewController, Loggable {
         }
 
         locatorPublisher
-            .sink(receiveValue: { locator in
-                self.navigator.go(to: locator, animated: false) {
-                    self.dismiss(animated: true)
+            .sink(receiveValue: { [weak self] locator in
+                self?.navigator.go(to: locator, animated: false) {
+                    self?.dismiss(animated: true)
                 }
             })
             .store(in: &subscriptions)
@@ -218,9 +218,9 @@ class ReaderViewController: UIViewController, Loggable {
     @objc func showSearchUI() {
         if searchViewModel == nil {
             searchViewModel = SearchViewModel(publication: publication)
-            searchViewModel?.$selectedLocator.sink(receiveValue: { locator in
-                self.searchViewController?.dismiss(animated: true, completion: nil)
-                if let locator = locator {
+            searchViewModel?.$selectedLocator.sink(receiveValue: { [weak self] locator in
+                self?.searchViewController?.dismiss(animated: true, completion: nil)
+                if let self = self, let locator = locator {
                     self.navigator.go(to: locator, animated: true) {
                         if let decorator = self.navigator as? DecorableNavigator {
                             let decoration = Decoration(id: "selectedSearchResult", locator: locator, style: Decoration.Style.highlight(tint: .yellow, isActive: false))
@@ -244,8 +244,8 @@ class ReaderViewController: UIViewController, Loggable {
         if highlights == nil { return }
 
         if let decorator = self.navigator as? DecorableNavigator {
-            decorator.observeDecorationInteractions(inGroup: highlightDecorationGroup) { event in
-                self.activateDecoration(event)
+            decorator.observeDecorationInteractions(inGroup: highlightDecorationGroup) { [weak self] event in
+                self?.activateDecoration(event)
             }
         }
     }
@@ -255,8 +255,8 @@ class ReaderViewController: UIViewController, Loggable {
 
         highlights.all(for: bookId)
             .assertNoFailure()
-            .sink { highlights in
-                if let decorator = self.navigator as? DecorableNavigator {
+            .sink { [weak self] highlights in
+                if let self = self, let decorator = self.navigator as? DecorableNavigator {
                     let decorations = highlights.map { Decoration(id: $0.id, locator: $0.locator, style: .highlight(tint: $0.color.uiColor, isActive: false)) }
                     decorator.apply(decorations: decorations, in: self.highlightDecorationGroup)
                 }
@@ -283,17 +283,17 @@ class ReaderViewController: UIViewController, Loggable {
                                             systemFontSize: 20,
                                             colorScheme: colorScheme)
 
-        menuView.selectedColorPublisher.sink { color in
-            self.currentHighlightCancellable?.cancel()
-            self.updateHighlight(event.decoration.id, withColor: color)
-            self.highlightContextMenu?.dismiss(animated: true, completion: nil)
+        menuView.selectedColorPublisher.sink { [weak self] color in
+            self?.currentHighlightCancellable?.cancel()
+            self?.updateHighlight(event.decoration.id, withColor: color)
+            self?.highlightContextMenu?.dismiss(animated: true, completion: nil)
         }
         .store(in: &subscriptions)
 
-        menuView.selectedDeletePublisher.sink { _ in
-            self.currentHighlightCancellable?.cancel()
-            self.deleteHighlight(event.decoration.id)
-            self.highlightContextMenu?.dismiss(animated: true, completion: nil)
+        menuView.selectedDeletePublisher.sink { [weak self] _ in
+            self?.currentHighlightCancellable?.cancel()
+            self?.deleteHighlight(event.decoration.id)
+            self?.highlightContextMenu?.dismiss(animated: true, completion: nil)
         }
         .store(in: &subscriptions)
 
@@ -385,8 +385,8 @@ extension ReaderViewController: NavigatorDelegate {
 
     func navigator(_ navigator: Navigator, locationDidChange locator: Locator) {
         books.saveProgress(for: bookId, locator: locator)
-            .sink { completion in
-                if case .failure(let error) = completion {
+            .sink { [weak self] completion in
+                if let self = self, case .failure(let error) = completion {
                     self.moduleDelegate?.presentError(error, from: self)
                 }
             } receiveValue: { _ in }

TestApp/Sources/Reader/PDF/PDFViewController.swift

@@ -26,10 +26,6 @@ final class PDFViewController: ReaderViewController {
         navigator.delegate = self
     }
 
-    override func viewDidLoad() {
-        super.viewDidLoad()
-    }
-    
     override var currentBookmark: Bookmark? {
         guard let locator = navigator.currentLocation else {
             return nil