From 836d794b9e686cf17ec419c04dd2ad94bef16037 Mon Sep 17 00:00:00 2001 From: Tobias Kohlbau Date: Tue, 25 Nov 2025 22:19:57 +0100 Subject: [PATCH] termio: report color scheme synchronously The reporting of color scheme was handled asynchronously by queuing a handler in the surface. This could lead to race conditions where the DSR is reported after subsequent VT sequences. Fixes #5922 --- src/Surface.zig | 24 +----------------------- src/apprt/surface.zig | 5 ----- src/termio/Termio.zig | 21 +++++++++++++++++++++ src/termio/Thread.zig | 1 + src/termio/message.zig | 6 ++++++ src/termio/stream_handler.zig | 6 +++--- 6 files changed, 32 insertions(+), 31 deletions(-) diff --git a/src/Surface.zig b/src/Surface.zig index 4103b91fb..5ca75d865 100644 --- a/src/Surface.zig +++ b/src/Surface.zig @@ -1073,8 +1073,6 @@ pub fn handleMessage(self: *Surface, msg: Message) !void { .scrollbar => |scrollbar| self.updateScrollbar(scrollbar), - .report_color_scheme => |force| self.reportColorScheme(force), - .present_surface => try self.presentSurface(), .password_input => |v| try self.passwordInput(v), @@ -1386,26 +1384,6 @@ fn passwordInput(self: *Surface, v: bool) !void { try self.queueRender(); } -/// Sends a DSR response for the current color scheme to the pty. If -/// force is false then we only send the response if the terminal mode -/// 2031 is enabled. -fn reportColorScheme(self: *Surface, force: bool) void { - if (!force) { - self.renderer_state.mutex.lock(); - defer self.renderer_state.mutex.unlock(); - if (!self.renderer_state.terminal.modes.get(.report_color_scheme)) { - return; - } - } - - const output = switch (self.config_conditional_state.theme) { - .light => "\x1B[?997;2n", - .dark => "\x1B[?997;1n", - }; - - self.queueIo(.{ .write_stable = output }, .unlocked); -} - fn searchCallback(event: terminal.search.Thread.Event, ud: ?*anyopaque) void { // IMPORTANT: This function is run on the SEARCH THREAD! It is NOT SAFE // to access anything other than values that never change on the surface. @@ -5039,7 +5017,7 @@ pub fn colorSchemeCallback(self: *Surface, scheme: apprt.ColorScheme) !void { self.notifyConfigConditionalState(); // If mode 2031 is on, then we report the change live. - self.reportColorScheme(false); + self.queueIo(.{ .color_scheme_report = .{ .force = false } }, .unlocked); } pub fn posToViewport(self: Surface, xpos: f64, ypos: f64) terminal.point.Coordinate { diff --git a/src/apprt/surface.zig b/src/apprt/surface.zig index be2f59149..5c25281c8 100644 --- a/src/apprt/surface.zig +++ b/src/apprt/surface.zig @@ -63,11 +63,6 @@ pub const Message = union(enum) { /// Health status change for the renderer. renderer_health: renderer.Health, - /// Report the color scheme. The bool parameter is whether to force or not. - /// If force is true, the color scheme should be reported even if mode - /// 2031 is not set. - report_color_scheme: bool, - /// Tell the surface to present itself to the user. This may require raising /// a window and switching tabs. present_surface: void, diff --git a/src/termio/Termio.zig b/src/termio/Termio.zig index 7263418a7..a1bcea6d3 100644 --- a/src/termio/Termio.zig +++ b/src/termio/Termio.zig @@ -165,6 +165,7 @@ pub const DerivedConfig = struct { osc_color_report_format: configpkg.Config.OSCColorReportFormat, clipboard_write: configpkg.ClipboardAccess, enquiry_response: []const u8, + conditional_state: configpkg.ConditionalState, pub fn init( alloc_gpa: Allocator, @@ -185,6 +186,7 @@ pub const DerivedConfig = struct { .osc_color_report_format = config.@"osc-color-report-format", .clipboard_write = config.@"clipboard-write", .enquiry_response = try alloc.dupe(u8, config.@"enquiry-response"), + .conditional_state = config._conditional_state, // This has to be last so that we copy AFTER the arena allocations // above happen (Zig assigns in order). @@ -712,6 +714,25 @@ fn processOutputLocked(self: *Termio, buf: []const u8) void { } } +/// Sends a DSR response for the current color scheme to the pty. +pub fn colorSchemeReport(self: *Termio, td: *ThreadData, force: bool) !void { + self.renderer_state.mutex.lock(); + defer self.renderer_state.mutex.unlock(); + + try self.colorSchemeReportLocked(td, force); +} + +pub fn colorSchemeReportLocked(self: *Termio, td: *ThreadData, force: bool) !void { + if (!force and !self.renderer_state.terminal.modes.get(.report_color_scheme)) { + return; + } + const output = switch (self.config.conditional_state.theme) { + .light => "\x1B[?997;2n", + .dark => "\x1B[?997;1n", + }; + try self.queueWrite(td, output, false); +} + /// ThreadData is the data created and stored in the termio thread /// when the thread is started and destroyed when the thread is /// stopped. diff --git a/src/termio/Thread.zig b/src/termio/Thread.zig index b111d5a52..6aa5e1c26 100644 --- a/src/termio/Thread.zig +++ b/src/termio/Thread.zig @@ -311,6 +311,7 @@ fn drainMailbox( log.debug("mailbox message={s}", .{@tagName(message)}); switch (message) { + .color_scheme_report => |v| try io.colorSchemeReport(data, v.force), .crash => @panic("crash request, crashing intentionally"), .change_config => |config| { defer config.alloc.destroy(config.ptr); diff --git a/src/termio/message.zig b/src/termio/message.zig index f78da2058..d7a59bf5e 100644 --- a/src/termio/message.zig +++ b/src/termio/message.zig @@ -16,6 +16,12 @@ pub const Message = union(enum) { /// in the future. pub const WriteReq = MessageData(u8, 38); + /// Request a color scheme report is sent to the pty. + color_scheme_report: struct { + /// Force write the current color scheme + force: bool, + }, + /// Purposely crash the renderer. This is used for testing and debugging. /// See the "crash" binding action. crash: void, diff --git a/src/termio/stream_handler.zig b/src/termio/stream_handler.zig index 082a0fa10..75f6da57e 100644 --- a/src/termio/stream_handler.zig +++ b/src/termio/stream_handler.zig @@ -119,7 +119,7 @@ pub const StreamHandler = struct { }; // The config could have changed any of our colors so update mode 2031 - self.surfaceMessageWriter(.{ .report_color_scheme = false }); + self.messageWriter(.{ .color_scheme_report = .{ .force = false } }); } inline fn surfaceMessageWriter( @@ -871,7 +871,7 @@ pub const StreamHandler = struct { self.messageWriter(msg); }, - .color_scheme => self.surfaceMessageWriter(.{ .report_color_scheme = true }), + .color_scheme => self.messageWriter(.{ .color_scheme_report = .{ .force = true } }), } } @@ -956,7 +956,7 @@ pub const StreamHandler = struct { try self.setMouseShape(.text); // Reset resets our palette so we report it for mode 2031. - self.surfaceMessageWriter(.{ .report_color_scheme = false }); + self.messageWriter(.{ .color_scheme_report = .{ .force = false } }); // Clear the progress bar self.progressReport(.{ .state = .remove });