gpui: Fix text system reference cycle through pooled line wrappers (#66206)

`TextSystem` pooled whole `LineWrapper`s, and each wrapper owns an
`Arc<TextSystem>`, so once any text was wrapped the pool kept its own
text system (and font caches) alive forever. This leaked one text system
per app for tests and embedders that create and drop apps. The pool now
keeps only each wrapper's per-font character width caches
(`CharWidthCache`), so the cache reuse is preserved without the cycle. A
regression test checks that a `TextSystem` is released after wrapping a
line and dropping all handles.

Release Notes:

- [GPUI] Fixed a memory leak where a `TextSystem` was never freed after
wrapping text, due to a reference cycle through its line wrapper pool
This commit is contained in:
Anthony Eid authored and GitHub committed 2026-10-27 12:58:50 +00:00
1 parent fdcb198110
commit e4a3732b94
2 files changed
+65 -16

No files matched your search

+28 -6
View File
@@ -233,7 +233,7 @@ pub struct TextSystem {
font_ids_by_font: RwLock<FxHashMap<Font, CachedFont>>,
font_metrics: RwLock<FxHashMap<FontId, FontMetrics>>,
raster_bounds: RwLock<FxHashMap<RenderGlyphParams, Bounds<DevicePixels>>>,
wrapper_pool: Mutex<FxHashMap<FontIdWithSize, Vec<LineWrapper>>>,
wrapper_pool: Mutex<FxHashMap<FontIdWithSize, Vec<CharWidthCache>>>,
fallback_font_stack: SmallVec<[Font; 2]>,
font_generation: Arc<AtomicUsize>,
missing_glyph_reporter: Arc<MissingGlyphReporter>,
@@ -515,12 +515,12 @@ impl TextSystem {
pub fn line_wrapper(self: &Arc<Self>, font: Font, font_size: Pixels) -> LineWrapperHandle {
let lock = &mut self.wrapper_pool.lock();
let font_id = self.resolve_font(&font);
let wrappers = lock
let char_widths = lock
.entry(FontIdWithSize { font_id, font_size })
.or_default();
let wrapper = wrappers
.or_default()
.pop()
.unwrap_or_else(|| LineWrapper::new(font_id, font_size, self.clone()));
.unwrap_or_default();
let wrapper = LineWrapper::new(font_id, font_size, self.clone(), char_widths);
LineWrapperHandle {
wrapper: Some(wrapper),
@@ -945,7 +945,7 @@ impl Drop for LineWrapperHandle {
font_size: wrapper.font_size,
})
.unwrap()
.push(wrapper);
.push(wrapper.into_char_widths());
}
}
@@ -1315,6 +1315,28 @@ mod font_resolution_tests {
}
}
#[cfg(test)]
mod line_wrapper_pool_tests {
use super::*;
use crate::NoopTextSystem;
#[test]
fn pooled_line_wrappers_do_not_retain_text_system() {
let text_system = Arc::new(TextSystem::new(Arc::new(NoopTextSystem::new())));
let weak_text_system = Arc::downgrade(&text_system);
let mut wrapper = text_system.line_wrapper(font(".ZedMono"), px(16.));
let fragments = [LineFragment::text("hello world")];
wrapper
.wrap_line(&fragments, px(40.), IndentAdjustment::default())
.for_each(drop);
drop(wrapper);
drop(text_system);
assert!(weak_text_system.upgrade().is_none());
}
}
#[cfg(test)]
mod missing_glyph_tests {
use super::*;
+37 -10
View File
@@ -30,24 +30,46 @@ pub struct LineWrapper {
text_system: Arc<TextSystem>,
pub(crate) font_id: FontId,
pub(crate) font_size: Pixels,
cached_ascii_char_widths: [Option<Pixels>; 128],
cached_other_char_widths: HashMap<char, Pixels>,
char_widths: CharWidthCache,
}
/// Cached character widths for one font and size.
pub(crate) struct CharWidthCache {
ascii: [Option<Pixels>; 128],
other: HashMap<char, Pixels>,
}
impl Default for CharWidthCache {
fn default() -> Self {
Self {
ascii: [None; 128],
other: HashMap::default(),
}
}
}
impl LineWrapper {
/// The maximum indent that can be applied to a line.
pub const MAX_INDENT: u32 = 256;
pub(crate) fn new(font_id: FontId, font_size: Pixels, text_system: Arc<TextSystem>) -> Self {
pub(crate) fn new(
font_id: FontId,
font_size: Pixels,
text_system: Arc<TextSystem>,
char_widths: CharWidthCache,
) -> Self {
Self {
text_system,
font_id,
font_size,
cached_ascii_char_widths: [None; 128],
cached_other_char_widths: HashMap::default(),
char_widths,
}
}
pub(crate) fn into_char_widths(self) -> CharWidthCache {
self.char_widths
}
/// Wrap a line of text to the given width with this wrapper's font and font size.
pub fn wrap_line<'a>(
&'a mut self,
@@ -543,22 +565,22 @@ impl LineWrapper {
#[inline(always)]
fn width_for_char(&mut self, c: char) -> Pixels {
if (c as u32) < 128 {
if let Some(cached_width) = self.cached_ascii_char_widths[c as usize] {
if let Some(cached_width) = self.char_widths.ascii[c as usize] {
cached_width
} else {
let width = self
.text_system
.layout_width(self.font_id, self.font_size, c);
self.cached_ascii_char_widths[c as usize] = Some(width);
self.char_widths.ascii[c as usize] = Some(width);
width
}
} else if let Some(cached_width) = self.cached_other_char_widths.get(&c) {
} else if let Some(cached_width) = self.char_widths.other.get(&c) {
*cached_width
} else {
let width = self
.text_system
.layout_width(self.font_id, self.font_size, c);
self.cached_other_char_widths.insert(c, width);
self.char_widths.other.insert(c, width);
width
}
}
@@ -746,7 +768,12 @@ mod tests {
let dispatcher = TestDispatcher::new(0);
let cx = TestAppContext::build(dispatcher, None);
let id = cx.text_system().resolve_font(&font(".ZedMono"));
LineWrapper::new(id, px(16.), cx.text_system().clone())
LineWrapper::new(
id,
px(16.),
cx.text_system().clone(),
CharWidthCache::default(),
)
}
fn generate_test_runs(input_run_len: &[usize]) -> Vec<TextRun> {