linux: Fix some panics related to xkb compose (#13529)

Fernando Tagawa created

Release Notes:

- N/A

Fixed #13463 Fixed crash when the locale was non UTF-8 and fixed the
fallback locale.
Fixed #13010 Fixed crash when `compose.keysym()` was `XKB_KEY_NoSymbol`

I also extracted the `xkb_compose_state` to a single place

Change summary

crates/gpui/src/platform/linux/platform.rs       | 22 ++++
crates/gpui/src/platform/linux/wayland/client.rs | 23 +---
crates/gpui/src/platform/linux/x11/client.rs     | 89 ++++++++---------
3 files changed, 71 insertions(+), 63 deletions(-)

Detailed changes

crates/gpui/src/platform/linux/platform.rs 🔗

@@ -3,6 +3,7 @@
 use std::any::{type_name, Any};
 use std::cell::{self, RefCell};
 use std::env;
+use std::ffi::OsString;
 use std::fs::File;
 use std::io::Read;
 use std::ops::{Deref, DerefMut};
@@ -508,6 +509,27 @@ pub(super) fn is_within_click_distance(a: Point<Pixels>, b: Point<Pixels>) -> bo
     diff.x.abs() <= DOUBLE_CLICK_DISTANCE && diff.y.abs() <= DOUBLE_CLICK_DISTANCE
 }
 
+pub(super) fn get_xkb_compose_state(cx: &xkb::Context) -> Option<xkb::compose::State> {
+    let mut locales = Vec::default();
+    if let Some(locale) = std::env::var_os("LC_CTYPE") {
+        locales.push(locale);
+    }
+    locales.push(OsString::from("C"));
+    let mut state: Option<xkb::compose::State> = None;
+    for locale in locales {
+        if let Ok(table) =
+            xkb::compose::Table::new_from_locale(&cx, &locale, xkb::compose::COMPILE_NO_FLAGS)
+        {
+            state = Some(xkb::compose::State::new(
+                &table,
+                xkb::compose::STATE_NO_FLAGS,
+            ));
+            break;
+        }
+    }
+    state
+}
+
 pub(super) unsafe fn read_fd(mut fd: FileDescriptor) -> Result<String> {
     let mut file = File::from_raw_fd(fd.as_raw_fd());
 

crates/gpui/src/platform/linux/wayland/client.rs 🔗

@@ -1,5 +1,4 @@
 use std::cell::{RefCell, RefMut};
-use std::ffi::OsString;
 use std::hash::Hash;
 use std::os::fd::{AsRawFd, BorrowedFd};
 use std::path::PathBuf;
@@ -65,7 +64,6 @@ use xkbcommon::xkb::{self, Keycode, KEYMAP_COMPILE_NO_FLAGS};
 use super::super::{open_uri_internal, read_fd, DOUBLE_CLICK_INTERVAL};
 use super::display::WaylandDisplay;
 use super::window::{ImeInput, WaylandWindowStatePtr};
-use crate::platform::linux::is_within_click_distance;
 use crate::platform::linux::wayland::clipboard::{
     Clipboard, DataOffer, FILE_LIST_MIME_TYPE, TEXT_MIME_TYPE,
 };
@@ -74,6 +72,7 @@ use crate::platform::linux::wayland::serial::{SerialKind, SerialTracker};
 use crate::platform::linux::wayland::window::WaylandWindow;
 use crate::platform::linux::xdg_desktop_portal::{Event as XDPEvent, XDPEventSource};
 use crate::platform::linux::LinuxClient;
+use crate::platform::linux::{get_xkb_compose_state, is_within_click_distance};
 use crate::platform::PlatformWindow;
 use crate::{
     point, px, size, Bounds, DevicePixels, FileDropEvent, ForegroundExecutor, MouseExitEvent, Size,
@@ -1054,21 +1053,8 @@ impl Dispatch<wl_keyboard::WlKeyboard, ()> for WaylandClientStatePtr {
                     .flatten()
                     .expect("Failed to create keymap")
                 };
-                let table = {
-                    let locale = std::env::var_os("LC_CTYPE").unwrap_or(OsString::from("C"));
-                    xkb::compose::Table::new_from_locale(
-                        &xkb_context,
-                        &locale,
-                        xkb::compose::COMPILE_NO_FLAGS,
-                    )
-                    .log_err()
-                    .unwrap()
-                };
                 state.keymap_state = Some(xkb::State::new(&keymap));
-                state.compose_state = Some(xkb::compose::State::new(
-                    &table,
-                    xkb::compose::STATE_NO_FLAGS,
-                ));
+                state.compose_state = get_xkb_compose_state(&xkb_context);
             }
             wl_keyboard::Event::Enter {
                 serial, surface, ..
@@ -1148,6 +1134,7 @@ impl Dispatch<wl_keyboard::WlKeyboard, ()> for WaylandClientStatePtr {
                             compose.feed(keysym);
                             match compose.status() {
                                 xkb::Status::Composing => {
+                                    keystroke.ime_key = None;
                                     state.pre_edit_text =
                                         compose.utf8().or(Keystroke::underlying_dead_key(keysym));
                                     let pre_edit =
@@ -1160,7 +1147,9 @@ impl Dispatch<wl_keyboard::WlKeyboard, ()> for WaylandClientStatePtr {
                                 xkb::Status::Composed => {
                                     state.pre_edit_text.take();
                                     keystroke.ime_key = compose.utf8();
-                                    keystroke.key = xkb::keysym_get_name(compose.keysym().unwrap());
+                                    if let Some(keysym) = compose.keysym() {
+                                        keystroke.key = xkb::keysym_get_name(keysym);
+                                    }
                                 }
                                 xkb::Status::Cancelled => {
                                     let pre_edit = state.pre_edit_text.take();

crates/gpui/src/platform/linux/x11/client.rs 🔗

@@ -1,6 +1,5 @@
 use std::cell::RefCell;
 use std::collections::HashSet;
-use std::ffi::OsString;
 use std::ops::Deref;
 use std::rc::{Rc, Weak};
 use std::time::{Duration, Instant};
@@ -35,7 +34,7 @@ use crate::{
 };
 
 use super::{
-    super::{open_uri_internal, SCROLL_LINES},
+    super::{get_xkb_compose_state, open_uri_internal, SCROLL_LINES},
     X11Display, X11WindowStatePtr, XcbAtoms,
 };
 use super::{button_of_key, modifiers_from_state, pressed_button_from_mask};
@@ -116,7 +115,7 @@ pub struct X11ClientState {
     pub(crate) xim_handler: Option<XimHandler>,
     pub modifiers: Modifiers,
 
-    pub(crate) compose_state: xkbc::compose::State,
+    pub(crate) compose_state: Option<xkbc::compose::State>,
     pub(crate) pre_edit_text: Option<String>,
     pub(crate) composing: bool,
     pub(crate) cursor_handle: cursor::Handle,
@@ -250,18 +249,7 @@ impl X11Client {
             );
             xkbc::x11::state_new_from_device(&xkb_keymap, &xcb_connection, xkb_device_id)
         };
-        let compose_state = {
-            let locale = std::env::var_os("LC_CTYPE").unwrap_or(OsString::from("C"));
-            let table = xkbc::compose::Table::new_from_locale(
-                &xkb_context,
-                &locale,
-                xkbc::compose::COMPILE_NO_FLAGS,
-            )
-            .log_err()
-            .unwrap();
-            xkbc::compose::State::new(&table, xkbc::compose::STATE_NO_FLAGS)
-        };
-
+        let compose_state = get_xkb_compose_state(&xkb_context);
         let resource_database = x11rb::resource_manager::new_from_default(&xcb_connection).unwrap();
 
         let scale_factor = resource_database
@@ -401,7 +389,7 @@ impl X11Client {
             ximc,
             xim_handler,
 
-            compose_state: compose_state,
+            compose_state,
             pre_edit_text: None,
             composing: false,
 
@@ -526,7 +514,9 @@ impl X11Client {
                 window.set_focused(false);
                 let mut state = self.0.borrow_mut();
                 state.focused_window = None;
-                state.compose_state.reset();
+                if let Some(compose_state) = state.compose_state.as_mut() {
+                    compose_state.reset();
+                }
                 state.pre_edit_text.take();
                 drop(state);
                 self.disable_ime();
@@ -572,37 +562,42 @@ impl X11Client {
                     if keysym.is_modifier_key() {
                         return Some(());
                     }
-                    state.compose_state.feed(keysym);
-                    match state.compose_state.status() {
-                        xkbc::Status::Composed => {
-                            state.pre_edit_text.take();
-                            keystroke.ime_key = state.compose_state.utf8();
-                            keystroke.key =
-                                xkbc::keysym_get_name(state.compose_state.keysym().unwrap());
-                        }
-                        xkbc::Status::Composing => {
-                            state.pre_edit_text = state
-                                .compose_state
-                                .utf8()
-                                .or(crate::Keystroke::underlying_dead_key(keysym));
-                            let pre_edit = state.pre_edit_text.clone().unwrap_or(String::default());
-                            drop(state);
-                            window.handle_ime_preedit(pre_edit);
-                            state = self.0.borrow_mut();
-                        }
-                        xkbc::Status::Cancelled => {
-                            let pre_edit = state.pre_edit_text.take();
-                            drop(state);
-                            if let Some(pre_edit) = pre_edit {
-                                window.handle_ime_commit(pre_edit);
+                    if let Some(mut compose_state) = state.compose_state.take() {
+                        compose_state.feed(keysym);
+                        match compose_state.status() {
+                            xkbc::Status::Composed => {
+                                state.pre_edit_text.take();
+                                keystroke.ime_key = compose_state.utf8();
+                                if let Some(keysym) = compose_state.keysym() {
+                                    keystroke.key = xkbc::keysym_get_name(keysym);
+                                }
+                            }
+                            xkbc::Status::Composing => {
+                                keystroke.ime_key = None;
+                                state.pre_edit_text = compose_state
+                                    .utf8()
+                                    .or(crate::Keystroke::underlying_dead_key(keysym));
+                                let pre_edit =
+                                    state.pre_edit_text.clone().unwrap_or(String::default());
+                                drop(state);
+                                window.handle_ime_preedit(pre_edit);
+                                state = self.0.borrow_mut();
                             }
-                            if let Some(current_key) = Keystroke::underlying_dead_key(keysym) {
-                                window.handle_ime_preedit(current_key);
+                            xkbc::Status::Cancelled => {
+                                let pre_edit = state.pre_edit_text.take();
+                                drop(state);
+                                if let Some(pre_edit) = pre_edit {
+                                    window.handle_ime_commit(pre_edit);
+                                }
+                                if let Some(current_key) = Keystroke::underlying_dead_key(keysym) {
+                                    window.handle_ime_preedit(current_key);
+                                }
+                                state = self.0.borrow_mut();
+                                compose_state.feed(keysym);
                             }
-                            state = self.0.borrow_mut();
-                            state.compose_state.feed(keysym);
+                            _ => {}
                         }
-                        _ => {}
+                        state.compose_state = Some(compose_state);
                     }
                     keystroke
                 };
@@ -651,7 +646,9 @@ impl X11Client {
                     window.handle_ime_unmark();
                     state = self.0.borrow_mut();
                 } else if let Some(text) = state.pre_edit_text.take() {
-                    state.compose_state.reset();
+                    if let Some(compose_state) = state.compose_state.as_mut() {
+                        compose_state.reset();
+                    }
                     drop(state);
                     window.handle_ime_commit(text);
                     state = self.0.borrow_mut();