From cf94b310aa2ea343c619845b89c00d6cc5a10e5d Mon Sep 17 00:00:00 2001 From: Johannes Altmanninger Date: Sun, 12 Mar 2023 09:25:55 +0100 Subject: [PATCH] Fix crash after multiple terminal resizes When Kakoune's terminal is shown on my laptop monitor and I plug in my external monitor, the terminal's workspace will move to that external monitor. When this happens, Kakoune may segfault. There are multiple resize events (SIGWINCH) in quick succession; it crashes because we handle SIGWINCH during rendering. The problem happens during execution of "TerminalUI::Screen::output" (frame #18). When we receive SIGWINCH while writing to stdout, write(2) fails with EAGAIN, prompting us to handle pending events (See ae001a1f9 (Run EventManager whenever writing to a file descriptor would block, 2022-05-10)). We update the screen size in check_resize() here: #4 Kakoune::TerminalUI::check_resize (force=) at terminal_ui.cc:683 #5 Kakoune::TerminalUI::get_next_key () at terminal_ui.cc:719 #6 operator() (__closure=0x555555984198) at terminal_ui.cc:484 #7 std::__invoke_impl&, Kakoune::FDWatcher&, Kakoune::FdEvents, Kakoune::EventMode> (__f=...) at /usr/include/c++/12.2.1/bits/invoke.h:61 #8 std::__invoke_r&, Kakoune::FDWatcher&, Kakoune::FdEvents, Kakoune::EventMode> (__fn=...) at /usr/include/c++/12.2.1/bits/invoke.h:111 #9 std::_Function_handler >::_M_invoke(const std::_Any_data &, Kakoune::FDWatcher &, Kakoune::FdEvents &&, Kakoune::EventMode &&) (__functor=..., __args#0=..., __args#1=, __args#2=) at /usr/include/c++/12.2.1/bits/std_function.h:290 #10 std::function::operator()(Kakoune::FDWatcher&, Kakoune::FdEvents, Kakoune::EventMode) const (__args#2=, __args#1=, __args#0=...) at /usr/include/c++/12.2.1/bits/std_function.h:591 #11 Kakoune::FDWatcher::run (mode=Kakoune::EventMode::Urgent, events=) at event_manager.cc:28 #12 Kakoune::EventManager::handle_next_events (mode=mode@entry=Kakoune::EventMode::Urgent, sigmask=sigmask@entry=0x0, block=, block@entry=false) at event_manager.cc:143 #13 Kakoune::write (fd=1, data=...) at file.cc:273 #14 Kakoune::BufferedWriter<4096>::flush () at string.hh:236 #15 Kakoune::BufferedWriter<4096>::write (data="t file.hh:145 #16 Kakoune::TerminalUI::Screen::set_face (face=..., writer=...) at terminal_ui.cc:255 #17 operator() (line=..., __closure=) at terminal_ui.cc:326 #18 Kakoune::TerminalUI::Screen::output (force=force@entry=true, synchronized=, writer=...) at terminal_ui.cc:402 #19 Kakoune::TerminalUI::redraw (force=force@entry=true) at terminal_ui.cc:571 #20 Kakoune::TerminalUI::refresh (force=) at terminal_ui.cc:592 #21 Kakoune::Client::redraw_ifn () at client.cc:282 #22 Kakoune::ClientManager::redraw_clients () at client_manager.cc:232 #23 Kakoune::run_server (session=..., server_init=..., client_init=..., init_buffer="fish-rust/src/ast.rs", init_coord=..., flags=Kakoune::ServerFlags::None, ui_type=Kakoune::UIType::Terminal, debug_flags=, files=ArrayView = {...}) at main.cc:893 #24 main (argc=, argv=) at main.cc:1243 Thereafter, "TerminalUI::Screen::output" resumes and crashes due to a buffer overflow in "lines" which has been resized. --- src/file.cc | 10 +++++++--- src/file.hh | 5 +++-- src/terminal_ui.cc | 2 +- 3 files changed, 11 insertions(+), 6 deletions(-) diff --git a/src/file.cc b/src/file.cc index a2ad279b..58eb9284 100644 --- a/src/file.cc +++ b/src/file.cc @@ -252,13 +252,14 @@ bool regular_file_exists(StringView filename) (st.st_mode & S_IFMT) == S_IFREG; } +template void write(int fd, StringView data) { const char* ptr = data.data(); ssize_t count = (int)data.length(); int flags = fcntl(fd, F_GETFL, 0); - if (EventManager::has_instance()) + if (not atomic and EventManager::has_instance()) fcntl(fd, F_SETFL, flags | O_NONBLOCK); auto restore_flags = on_scope_end([&] { fcntl(fd, F_SETFL, flags); }); @@ -269,12 +270,15 @@ void write(int fd, StringView data) ptr += written; count -= written; } - else if (errno == EAGAIN and EventManager::has_instance()) + else if (errno == EAGAIN and not atomic and EventManager::has_instance()) EventManager::instance().handle_next_events(EventMode::Urgent, nullptr, false); else throw file_access_error(format("fd: {}", fd), strerror(errno)); } } +template void write(int fd, StringView data); +template void write(int fd, StringView data); + void write_to_file(StringView filename, StringView data) { @@ -295,7 +299,7 @@ void write_buffer_to_fd(Buffer& buffer, int fd) eoldata = "\n"; - BufferedWriter writer{fd}; + BufferedWriter writer{fd}; if (buffer.options()["BOM"].get() == ByteOrderMark::Utf8) writer.write("\xEF\xBB\xBF"); diff --git a/src/file.hh b/src/file.hh index 1d9391fb..65abd3c8 100644 --- a/src/file.hh +++ b/src/file.hh @@ -39,6 +39,7 @@ bool fd_readable(int fd); bool fd_writable(int fd); String read_fd(int fd, bool text = false); String read_file(StringView filename, bool text = false); +template void write(int fd, StringView data); void write_to_file(StringView filename, StringView data); @@ -121,7 +122,7 @@ CandidateList complete_filename(StringView prefix, const Regex& ignore_regex, CandidateList complete_command(StringView prefix, ByteCount cursor_pos = -1); -template +template struct BufferedWriter { BufferedWriter(int fd) @@ -149,7 +150,7 @@ struct BufferedWriter void flush() { - Kakoune::write(m_fd, {m_buffer, m_pos}); + Kakoune::write(m_fd, {m_buffer, m_pos}); m_pos = 0; } diff --git a/src/terminal_ui.cc b/src/terminal_ui.cc index d411dd67..9685129a 100644 --- a/src/terminal_ui.cc +++ b/src/terminal_ui.cc @@ -208,7 +208,7 @@ void TerminalUI::Window::draw(DisplayCoord pos, lines[(int)pos.line].append({}, size.column - pos.column, default_face); } -struct Writer : BufferedWriter<> +struct Writer : BufferedWriter { using Writer::BufferedWriter::BufferedWriter; ~Writer() noexcept(false) = default;