github-starred/MonitorControl

Watch

Fork 0

mirror of https://github.com/MonitorControl/MonitorControl.git synced 2026-10-23 08:07:35 -06:00

[PR #1874] Fix concurrency bugs, DDC reliability, crashes, and memory leaks #1318

New issue

Open

opened 2026-10-02 22:09:45 -06:00 by gitea-mirror · 0 comments

gitea-mirror commented

2026-10-02 22:09:45 -06:00

Owner

📋 Pull Request Information

Original PR: https://github.com/MonitorControl/MonitorControl/pull/1874
Author: @MyronKoch
Created: 5/29/2026
Status: 🔄 Open

Base: main ← Head: fix-concurrency-safety

📝 Commits (2)

85b5a65 Fix concurrency bugs, DDC reliability, crashes, and memory leaks
5afd215 Address review feedback on concurrency and DDC fixes

📊 Changes

8 files changed (+132 additions, -81 deletions)

View changed files

📝 MonitorControl/Model/Display.swift (+32 -21)
📝 MonitorControl/Model/OtherDisplay.swift (+24 -9)
📝 MonitorControl/Support/AppDelegate.swift (+48 -21)
📝 MonitorControl/Support/Arm64DDC.swift (+12 -2)
📝 MonitorControl/Support/DisplayManager.swift (+5 -19)
📝 MonitorControl/Support/MenuHandler.swift (+2 -1)
📝 MonitorControl/Support/SliderHandler.swift (+2 -6)
📝 MonitorControl/View Controllers/Preferences/MainPrefsViewController.swift (+7 -2)

📄 Description

Summary

Fix pass addressing thread safety, DDC communication reliability, crash-inducing edge cases, and resource leaks. Rebased onto current main; review feedback addressed in a follow-up commit.

Thread Safety

Fix swBrightnessSemaphore misuse causing race conditions in software brightness
Signal semaphore before async dispatch rather than holding it across the async boundary
Keep gamma table writes off the main thread in the smooth brightness loop
Wrap shade alpha updates in DispatchQueue.main.async for AppKit safety
Move DDC probe work off main thread with a staleness guard
Defer gamma interference NSAlert to the next run loop

DDC Reliability

Only persist DDC write state after confirming every attempted hardware write succeeded
Clear stale arm64avService handles before rematching on wake/reconfig
Validate DDC/CI reply header fields before accepting read values
Cap retry attempts at 30 to bound startup on unresponsive displays, preserving the prior attempt count underneath the cap
Guard division by zero in DDC value normalization

Crash Fixes

Replace currentDisplay! force-unwrap in MenuHandler (crashes when the pointer is between screens)
Handle CGGetDisplayTransferByTable failure safely
Guard zero defaultGammaTablePeak in getSwBrightness

Cleanup

Remove shadeGrave memory leak
Remove unused sortDisplays()
Remove duplicate audioSpeakerVolume default assignment
Remove tautological self-comparisons in SliderHandler
Migrate login item registration and status read to SMAppService on macOS 13+, legacy path retained below
Use kIOMainPortDefault in Arm64DDC behind an availability check. Other call sites stay on kIOMasterPortDefault, since the deployment target is 10.14
Replace direct plist read with UserDefaults.globalDomain via UserDefaults.persistentDomain

Testing

Tested on an M3 Max with four displays (built-in plus three external TVs over HDMI and DisplayLink). Verified smooth brightness, sleep/wake recovery, and slider responsiveness. Surgical fixes only, no new features and no UI changes.

Note: a clean checkout of main currently fails to build on recent Xcode because MACOSX_DEPLOYMENT_TARGET is 10.14 while Xcode floors at 12.0. That is pre-existing and untouched here; builds were verified with an override.

_{🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.}

## 📋 Pull Request Information **Original PR:** https://github.com/MonitorControl/MonitorControl/pull/1874 **Author:** [@MyronKoch](https://github.com/MyronKoch) **Created:** 5/29/2026 **Status:** 🔄 Open **Base:** `main` ← **Head:** `fix-concurrency-safety` --- ### 📝 Commits (2) - [`85b5a65`](https://github.com/MonitorControl/MonitorControl/commit/85b5a652891b1919b6e1c6ecca57daf8704914fc) Fix concurrency bugs, DDC reliability, crashes, and memory leaks - [`5afd215`](https://github.com/MonitorControl/MonitorControl/commit/5afd2153ca0ab1885dff2123dc674e29c94f720b) Address review feedback on concurrency and DDC fixes ### 📊 Changes **8 files changed** (+132 additions, -81 deletions) <details> <summary>View changed files</summary> 📝 `MonitorControl/Model/Display.swift` (+32 -21) 📝 `MonitorControl/Model/OtherDisplay.swift` (+24 -9) 📝 `MonitorControl/Support/AppDelegate.swift` (+48 -21) 📝 `MonitorControl/Support/Arm64DDC.swift` (+12 -2) 📝 `MonitorControl/Support/DisplayManager.swift` (+5 -19) 📝 `MonitorControl/Support/MenuHandler.swift` (+2 -1) 📝 `MonitorControl/Support/SliderHandler.swift` (+2 -6) 📝 `MonitorControl/View Controllers/Preferences/MainPrefsViewController.swift` (+7 -2) </details> ### 📄 Description ## Summary Fix pass addressing thread safety, DDC communication reliability, crash-inducing edge cases, and resource leaks. Rebased onto current `main`; review feedback addressed in a follow-up commit. ### Thread Safety - Fix swBrightnessSemaphore misuse causing race conditions in software brightness - Signal semaphore before async dispatch rather than holding it across the async boundary - Keep gamma table writes off the main thread in the smooth brightness loop - Wrap shade alpha updates in DispatchQueue.main.async for AppKit safety - Move DDC probe work off main thread with a staleness guard - Defer gamma interference NSAlert to the next run loop ### DDC Reliability - Only persist DDC write state after confirming every attempted hardware write succeeded - Clear stale arm64avService handles before rematching on wake/reconfig - Validate DDC/CI reply header fields before accepting read values - Cap retry attempts at 30 to bound startup on unresponsive displays, preserving the prior attempt count underneath the cap - Guard division by zero in DDC value normalization ### Crash Fixes - Replace currentDisplay! force-unwrap in MenuHandler (crashes when the pointer is between screens) - Handle CGGetDisplayTransferByTable failure safely - Guard zero defaultGammaTablePeak in getSwBrightness ### Cleanup - Remove shadeGrave memory leak - Remove unused sortDisplays() - Remove duplicate audioSpeakerVolume default assignment - Remove tautological self-comparisons in SliderHandler - Migrate login item registration and status read to SMAppService on macOS 13+, legacy path retained below - Use kIOMainPortDefault in Arm64DDC behind an availability check. Other call sites stay on kIOMasterPortDefault, since the deployment target is 10.14 - Replace direct plist read with UserDefaults.globalDomain via UserDefaults.persistentDomain ## Testing Tested on an M3 Max with four displays (built-in plus three external TVs over HDMI and DisplayLink). Verified smooth brightness, sleep/wake recovery, and slider responsiveness. Surgical fixes only, no new features and no UI changes. Note: a clean checkout of `main` currently fails to build on recent Xcode because MACOSX_DEPLOYMENT_TARGET is 10.14 while Xcode floors at 12.0. That is pre-existing and untouched here; builds were verified with an override. --- <sub>🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.</sub>

gitea-mirror added the

pull-request

label

2026-10-02 22:09:45 -06:00

No milestone

No project

No assignees

1 participant

Notifications

Due date

The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set

Reference

github-starred/MonitorControl#1318

No description provided.

Rows
Columns