ZIm/crates/gpui/src/platform/mac/display_link.rs
Conrad Irwin a2e98e9f0e
Fix potential race-condition in DisplayLink::drop on macOS (#32116)
Fix a segfault in CVDisplayLink

We see 1-2 crashes a day on macOS on the `CVDisplayLink` thread.

```
Segmentation fault: 11 on thread 9325960 (CVDisplayLink)
CoreVideo	CVHWTime::reset()
CoreVideo	CVXTime::reset()
CoreVideo	CVDisplayLink::runIOThread()
libsystem_pthread.dylib	_pthread_start
libsystem_pthread.dylib	thread_start
```

With the help of the Zed AI, I dove into the crash report, which looks
like this:

```
Crashed Thread:        49  CVDisplayLink

Exception Type:        EXC_BAD_ACCESS (SIGSEGV)
Exception Codes:       KERN_INVALID_ADDRESS at 0x00000000000001f6
Exception Codes:       0x0000000000000001, 0x00000000000001f6

Thread 49 Crashed:: CVDisplayLink
0   CoreVideo                     	       0x18c1ed994 CVHWTime::reset() + 64
1   CoreVideo                     	       0x18c1ee474 CVXTime::reset() + 52
2   CoreVideo                     	       0x18c1ee198 CVDisplayLink::runIOThread() + 176
3   libsystem_pthread.dylib       	       0x18285ac0c _pthread_start + 136
4   libsystem_pthread.dylib       	       0x182855b80 thread_start + 8

Thread 49 crashed with ARM Thread State (64-bit):
    x0: 0x0000000000000000   x1: 0x000000018c206e08   x2: 0x0000002c00001513   x3: 0x0001d4630002a433
    x4: 0x00000e2100000000   x5: 0x0001d46300000000   x6: 0x000000000000002c   x7: 0x0000000000000000
    x8: 0x000000000000002e   x9: 0x000000004d555458  x10: 0x0000000000000000  x11: 0x0000000000000000
   x12: 0x0000000000000000  x13: 0x0000000000000000  x14: 0x0000000000000000  x15: 0x0000000000000000
   x16: 0x0000000182856a9c  x17: 0x00000001f19bc540  x18: 0x0000000000000000  x19: 0x0000600003c56ed8
   x20: 0x000000000002a433  x21: 0x0000000000000000  x22: 0x0000000000000000  x23: 0x0000000000000000
   x24: 0x0000000000000000  x25: 0x0000000000000000  x26: 0x0000000000000000  x27: 0x0000000000000000
   x28: 0x0000000000000000   fp: 0x000000016b02ade0   lr: 0x000000018c1ed984
    sp: 0x000000016b02adc0   pc: 0x000000018c1ed994 cpsr: 0x80001000
   far: 0x00000000000001f6  esr: 0x92000006 (Data Abort) byte read Translation fault

Binary Images:
       0x1828c9000 -        0x182e07fff com.apple.CoreFoundation (6.9) <df489a59-b4f6-32b8-9bb4-9b832960aa52> /System/Library/Frameworks/CoreFoundation.framework/Versions/A/CoreFoundation
```

Using lldb to disassemble `CVHWTime::reset()` (and the AI to interpret
it), the crash is caused by dereferencing the pointer at the start of
the CVHWTime struct + 0x1c8. In this case the pointer has (the clearly
nonsense) value 0x2e (and 0x2e + 0x1c8 = 0x1f6, the failing address).

As to how this could happen...

Looking at the implementation of `CVDisplayLinkRelease`, it calls
straight into `CFRelease` on the main thread; and so it is not safe to
call `CVDisplayLinkRelease` concurrently with other threads that access
the CVDisplayLink. While we already stopped the display link, it turns
out that `CVDisplayLinkStop` just sets a flag on the struct to instruct
the io-thread to exit "soon", and returns immediately. That means we
don't know when the other thread will actually exit, and so we can't
safely call `CVDisplayLinkRelease`.

So, for now, we just leak these objects. They should be created
relatively infrequently (when the app is foregrounded/backgrounded), so
I don't think this is a huge problem.

Release Notes:

- Fix a rare crash on macOS when putting the app in the background.
2025-06-04 17:10:27 -06:00

283 lines
9.9 KiB
Rust

use crate::{
dispatch_get_main_queue,
dispatch_sys::{
_dispatch_source_type_data_add, dispatch_resume, dispatch_set_context,
dispatch_source_cancel, dispatch_source_create, dispatch_source_merge_data,
dispatch_source_set_event_handler_f, dispatch_source_t, dispatch_suspend,
},
};
use anyhow::Result;
use core_graphics::display::CGDirectDisplayID;
use std::ffi::c_void;
use util::ResultExt;
pub struct DisplayLink {
display_link: Option<sys::DisplayLink>,
frame_requests: dispatch_source_t,
}
impl DisplayLink {
pub fn new(
display_id: CGDirectDisplayID,
data: *mut c_void,
callback: unsafe extern "C" fn(*mut c_void),
) -> Result<DisplayLink> {
unsafe extern "C" fn display_link_callback(
_display_link_out: *mut sys::CVDisplayLink,
_current_time: *const sys::CVTimeStamp,
_output_time: *const sys::CVTimeStamp,
_flags_in: i64,
_flags_out: *mut i64,
frame_requests: *mut c_void,
) -> i32 {
unsafe {
let frame_requests = frame_requests as dispatch_source_t;
dispatch_source_merge_data(frame_requests, 1);
0
}
}
unsafe {
let frame_requests = dispatch_source_create(
&_dispatch_source_type_data_add,
0,
0,
dispatch_get_main_queue(),
);
dispatch_set_context(
crate::dispatch_sys::dispatch_object_t {
_ds: frame_requests,
},
data,
);
dispatch_source_set_event_handler_f(frame_requests, Some(callback));
let display_link = sys::DisplayLink::new(
display_id,
display_link_callback,
frame_requests as *mut c_void,
)?;
Ok(Self {
display_link: Some(display_link),
frame_requests,
})
}
}
pub fn start(&mut self) -> Result<()> {
unsafe {
dispatch_resume(crate::dispatch_sys::dispatch_object_t {
_ds: self.frame_requests,
});
self.display_link.as_mut().unwrap().start()?;
}
Ok(())
}
pub fn stop(&mut self) -> Result<()> {
unsafe {
dispatch_suspend(crate::dispatch_sys::dispatch_object_t {
_ds: self.frame_requests,
});
self.display_link.as_mut().unwrap().stop()?;
}
Ok(())
}
}
impl Drop for DisplayLink {
fn drop(&mut self) {
self.stop().log_err();
// We see occasional segfaults on the CVDisplayLink thread.
//
// It seems possible that this happens because CVDisplayLinkRelease releases the CVDisplayLink
// on the main thread immediately, but the background thread that CVDisplayLink uses for timers
// is still accessing it.
//
// We might also want to upgrade to CADisplayLink, but that requires dropping old macOS support.
std::mem::forget(self.display_link.take());
unsafe {
dispatch_source_cancel(self.frame_requests);
}
}
}
mod sys {
//! Derived from display-link crate under the following license:
//! <https://github.com/BrainiumLLC/display-link/blob/master/LICENSE-MIT>
//! Apple docs: [CVDisplayLink](https://developer.apple.com/documentation/corevideo/cvdisplaylinkoutputcallback?language=objc)
#![allow(dead_code, non_upper_case_globals)]
use anyhow::Result;
use core_graphics::display::CGDirectDisplayID;
use foreign_types::{ForeignType, foreign_type};
use std::{
ffi::c_void,
fmt::{self, Debug, Formatter},
};
#[derive(Debug)]
pub enum CVDisplayLink {}
foreign_type! {
pub unsafe type DisplayLink {
type CType = CVDisplayLink;
fn drop = CVDisplayLinkRelease;
fn clone = CVDisplayLinkRetain;
}
}
impl Debug for DisplayLink {
fn fmt(&self, formatter: &mut Formatter) -> fmt::Result {
formatter
.debug_tuple("DisplayLink")
.field(&self.as_ptr())
.finish()
}
}
#[repr(C)]
#[derive(Clone, Copy)]
pub(crate) struct CVTimeStamp {
pub version: u32,
pub video_time_scale: i32,
pub video_time: i64,
pub host_time: u64,
pub rate_scalar: f64,
pub video_refresh_period: i64,
pub smpte_time: CVSMPTETime,
pub flags: u64,
pub reserved: u64,
}
pub type CVTimeStampFlags = u64;
pub const kCVTimeStampVideoTimeValid: CVTimeStampFlags = 1 << 0;
pub const kCVTimeStampHostTimeValid: CVTimeStampFlags = 1 << 1;
pub const kCVTimeStampSMPTETimeValid: CVTimeStampFlags = 1 << 2;
pub const kCVTimeStampVideoRefreshPeriodValid: CVTimeStampFlags = 1 << 3;
pub const kCVTimeStampRateScalarValid: CVTimeStampFlags = 1 << 4;
pub const kCVTimeStampTopField: CVTimeStampFlags = 1 << 16;
pub const kCVTimeStampBottomField: CVTimeStampFlags = 1 << 17;
pub const kCVTimeStampVideoHostTimeValid: CVTimeStampFlags =
kCVTimeStampVideoTimeValid | kCVTimeStampHostTimeValid;
pub const kCVTimeStampIsInterlaced: CVTimeStampFlags =
kCVTimeStampTopField | kCVTimeStampBottomField;
#[repr(C)]
#[derive(Clone, Copy, Default)]
pub(crate) struct CVSMPTETime {
pub subframes: i16,
pub subframe_divisor: i16,
pub counter: u32,
pub time_type: u32,
pub flags: u32,
pub hours: i16,
pub minutes: i16,
pub seconds: i16,
pub frames: i16,
}
pub type CVSMPTETimeType = u32;
pub const kCVSMPTETimeType24: CVSMPTETimeType = 0;
pub const kCVSMPTETimeType25: CVSMPTETimeType = 1;
pub const kCVSMPTETimeType30Drop: CVSMPTETimeType = 2;
pub const kCVSMPTETimeType30: CVSMPTETimeType = 3;
pub const kCVSMPTETimeType2997: CVSMPTETimeType = 4;
pub const kCVSMPTETimeType2997Drop: CVSMPTETimeType = 5;
pub const kCVSMPTETimeType60: CVSMPTETimeType = 6;
pub const kCVSMPTETimeType5994: CVSMPTETimeType = 7;
pub type CVSMPTETimeFlags = u32;
pub const kCVSMPTETimeValid: CVSMPTETimeFlags = 1 << 0;
pub const kCVSMPTETimeRunning: CVSMPTETimeFlags = 1 << 1;
pub type CVDisplayLinkOutputCallback = unsafe extern "C" fn(
display_link_out: *mut CVDisplayLink,
// A pointer to the current timestamp. This represents the timestamp when the callback is called.
current_time: *const CVTimeStamp,
// A pointer to the output timestamp. This represents the timestamp for when the frame will be displayed.
output_time: *const CVTimeStamp,
// Unused
flags_in: i64,
// Unused
flags_out: *mut i64,
// A pointer to app-defined data.
display_link_context: *mut c_void,
) -> i32;
#[link(name = "CoreFoundation", kind = "framework")]
#[link(name = "CoreVideo", kind = "framework")]
#[allow(improper_ctypes, unknown_lints, clippy::duplicated_attributes)]
unsafe extern "C" {
pub fn CVDisplayLinkCreateWithActiveCGDisplays(
display_link_out: *mut *mut CVDisplayLink,
) -> i32;
pub fn CVDisplayLinkSetCurrentCGDisplay(
display_link: &mut DisplayLinkRef,
display_id: u32,
) -> i32;
pub fn CVDisplayLinkSetOutputCallback(
display_link: &mut DisplayLinkRef,
callback: CVDisplayLinkOutputCallback,
user_info: *mut c_void,
) -> i32;
pub fn CVDisplayLinkStart(display_link: &mut DisplayLinkRef) -> i32;
pub fn CVDisplayLinkStop(display_link: &mut DisplayLinkRef) -> i32;
pub fn CVDisplayLinkRelease(display_link: *mut CVDisplayLink);
pub fn CVDisplayLinkRetain(display_link: *mut CVDisplayLink) -> *mut CVDisplayLink;
}
impl DisplayLink {
/// Apple docs: [CVDisplayLinkCreateWithCGDisplay](https://developer.apple.com/documentation/corevideo/1456981-cvdisplaylinkcreatewithcgdisplay?language=objc)
pub unsafe fn new(
display_id: CGDirectDisplayID,
callback: CVDisplayLinkOutputCallback,
user_info: *mut c_void,
) -> Result<Self> {
unsafe {
let mut display_link: *mut CVDisplayLink = 0 as _;
let code = CVDisplayLinkCreateWithActiveCGDisplays(&mut display_link);
anyhow::ensure!(code == 0, "could not create display link, code: {}", code);
let mut display_link = DisplayLink::from_ptr(display_link);
let code = CVDisplayLinkSetOutputCallback(&mut display_link, callback, user_info);
anyhow::ensure!(code == 0, "could not set output callback, code: {}", code);
let code = CVDisplayLinkSetCurrentCGDisplay(&mut display_link, display_id);
anyhow::ensure!(
code == 0,
"could not assign display to display link, code: {}",
code
);
Ok(display_link)
}
}
}
impl DisplayLinkRef {
/// Apple docs: [CVDisplayLinkStart](https://developer.apple.com/documentation/corevideo/1457193-cvdisplaylinkstart?language=objc)
pub unsafe fn start(&mut self) -> Result<()> {
unsafe {
let code = CVDisplayLinkStart(self);
anyhow::ensure!(code == 0, "could not start display link, code: {}", code);
Ok(())
}
}
/// Apple docs: [CVDisplayLinkStop](https://developer.apple.com/documentation/corevideo/1457281-cvdisplaylinkstop?language=objc)
pub unsafe fn stop(&mut self) -> Result<()> {
unsafe {
let code = CVDisplayLinkStop(self);
anyhow::ensure!(code == 0, "could not stop display link, code: {}", code);
Ok(())
}
}
}
}