Auto-dismount volumes on system sleep and screen lock on macOS#1818
Auto-dismount volumes on system sleep and screen lock on macOS#1818damianrickard wants to merge 1 commit into
Conversation
|
Thanks for the PR. I agree this addresses a real macOS gap: the existing wx power event path is not active on macOS, and the screen saver auto dismount logic is Windows only. Before merging, I think a few points need to be addressed:
The overall approach is reasonable, but I would prefer these points fixed or explicitly validated before merging. |
| - (void) systemWillSleep: (NSNotification *) notification | ||
| { | ||
| (void) notification; | ||
| VeraCrypt::OnMacOSXSystemWillSleep (); | ||
| } | ||
|
|
||
| - (void) screenLocked: (NSNotification *) notification | ||
| { | ||
| (void) notification; | ||
| VeraCrypt::OnMacOSXScreenLocked (); | ||
| } |
There was a problem hiding this comment.
Please add an exception boundary before returning to Cocoa notification delivery. These callbacks enter the VeraCrypt dismount path, and GetMountedVolumes can still throw before or after DismountVolumes. Exceptions shouldn't escape through AppKit notification dispatch.
| // Screen lock. macOS exposes no public notification for this; the | ||
| // com.apple.screenIsLocked distributed notification is the long-standing | ||
| // de-facto mechanism used to detect it. | ||
| [[NSDistributedNotificationCenter defaultCenter] | ||
| addObserver: SleepLockObserver | ||
| selector: @selector (screenLocked:) | ||
| name: @"com.apple.screenIsLocked" | ||
| object: nil]; | ||
| } |
There was a problem hiding this comment.
Please treat this distributed notification as untrusted and verify that the session is actually locked before dismounting. Otherwise another local process may be able to post the same notification and trigger VeraCrypt forced dismount.
| void OnMacOSXSystemWillSleep () | ||
| { | ||
| if (Gui && Gui->GetPreferences().BackgroundTaskEnabled && Gui->GetPreferences().DismountOnPowerSaving) | ||
| Gui->OnAutoDismountAllEvent(); | ||
| } | ||
|
|
||
| // Called from MacOSXSleepLock.mm on the main thread when the screen is | ||
| // locked. macOS has no equivalent of the Windows screen-saver polling used | ||
| // by MainFrame::OnTimer, so screen-lock dismount is wired up here instead. | ||
| void OnMacOSXScreenLocked () | ||
| { | ||
| if (Gui && Gui->GetPreferences().BackgroundTaskEnabled && Gui->GetPreferences().DismountOnScreenSaver) | ||
| Gui->OnAutoDismountAllEvent(); | ||
| } |
There was a problem hiding this comment.
This calls OnAutoDismountAllEvent, which currently uses AutoDismountVolumes with alwaysForce=true. Please make that behavior explicit and intentional for macOS sleep and screen lock, or use a path that honors ForceAutoDismount. Screen lock can otherwise detach busy volumes.
| // On macOS, wxHAS_POWER_EVENTS is undefined but sleep is handled by a | ||
| // native observer (see MacOSXSleepLock), so keep the checkbox visible. | ||
| #if !defined(wxHAS_POWER_EVENTS) && !defined(TC_MACOSX) | ||
| DismountOnPowerSavingCheckBox->Show (false); | ||
| #endif | ||
|
|
||
| #ifdef TC_MACOSX | ||
| DismountOnScreenSaverCheckBox->Show (false); | ||
| // The "screen saver" checkbox drives screen-lock dismount on macOS (see | ||
| // MacOSXSleepLock), so it is left visible here. | ||
| DismountOnLogOffCheckBox->SetLabel (LangString["LINUX_VC_QUITS"]); |
There was a problem hiding this comment.
Please align the visible preference text with the macOS behavior. If this means screen lock, please use the existing IDC_PREF_UNMOUNT_SESSION_LOCKED label on macOS, or also handle the actual screen saver notification. Also, making the power saving checkbox visible exposes the existing LINUX_ENTERING_POVERSAWING typo, which should be fixed or avoided.
On macOS, mounted volumes stayed mounted (and their keys resident in memory) when the system went to sleep or the screen was locked. The existing auto-dismount triggers do not cover these on macOS: - Power-suspend dismount is connected only under wxHAS_POWER_EVENTS, which wxWidgets does not define on macOS, so OnPowerSuspending never fires. - Screen-saver dismount in MainFrame::OnTimer is inside #ifdef TC_WINDOWS (SPI_GETSCREENSAVERRUNNING) and has no macOS path. Add a small Objective-C++ helper (MacOSXSleepLock) that observes NSWorkspaceWillSleepNotification and the com.apple.screenIsLocked distributed notification and calls back into GraphicUserInterface, gated by the existing DismountOnPowerSaving / DismountOnScreenSaver / BackgroundTaskEnabled preferences. The observer is installed in OnInit and removed in the destructor. Details: - The notification callbacks are exception boundaries: no C++ exception can unwind into AppKit notification dispatch (GetMountedVolumes and the dismount path can throw). The dismount is best-effort; UI cannot be shown reliably while the session is locking or the system is going to sleep. - com.apple.screenIsLocked is a de facto but undocumented distributed notification that any local process can post. It is treated only as a hint: the handler verifies via CGSessionCopyCurrentDictionary that the session is actually locked before dismounting, re-checking once after a short delay to tolerate notification/state ordering. - The dismount honors the ForceAutoDismount preference (alwaysForce = false), matching the Windows auto-dismount events, which all pass bForceAutoDismount. Volumes with open files are only detached if the user enabled forced auto-dismount. - The two preference checkboxes, previously hidden on macOS, are now shown. Because the checkbox drives screen-lock dismount on macOS, it is relabeled with the existing IDC_PREF_UNMOUNT_SESSION_LOCKED string (translated in all languages). The power-saving checkbox referenced the nonexistent key LINUX_ENTERING_POVERSAWING; point it at the correctly spelled LINUX_ENTERING_POWERSAVING, which exists in Language.xml and all translations (this also fixes the label on Linux builds that show this checkbox). All new code is macOS-only (guarded by TC_MACOSX and built solely via the MacOSX block of Main.make); the only cross-platform change is the Forms label key correction. No new framework link flags are needed: ApplicationServices/CoreGraphics symbols already resolve through the wxWidgets link line. Note: sleep dismount is best-effort within the short window NSWorkspace grants before sleep; delaying sleep via an IOKit power assertion to guarantee completion could be a follow-up.
03b0047 to
af355cc
Compare
|
Thanks for the detailed review — all five points are addressed:
Rebased onto current master; builds clean and Validation so far (Apple Silicon, macOS 26.5.1, FUSE-T 1.2.7), end-to-end with a real mounted volume:
I don't have MacFUSE or an Intel machine set up here, so I haven't yet covered MacFUSE, Intel, or the individual sleep variants (Apple-menu / lid-close / idle). I'll report on those separately rather than claim them now. |
On macOS, mounted volumes stayed mounted (and their keys resident in memory) when the system went to sleep or the screen was locked. The existing auto-dismount triggers do not cover these on macOS:
wxHAS_POWER_EVENTS, which wxWidgets does not define on macOS, soOnPowerSuspendingnever fires.MainFrame::OnTimeris inside#ifdef TC_WINDOWS(SPI_GETSCREENSAVERRUNNING) and has no macOS path.This adds a small Objective-C++ helper (
MacOSXSleepLock) that observesNSWorkspaceWillSleepNotificationand thecom.apple.screenIsLockeddistributed notification, and calls back intoGraphicUserInterfaceto run the existingOnAutoDismountAllEvent()path, gated by the existingDismountOnPowerSaving/DismountOnScreenSaver/BackgroundTaskEnabledpreferences. The observer is installed inOnInitand removed in the destructor. The two preference checkboxes, previously hidden on macOS, are now shown so users can enable the behaviour; the "screen saver" checkbox drives screen-lock dismount on macOS.All new code is macOS-only (guarded by
TC_MACOSX, built solely via the MacOSX block ofMain.make); no other platform is affected.Note: sleep dismount is best-effort within the short window
NSWorkspacegrants before sleep; delaying sleep via an IOKit power assertion to guarantee completion could be a follow-up.