1
0
mirror of https://github.com/nicoverbruggen/phpmon.git synced 2026-03-27 06:20:08 +01:00

🐛 Ensure isBusy is isolated to main thread

There was an issue where updating a configuration file (including .ini)
or having an action occur that marked PHP Monitor as busy would prevent
the icon from updating correctly. This happened because access to the
busy boolean state variable could happen from various threads.

Since adding main thread isolation to the variable, you must access
`PhpEnvironments.shared.isBusy` via the main thread, therefore ensuring
that the busy state that is read from the app is always synchronized
and accurate whenever it is checked, making it so that going from
busy to no longer busy can no longer fail.

(It was possible for work in another thread to complete and fail to set
the icon to "not busy" because the work was done faster than the icon
could be set to busy.)
This commit is contained in:
2023-10-30 19:34:36 +01:00
parent 541378f3f9
commit a62ebcff92
10 changed files with 40 additions and 61 deletions

View File

@@ -91,6 +91,8 @@ struct Constants {
string: "https://raw.githubusercontent.com/nicoverbruggen/homebrew-cask/master/Casks/phpmon-dev.rb" string: "https://raw.githubusercontent.com/nicoverbruggen/homebrew-cask/master/Casks/phpmon-dev.rb"
)! )!
// EAP URLs
static let EarlyAccessCaskFile = URL( static let EarlyAccessCaskFile = URL(
string: "https://phpmon.app/builds/early-access/sponsors/phpmon-eap.rb" string: "https://phpmon.app/builds/early-access/sponsors/phpmon-eap.rb"
)! )!

View File

@@ -13,7 +13,7 @@ class PhpEnvironments {
// MARK: - Initializer // MARK: - Initializer
/** /**
Loads the currently active PHP installation upon startup. May be empty.
*/ */
init() { init() {
self.currentInstall = ActivePhpInstallation.load() self.currentInstall = ActivePhpInstallation.load()
@@ -49,12 +49,10 @@ class PhpEnvironments {
static let shared = PhpEnvironments() static let shared = PhpEnvironments()
/** Whether the switcher is busy performing any actions. */ /** Whether the switcher is busy performing any actions. */
var isBusy: Bool = false { @MainActor var isBusy: Bool = false {
didSet { didSet {
Task { @MainActor in MainMenu.shared.refreshIcon()
MainMenu.shared.setBusyImage() MainMenu.shared.rebuild()
MainMenu.shared.rebuild()
}
} }
} }

View File

@@ -28,8 +28,6 @@ import Foundation
} }
PhpEnvironments.shared.isBusy = true PhpEnvironments.shared.isBusy = true
MainMenu.shared.setBusyImage()
MainMenu.shared.rebuild()
window = TerminalProgressWindowController.display( window = TerminalProgressWindowController.display(
title: "alert.composer_progress.title".localized, title: "alert.composer_progress.title".localized,
@@ -106,14 +104,11 @@ import Foundation
private func removeBusyStatus() { private func removeBusyStatus() {
PhpEnvironments.shared.isBusy = false PhpEnvironments.shared.isBusy = false
Task { @MainActor in
MainMenu.shared.updatePhpVersionInStatusBar()
}
} }
// MARK: Alert // MARK: Alert
@MainActor private func presentMissingAlert() { private func presentMissingAlert() {
BetterAlert() BetterAlert()
.withInformation( .withInformation(
title: "alert.composer_missing.title".localized, title: "alert.composer_missing.title".localized,

View File

@@ -283,7 +283,6 @@ extension MainMenu {
return return
} }
setBusyImage()
PhpEnvironments.shared.isBusy = true PhpEnvironments.shared.isBusy = true
PhpEnvironments.shared.delegate = self PhpEnvironments.shared.delegate = self
PhpEnvironments.shared.delegate?.switcherDidStartSwitching(to: version) PhpEnvironments.shared.delegate?.switcherDidStartSwitching(to: version)
@@ -298,7 +297,6 @@ extension MainMenu {
} }
@objc func switchToPhpVersion(_ version: String) { @objc func switchToPhpVersion(_ version: String) {
setBusyImage()
PhpEnvironments.shared.isBusy = true PhpEnvironments.shared.isBusy = true
PhpEnvironments.shared.delegate = self PhpEnvironments.shared.delegate = self
PhpEnvironments.shared.delegate?.switcherDidStartSwitching(to: version) PhpEnvironments.shared.delegate?.switcherDidStartSwitching(to: version)
@@ -325,7 +323,6 @@ extension MainMenu {
*/ */
func switchToPhp(_ version: String) async { func switchToPhp(_ version: String) async {
Task { @MainActor [self] in Task { @MainActor [self] in
setBusyImage()
PhpEnvironments.shared.isBusy = true PhpEnvironments.shared.isBusy = true
PhpEnvironments.shared.delegate = self PhpEnvironments.shared.delegate = self
PhpEnvironments.shared.delegate?.switcherDidStartSwitching(to: version) PhpEnvironments.shared.delegate?.switcherDidStartSwitching(to: version)

View File

@@ -45,21 +45,16 @@ extension MainMenu {
.broadcastServicesUpdate .broadcastServicesUpdate
] ]
) { ) {
if behaviours.contains(.reloadsPhpInstallation) { if behaviours.contains(.reloadsPhpInstallation) || behaviours.contains(.setsBusyUI) {
PhpEnvironments.shared.isBusy = true PhpEnvironments.shared.isBusy = true
} }
if behaviours.contains(.setsBusyUI) {
setBusyImage()
}
Task(priority: .userInitiated) { [unowned self] in Task(priority: .userInitiated) { [unowned self] in
var error: Error? var error: Error?
do { try execute() } catch let e { error = e } do { try execute() } catch let e {
error = e
if behaviours.contains(.setsBusyUI) { Log.err(e)
PhpEnvironments.shared.isBusy = false
} }
Task { @MainActor [self, error] in Task { @MainActor [self, error] in
@@ -69,14 +64,16 @@ extension MainMenu {
if behaviours.contains(.updatesMenuBarContents) { if behaviours.contains(.updatesMenuBarContents) {
updatePhpVersionInStatusBar() updatePhpVersionInStatusBar()
} else if behaviours.contains(.setsBusyUI) {
refreshIcon()
} }
if behaviours.contains(.broadcastServicesUpdate) { if behaviours.contains(.broadcastServicesUpdate) {
Task { await ServicesManager.shared.reloadServicesStatus() } Task { await ServicesManager.shared.reloadServicesStatus() }
} }
if behaviours.contains(.setsBusyUI) {
PhpEnvironments.shared.isBusy = false
}
if error != nil { if error != nil {
return failure(error!) return failure(error!)
} }

View File

@@ -16,7 +16,9 @@ extension MainMenu {
nonisolated func switcherDidCompleteSwitch(to version: String) { nonisolated func switcherDidCompleteSwitch(to version: String) {
// Mark as no longer busy // Mark as no longer busy
PhpEnvironments.shared.isBusy = false Task { @MainActor in
PhpEnvironments.shared.isBusy = false
}
Task { // Things to do after reloading domain list data Task { // Things to do after reloading domain list data
if Valet.installed { if Valet.installed {

View File

@@ -87,6 +87,7 @@ class MainMenu: NSObject, NSWindowDelegate, NSMenuDelegate, PhpSwitcherDelegate
} }
/** Updates the icon (refresh icon) and rebuilds the menu. */ /** Updates the icon (refresh icon) and rebuilds the menu. */
@available(*, deprecated, message: "Use the busy status instead")
@objc func updatePhpVersionInStatusBar() { @objc func updatePhpVersionInStatusBar() {
refreshIcon() refreshIcon()
rebuild() rebuild()
@@ -139,7 +140,7 @@ class MainMenu: NSObject, NSWindowDelegate, NSMenuDelegate, PhpSwitcherDelegate
@objc func reloadPhpMonitorMenuInBackground() { @objc func reloadPhpMonitorMenuInBackground() {
asyncExecution({ asyncExecution({
// This automatically reloads the menu // This automatically reloads the menu
Log.info("Reloading information about the PHP installation (in the background)...") Log.perf("Reloading information about the PHP installation (in the background)...")
}, behaviours: [ }, behaviours: [
.setsBusyUI, .setsBusyUI,
.reloadsPhpInstallation, .reloadsPhpInstallation,
@@ -150,10 +151,13 @@ class MainMenu: NSObject, NSWindowDelegate, NSMenuDelegate, PhpSwitcherDelegate
/** Refreshes the icon with the PHP version. */ /** Refreshes the icon with the PHP version. */
@objc func refreshIcon() { @objc func refreshIcon() {
Task { @MainActor [self] in Task { @MainActor [self] in
if PhpEnvironments.shared.isBusy { if PhpEnvironments.shared.isBusy {
Log.perf("Refreshing icon: currently busy")
setStatusBar(image: NSImage(named: NSImage.Name("StatusBarIcon"))!) setStatusBar(image: NSImage(named: NSImage.Name("StatusBarIcon"))!)
} else { } else {
Log.perf("Refreshing icon: no longer busy")
if Preferences.preferences[.shouldDisplayDynamicIcon] as! Bool == false { if Preferences.preferences[.shouldDisplayDynamicIcon] as! Bool == false {
// Static icon has been requested // Static icon has been requested
setStatusBar(image: NSImage(named: NSImage.Name("StatusBarIconStatic"))!) setStatusBar(image: NSImage(named: NSImage.Name("StatusBarIconStatic"))!)
@@ -172,13 +176,6 @@ class MainMenu: NSObject, NSWindowDelegate, NSMenuDelegate, PhpSwitcherDelegate
} }
} }
/** Updates the icon to be displayed as busy. */
@objc func setBusyImage() {
Task { @MainActor [self] in
setStatusBar(image: NSImage(named: NSImage.Name("StatusBarIcon"))!)
}
}
// MARK: - Menu Item Functionality // MARK: - Menu Item Functionality
@objc func openAbout() { @objc func openAbout() {

View File

@@ -12,7 +12,7 @@ import Cocoa
extension StatusMenu { extension StatusMenu {
func addPhpVersionMenuItems() { @MainActor func addPhpVersionMenuItems() {
if PhpEnvironments.phpInstall == nil { if PhpEnvironments.phpInstall == nil {
addItem(HeaderView.asMenuItem(text: "⚠️ " + "mi_no_php_linked".localized, minimumWidth: 280)) addItem(HeaderView.asMenuItem(text: "⚠️ " + "mi_no_php_linked".localized, minimumWidth: 280))
addItems([ addItems([
@@ -34,7 +34,7 @@ extension StatusMenu {
)) ))
} }
func addPhpActionMenuItems() { @MainActor func addPhpActionMenuItems() {
if PhpEnvironments.shared.isBusy { if PhpEnvironments.shared.isBusy {
addItem(NSMenuItem(title: "mi_busy".localized)) addItem(NSMenuItem(title: "mi_busy".localized))
return return
@@ -54,7 +54,7 @@ extension StatusMenu {
self.addItem(NSMenuItem.separator()) self.addItem(NSMenuItem.separator())
} }
func addServicesManagerMenuItem() { @MainActor func addServicesManagerMenuItem() {
if PhpEnvironments.shared.isBusy { if PhpEnvironments.shared.isBusy {
return return
} }
@@ -65,7 +65,7 @@ extension StatusMenu {
]) ])
} }
func addSwitchToPhpMenuItems() { @MainActor func addSwitchToPhpMenuItems() {
var shortcutKey = 1 var shortcutKey = 1
for index in (0..<PhpEnvironments.shared.availablePhpVersions.count) { for index in (0..<PhpEnvironments.shared.availablePhpVersions.count) {
// Get the short and long version // Get the short and long version
@@ -102,14 +102,14 @@ extension StatusMenu {
} }
} }
func addLiteModeMenuItem() { @MainActor func addLiteModeMenuItem() {
addItems([ addItems([
NSMenuItem.separator(), NSMenuItem.separator(),
NSMenuItem(title: "mi_lite_mode".localized, action: #selector(MainMenu.openLiteModeInfo)) NSMenuItem(title: "mi_lite_mode".localized, action: #selector(MainMenu.openLiteModeInfo))
]) ])
} }
func addPreferencesMenuItems() { @MainActor func addPreferencesMenuItems() {
addItems([ addItems([
NSMenuItem.separator(), NSMenuItem.separator(),
NSMenuItem(title: "mi_preferences".localized, NSMenuItem(title: "mi_preferences".localized,
@@ -119,7 +119,7 @@ extension StatusMenu {
]) ])
} }
func addCoreMenuItems() { @MainActor func addCoreMenuItems() {
addItems([ addItems([
NSMenuItem.separator(), NSMenuItem.separator(),
NSMenuItem(title: "mi_about".localized, NSMenuItem(title: "mi_about".localized,
@@ -131,7 +131,7 @@ extension StatusMenu {
// MARK: - Valet // MARK: - Valet
func addValetMenuItems() { @MainActor func addValetMenuItems() {
addItems([ addItems([
HeaderView.asMenuItem(text: "mi_valet".localized), HeaderView.asMenuItem(text: "mi_valet".localized),
NSMenuItem(title: "mi_valet_config".localized, NSMenuItem(title: "mi_valet_config".localized,
@@ -146,7 +146,7 @@ extension StatusMenu {
// MARK: - PHP Configuration // MARK: - PHP Configuration
func addConfigurationMenuItems() { @MainActor func addConfigurationMenuItems() {
addItems([ addItems([
HeaderView.asMenuItem(text: "mi_configuration".localized), HeaderView.asMenuItem(text: "mi_configuration".localized),
NSMenuItem(title: "mi_php_version_manager".localized, NSMenuItem(title: "mi_php_version_manager".localized,
@@ -166,7 +166,7 @@ extension StatusMenu {
// MARK: - Composer // MARK: - Composer
func addComposerMenuItems() { @MainActor func addComposerMenuItems() {
addItems([ addItems([
HeaderView.asMenuItem(text: "mi_composer".localized), HeaderView.asMenuItem(text: "mi_composer".localized),
NSMenuItem( NSMenuItem(
@@ -187,7 +187,7 @@ extension StatusMenu {
// MARK: - Stats // MARK: - Stats
func addStatsMenuItem() { @MainActor func addStatsMenuItem() {
guard let install = PhpEnvironments.phpInstall else { guard let install = PhpEnvironments.phpInstall else {
Log.info("Not showing stats menu item if no PHP version is linked.") Log.info("Not showing stats menu item if no PHP version is linked.")
return return
@@ -214,7 +214,7 @@ extension StatusMenu {
// MARK: - Extensions // MARK: - Extensions
func addExtensionsMenuItems() { @MainActor func addExtensionsMenuItems() {
guard let install = PhpEnvironments.phpInstall else { guard let install = PhpEnvironments.phpInstall else {
Log.info("Not showing extensions menu items if no PHP version is linked.") Log.info("Not showing extensions menu items if no PHP version is linked.")
return return
@@ -235,7 +235,7 @@ extension StatusMenu {
// MARK: - Presets // MARK: - Presets
func addPresetsMenuItem() { @MainActor func addPresetsMenuItem() {
guard let presets = Preferences.custom.presets else { guard let presets = Preferences.custom.presets else {
addEmptyPresetHelp() addEmptyPresetHelp()
return return

View File

@@ -9,7 +9,7 @@ import Cocoa
class StatusMenu: NSMenu { class StatusMenu: NSMenu {
// swiftlint:disable cyclomatic_complexity // swiftlint:disable cyclomatic_complexity
func addMenuItems() { @MainActor func addMenuItems() {
addPhpVersionMenuItems() addPhpVersionMenuItems()
addItem(NSMenuItem.separator()) addItem(NSMenuItem.separator())

View File

@@ -330,18 +330,9 @@ struct PhpVersionManagerView: View {
} }
public func setBusyStatus(_ busy: Bool) { public func setBusyStatus(_ busy: Bool) {
PhpEnvironments.shared.isBusy = busy Task { @MainActor in
if busy { PhpEnvironments.shared.isBusy = busy
Task { @MainActor in self.status.busy = busy
MainMenu.shared.setBusyImage()
MainMenu.shared.rebuild()
self.status.busy = busy
}
} else {
Task { @MainActor in
MainMenu.shared.updatePhpVersionInStatusBar()
self.status.busy = busy
}
} }
} }