From 4b8f05b8df6b8091292fd18f15c1fe3ddb3c8487 Mon Sep 17 00:00:00 2001 From: Nico Verbruggen Date: Sat, 6 Dec 2025 11:38:15 +0100 Subject: [PATCH] =?UTF-8?q?=E2=99=BB=EF=B8=8F=20Add=20Suspendable=20to=20C?= =?UTF-8?q?onfigWatchManager?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- PHP Monitor.xcodeproj/project.pbxproj | 10 +++ phpmon/Common/PHP/PhpConfigurationFile.swift | 15 ++--- phpmon/Common/Protocols/Suspendable.swift | 58 ++++++++++++++++++ phpmon/Domain/Menu/MainMenu+Actions.swift | 56 +++++++++-------- phpmon/Domain/Presets/Preset.swift | 6 +- .../Domain/Watcher/ConfigWatchManager.swift | 49 ++++++++++++--- .../Domain/Watcher/HomebrewWatchManager.swift | 61 +++++++------------ .../Data/BytePhpPreference.swift | 12 ++-- .../Data/PhpPreference.swift | 4 +- 9 files changed, 180 insertions(+), 91 deletions(-) create mode 100644 phpmon/Common/Protocols/Suspendable.swift diff --git a/PHP Monitor.xcodeproj/project.pbxproj b/PHP Monitor.xcodeproj/project.pbxproj index 699b5439..6e24957d 100644 --- a/PHP Monitor.xcodeproj/project.pbxproj +++ b/PHP Monitor.xcodeproj/project.pbxproj @@ -31,6 +31,10 @@ 0329A9A42E92A69000A62A12 /* WarningManager+Evaluations.swift in Sources */ = {isa = PBXBuildFile; fileRef = 0329A9A22E92A68B00A62A12 /* WarningManager+Evaluations.swift */; }; 0329A9A52E92A69000A62A12 /* WarningManager+Evaluations.swift in Sources */ = {isa = PBXBuildFile; fileRef = 0329A9A22E92A68B00A62A12 /* WarningManager+Evaluations.swift */; }; 0329A9A62E92A69000A62A12 /* WarningManager+Evaluations.swift in Sources */ = {isa = PBXBuildFile; fileRef = 0329A9A22E92A68B00A62A12 /* WarningManager+Evaluations.swift */; }; + 032C7A022EE43B7600758D98 /* Suspendable.swift in Sources */ = {isa = PBXBuildFile; fileRef = 032C7A012EE43B7400758D98 /* Suspendable.swift */; }; + 032C7A032EE43B7600758D98 /* Suspendable.swift in Sources */ = {isa = PBXBuildFile; fileRef = 032C7A012EE43B7400758D98 /* Suspendable.swift */; }; + 032C7A042EE43B7600758D98 /* Suspendable.swift in Sources */ = {isa = PBXBuildFile; fileRef = 032C7A012EE43B7400758D98 /* Suspendable.swift */; }; + 032C7A052EE43B7600758D98 /* Suspendable.swift in Sources */ = {isa = PBXBuildFile; fileRef = 032C7A012EE43B7400758D98 /* Suspendable.swift */; }; 032DAC282E8BEB5B0018E01C /* RealWebApi.swift in Sources */ = {isa = PBXBuildFile; fileRef = 032DAC272E8BEB590018E01C /* RealWebApi.swift */; }; 032DAC292E8BEB5B0018E01C /* RealWebApi.swift in Sources */ = {isa = PBXBuildFile; fileRef = 032DAC272E8BEB590018E01C /* RealWebApi.swift */; }; 032DAC2A2E8BEB5B0018E01C /* RealWebApi.swift in Sources */ = {isa = PBXBuildFile; fileRef = 032DAC272E8BEB590018E01C /* RealWebApi.swift */; }; @@ -1028,6 +1032,7 @@ 03263A372E86D5E800BD0415 /* UpdateScheduler.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = UpdateScheduler.swift; sourceTree = ""; }; 0329A9A02E92A2A800A62A12 /* Container.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = Container.swift; sourceTree = ""; }; 0329A9A22E92A68B00A62A12 /* WarningManager+Evaluations.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = "WarningManager+Evaluations.swift"; sourceTree = ""; }; + 032C7A012EE43B7400758D98 /* Suspendable.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = Suspendable.swift; sourceTree = ""; }; 032DAC272E8BEB590018E01C /* RealWebApi.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = RealWebApi.swift; sourceTree = ""; }; 032DAC2C2E8BEB690018E01C /* WebApiProtocol.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = WebApiProtocol.swift; sourceTree = ""; }; 0336CAAF2B0D0CDA009A1034 /* fr */ = {isa = PBXFileReference; lastKnownFileType = text.plist.strings; name = fr; path = fr.lproj/Localizable.strings; sourceTree = ""; }; @@ -1541,6 +1546,7 @@ 5489625628312F95004F647A /* Protocols */ = { isa = PBXGroup; children = ( + 032C7A012EE43B7400758D98 /* Suspendable.swift */, 5489625728312FAD004F647A /* CreatedFromFile.swift */, ); path = Protocols; @@ -2910,6 +2916,7 @@ C41CA5ED2774F8EE00A2C80E /* DomainListVC+Actions.swift in Sources */, 03B675EA2EBA30D800EE04A9 /* NSImageExtension.swift in Sources */, C412E5FC25700D5300A1FB67 /* HomebrewDecodable.swift in Sources */, + 032C7A042EE43B7600758D98 /* Suspendable.swift in Sources */, 03BFF52E2E313244007F96FA /* StatusMenu+Driver.swift in Sources */, C4D9ADBF277610E1007277F4 /* PhpSwitcher.swift in Sources */, C45E76142854A65300B4FE0C /* ServicesManager.swift in Sources */, @@ -3074,6 +3081,7 @@ C4611E5B2AEAD2E30010BE24 /* ConfigManagerWindowController.swift in Sources */, C471E85A28F9BB650021E251 /* DomainListTypeCell.swift in Sources */, C471E85B28F9BB650021E251 /* DomainListKindCell.swift in Sources */, + 032C7A052EE43B7600758D98 /* Suspendable.swift in Sources */, C4611E5E2AEAD2FB0010BE24 /* ConfigManagerView.swift in Sources */, 031F24822EA1071A00CFB8D9 /* Container+Fake.swift in Sources */, C4BF56AD2949381100379603 /* FakeValetInteractor.swift in Sources */, @@ -3454,6 +3462,7 @@ C4D3661D291173EA006BD146 /* DictionaryExtension.swift in Sources */, C471E80D28F9BAE80021E251 /* ArrayExtension.swift in Sources */, 035983A32E97FA9100218DC7 /* Container.swift in Sources */, + 032C7A032EE43B7600758D98 /* Suspendable.swift in Sources */, C471E7CD28F9BA600021E251 /* ShellProtocol.swift in Sources */, C471E7EC28F9BAC30021E251 /* Events.swift in Sources */, C471E7CE28F9BA600021E251 /* RealShell.swift in Sources */, @@ -3658,6 +3667,7 @@ C40F505628ECA64E004AD45B /* TestableConfigurations.swift in Sources */, C4D9ADC9277611A0007277F4 /* InternalSwitcher.swift in Sources */, C449B4F227EE7FC400C47E8A /* DomainListPhpCell.swift in Sources */, + 032C7A022EE43B7600758D98 /* Suspendable.swift in Sources */, C42CFB1A27DFE8BD00862737 /* NginxConfigurationTest.swift in Sources */, C4BF56AC2949381100379603 /* FakeValetInteractor.swift in Sources */, C471E79428F9B23B0021E251 /* FileSystemProtocol.swift in Sources */, diff --git a/phpmon/Common/PHP/PhpConfigurationFile.swift b/phpmon/Common/PHP/PhpConfigurationFile.swift index a6a227b3..8499633f 100644 --- a/phpmon/Common/PHP/PhpConfigurationFile.swift +++ b/phpmon/Common/PHP/PhpConfigurationFile.swift @@ -83,7 +83,7 @@ class PhpConfigurationFile: CreatedFromFile { Replaces the value for a specific (existing) key with a new value. The key must exist for this to work. */ - public func replace(key: String, value: String) throws { + public func replace(key: String, value: String) async throws { // Ensure that the key exists guard let item = getConfig(for: key) else { throw ReplacementErrors.missingKey @@ -102,14 +102,11 @@ class PhpConfigurationFile: CreatedFromFile { self.lines[item.lineIndex] = components.joined(separator: "=") // Ensure the watchers aren't tripped up by config changes - ConfigWatchManager.ignoresModificationsToConfigValues = true - - // Finally, join the string and save the file atomatically again - try self.lines.joined(separator: "\n") - .write(toFile: self.filePath, atomically: true, encoding: .utf8) - - // Ensure watcher behaviour is reverted - ConfigWatchManager.ignoresModificationsToConfigValues = false + try await ConfigWatchManager.withSuspended { + // Finally, join the string and save the file atomically again + try self.lines.joined(separator: "\n") + .write(toFile: self.filePath, atomically: true, encoding: .utf8) + } // Reload the original file self.reload() diff --git a/phpmon/Common/Protocols/Suspendable.swift b/phpmon/Common/Protocols/Suspendable.swift new file mode 100644 index 00000000..d7f6e209 --- /dev/null +++ b/phpmon/Common/Protocols/Suspendable.swift @@ -0,0 +1,58 @@ +// +// Suspendable.swift +// PHP Monitor +// +// Created by Nico Verbruggen on 06/12/2025. +// Copyright © 2025 Nico Verbruggen. All rights reserved. +// + +import Foundation + +/** + A protocol for actors that manage filesystem watchers and can temporarily + suspend their responses to changes. + + This is useful when the application itself makes changes to watched files, + preventing duplicate work or unwanted side effects. + */ +protocol Suspendable: Actor { + /** + Suspends responding to filesystem events. + Events are still observed but handlers won't fire. + */ + func suspend() async + + /** + Resumes responding to filesystem events. + Handlers will fire normally for observed events. + */ + func resume() async + + /** + Executes an action while suspended, ensuring resume happens + even if the action throws. + + - Parameter action: The async throwing closure to execute while suspended + - Returns: The result of the action + - Throws: Rethrows any error from the action + */ + func withSuspended(_ action: () async throws -> T) async rethrows -> T +} + +extension Suspendable { + /** + Default implementation of withSuspended that ensures proper + suspend/resume lifecycle even when errors occur. + */ + func withSuspended(_ action: () async throws -> T) async rethrows -> T { + await suspend() + do { + let result = try await action() + await resume() + return result + } catch { + await resume() + throw error + } + } +} diff --git a/phpmon/Domain/Menu/MainMenu+Actions.swift b/phpmon/Domain/Menu/MainMenu+Actions.swift index 8b48e972..9308cd40 100644 --- a/phpmon/Domain/Menu/MainMenu+Actions.swift +++ b/phpmon/Domain/Menu/MainMenu+Actions.swift @@ -110,14 +110,16 @@ extension MainMenu { return } - do { - try file.replace(key: "xdebug.mode", value: "off") + Task { + do { + try await file.replace(key: "xdebug.mode", value: "off") - Log.perf("Refreshing menu...") - MainMenu.shared.rebuild() - restartPhpFpm() - } catch { - Log.err("There was an issue replacing `xdebug.mode` in \(file.filePath)") + Log.perf("Refreshing menu...") + MainMenu.shared.rebuild() + restartPhpFpm() + } catch { + Log.err("There was an issue replacing `xdebug.mode` in \(file.filePath).") + } } } @@ -128,27 +130,29 @@ extension MainMenu { return Log.info("xdebug.mode could not be found in any .ini file, aborting.") } - do { - var modes = Xdebug(container).activeModes + Task { + do { + var modes = Xdebug(container).activeModes - if let index = modes.firstIndex(of: sender.mode) { - modes.remove(at: index) - } else { - modes.append(sender.mode) + if let index = modes.firstIndex(of: sender.mode) { + modes.remove(at: index) + } else { + modes.append(sender.mode) + } + + var newValue = modes.joined(separator: ",") + if newValue.isEmpty { + newValue = "off" + } + + try await file.replace(key: "xdebug.mode", value: newValue) + + Log.perf("Refreshing menu...") + MainMenu.shared.rebuild() + restartPhpFpm() + } catch { + Log.err("There was an issue replacing `xdebug.mode` in \(file.filePath).") } - - var newValue = modes.joined(separator: ",") - if newValue.isEmpty { - newValue = "off" - } - - try file.replace(key: "xdebug.mode", value: newValue) - - Log.perf("Refreshing menu...") - MainMenu.shared.rebuild() - restartPhpFpm() - } catch { - Log.err("There was an issue replacing `xdebug.mode` in \(file.filePath)") } } diff --git a/phpmon/Domain/Presets/Preset.swift b/phpmon/Domain/Presets/Preset.swift index a654f0ee..cb243dc0 100644 --- a/phpmon/Domain/Presets/Preset.swift +++ b/phpmon/Domain/Presets/Preset.swift @@ -90,7 +90,7 @@ struct Preset: Codable, Equatable { // Apply the configuration changes first for conf in configuration { - applyConfigurationValue(key: conf.key, value: conf.value ?? "") + await applyConfigurationValue(key: conf.key, value: conf.value ?? "") } guard let install = container.phpEnvs.phpInstall else { @@ -159,7 +159,7 @@ struct Preset: Codable, Equatable { } } - private func applyConfigurationValue(key: String, value: String) { + private func applyConfigurationValue(key: String, value: String) async { guard let file = container.phpEnvs.getConfigFile(forKey: key) else { return } @@ -167,7 +167,7 @@ struct Preset: Codable, Equatable { do { if file.has(key: key) { Log.info("Setting config value \(key) in \(file.filePath)") - try file.replace(key: key, value: value) + try await file.replace(key: key, value: value) } } catch { Log.err("Setting \(key) to \(value) failed.") diff --git a/phpmon/Domain/Watcher/ConfigWatchManager.swift b/phpmon/Domain/Watcher/ConfigWatchManager.swift index 5470e05f..b88ec72e 100644 --- a/phpmon/Domain/Watcher/ConfigWatchManager.swift +++ b/phpmon/Domain/Watcher/ConfigWatchManager.swift @@ -8,18 +8,13 @@ import Foundation -actor ConfigWatchManager { +actor ConfigWatchManager: Suspendable { enum Behaviour { case reloadsMenu case reloadsWatchers } - // MARK: Global state (applicable to ALL watchers) - - // TODO: Rework into suspend mechanism (like `HomebrewWatchManager`) to avoid issues with concurrency - static var ignoresModificationsToConfigValues: Bool = false - // MARK: Static methods /** @@ -161,8 +156,7 @@ actor ConfigWatchManager { guard let self = self else { return } Task { - if behaviour == .reloadsWatchers - && !ConfigWatchManager.ignoresModificationsToConfigValues { + if behaviour == .reloadsWatchers { // Reload all configuration watchers on this manager await self.reloadWatchers() return @@ -184,4 +178,43 @@ actor ConfigWatchManager { Log.perf("deinit: \(String(describing: self)).\(#function)") } + // MARK: - Suspendable Protocol + + /** + Performs a particular action while suspending the config watcher, + until the task is completed. + + This should be used when the application writes to PHP configuration files, + to prevent the watcher from responding to our own changes. + */ + public static func withSuspended(_ action: () async throws -> T) async rethrows -> T { + guard let manager = App.shared.configWatchManager else { + // If there's no manager, run the task as-is + return try await action() + } + + // Suspend, execute the action, and resume + return try await manager.withSuspended(action) + } + + /** + Suspends the `ConfigWatchManager`. + This prevents any changes to config files from causing events to fire. + */ + func suspend() async { + for watcher in watchers { + await watcher.suspend() + } + await debouncer.cancel() + } + + /** + Resumes the `ConfigWatchManager`. + Any changes to config files are picked up again. + */ + func resume() async { + for watcher in watchers { + await watcher.resume() + } + } } diff --git a/phpmon/Domain/Watcher/HomebrewWatchManager.swift b/phpmon/Domain/Watcher/HomebrewWatchManager.swift index 0658ba8c..e7d683d1 100644 --- a/phpmon/Domain/Watcher/HomebrewWatchManager.swift +++ b/phpmon/Domain/Watcher/HomebrewWatchManager.swift @@ -8,7 +8,7 @@ import Foundation -actor HomebrewWatchManager { +actor HomebrewWatchManager: Suspendable { // MARK: Public API @@ -37,24 +37,6 @@ actor HomebrewWatchManager { App.shared.homebrewWatchManager = manager } - /** - Performs a particular action while suspending the Homebrew watcher, - until the task is completed. - - Any operations that cause Homebrew to perform tasks (installing, - updating, removing packages) should be wrapped in this helper method, - to prevent the app from doing duplicate work. - */ - public static func withSuspended(_ action: () async throws -> T) async rethrows -> T { - guard let manager = App.shared.homebrewWatchManager else { - // If there's no manager, run the task as-is - return try await action() - } - - // Suspend, execute the action, and resume - return try await manager.withSuspended(action) - } - // MARK: - Instance variables /** @@ -146,13 +128,32 @@ actor HomebrewWatchManager { } } - // MARK: - Suspend and resume + // MARK: - Suspendable Protocol + + /** + Performs a particular action while suspending the Homebrew watcher, + until the task is completed. + + Any operations that cause Homebrew to perform tasks (installing, + updating, removing packages) should be wrapped in this helper method, + to prevent the app from doing duplicate work. + */ + public static func withSuspended(_ action: () async throws -> T) async rethrows -> T { + guard let manager = App.shared.homebrewWatchManager else { + // If there's no manager, run the task as-is + return try await action() + } + + // Suspend, execute the action, and resume + return try await manager.withSuspended(action) + } + /** Suspends the `HomebrewWatchManager`. This prevents any changes to `/homebrew/bin` from causing events to fire. */ - private func suspend() async { + func suspend() async { await watcher?.suspend() await debouncer.cancel() } @@ -161,23 +162,7 @@ actor HomebrewWatchManager { Resumes the `HomebrewWatchManager`. Any changes to `/homebrew/bin` are picked up again. */ - private func resume() async { + func resume() async { await watcher?.resume() } - - /** - Executes an `action` callback after suspending the watcher. - */ - private func withSuspended(_ action: () async throws -> T) async rethrows -> T { - await suspend() - do { - let result = try await action() - await resume() - return result - } catch { - await resume() - throw error - } - } - } diff --git a/phpmon/Modules/PHP Config Editor/Data/BytePhpPreference.swift b/phpmon/Modules/PHP Config Editor/Data/BytePhpPreference.swift index d71d5c66..08f88817 100644 --- a/phpmon/Modules/PHP Config Editor/Data/BytePhpPreference.swift +++ b/phpmon/Modules/PHP Config Editor/Data/BytePhpPreference.swift @@ -62,11 +62,13 @@ class BytePhpPreference: PhpPreference { internalValue = "\(value)\(unit.rawValue)" } - do { - try PhpPreference.persistToIniFile(key: self.key, value: self.internalValue) - Log.info("The preference \(key) was updated to: \(value)") - } catch { - Log.info("The preference \(key) could not be updated") + Task { + do { + try await PhpPreference.persistToIniFile(key: self.key, value: self.internalValue) + Log.info("The preference \(key) was updated to: \(value)") + } catch { + Log.info("The preference \(key) could not be updated") + } } } diff --git a/phpmon/Modules/PHP Config Editor/Data/PhpPreference.swift b/phpmon/Modules/PHP Config Editor/Data/PhpPreference.swift index 106e9999..3d216dd2 100644 --- a/phpmon/Modules/PHP Config Editor/Data/PhpPreference.swift +++ b/phpmon/Modules/PHP Config Editor/Data/PhpPreference.swift @@ -20,9 +20,9 @@ class PhpPreference { self.key = key } - internal static func persistToIniFile(key: String, value: String) throws { + internal static func persistToIniFile(key: String, value: String) async throws { if let file = App.shared.container.phpEnvs.getConfigFile(forKey: key) { - return try file.replace(key: key, value: value) + return try await file.replace(key: key, value: value) } throw PhpConfigurationFile.ReplacementErrors.missingFile