From 919b00571aa18a97e05c03883e70c2e1051d8e03 Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Fri, 7 Jun 2024 20:29:15 -0700 Subject: [PATCH] font: preload deferred faces in SharedGrid to avoid data races Fixes #1722 Previously, SharedGrid.getIndex would properly lock any access to search for and load the face metadata for a font face that contains a codepoint. However, that face may be "deferred" (metadata loaded but the actual face not loaded). Later, outside of the SharedGrid write lock, a deferred face may be initialized and cause a data race if two threads are doing this at the same time, sometimes loading to a crash. This commit fixes the issue by always preloading font indexes in getIndex because the usage of getIndex implies a very near term future use of getFace. --- src/font/SharedGrid.zig | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/src/font/SharedGrid.zig b/src/font/SharedGrid.zig index e99d39078..8ebbc1710 100644 --- a/src/font/SharedGrid.zig +++ b/src/font/SharedGrid.zig @@ -142,6 +142,11 @@ pub fn cellSize(self: *SharedGrid) renderer.CellSize { } /// Get the font index for a given codepoint. This is cached. +/// +/// This always forces loading any deferred fonts since we assume that if +/// you're looking up an index that the caller plans to use the font. By +/// loading the font in this function we can ensure thread-safety on the +/// load without complicating future calls. pub fn getIndex( self: *SharedGrid, alloc: Allocator, @@ -171,6 +176,18 @@ pub fn getIndex( // Load a value and cache it. This even caches negative matches. const value = self.resolver.getIndex(alloc, cp, style, p); gop.value_ptr.* = value; + + if (value) |idx| preload: { + // If the font is a sprite font then we don't need to preload + // because getFace doesn't work with special fonts. + if (idx.special() != null) break :preload; + + // Load the face in case its deferred. If this fails then we would've + // failed to load it in the future anyways so we want to undo all + // the caching we did. + _ = try self.resolver.collection.getFace(idx); + } + return value; }