diff --git a/src/output.rs b/src/output.rs index 33d9fcf78..35554b35c 100644 --- a/src/output.rs +++ b/src/output.rs @@ -3,6 +3,7 @@ use crate::color::RgbColor; use crate::common::{self, wcs2string_appending}; use crate::curses::{self, tparm1, Term}; use crate::env::EnvVar; +use crate::threads::MainThread; use crate::wchar::prelude::*; use bitflags::bitflags; use std::cell::RefCell; @@ -444,14 +445,10 @@ impl Outputter { /// Access the outputter for stdout. /// This should only be used from the main thread. - pub fn stdoutput() -> &'static mut RefCell { - crate::threads::assert_is_main_thread(); - static mut STDOUTPUT: RefCell = - RefCell::new(Outputter::new_from_fd(libc::STDOUT_FILENO)); - // Safety: this is only called from the main thread. - // XXX: creating and using multiple (read or write!) references to the same mutable static - // is undefined behavior! - unsafe { &mut STDOUTPUT } + pub fn stdoutput() -> &'static RefCell { + static STDOUTPUT: MainThread> = + MainThread::new(RefCell::new(Outputter::new_from_fd(libc::STDOUT_FILENO))); + STDOUTPUT.get() } } diff --git a/src/parser.rs b/src/parser.rs index 923306eea..eeec59299 100644 --- a/src/parser.rs +++ b/src/parser.rs @@ -27,7 +27,7 @@ use crate::parse_execution::{EndExecutionReason, ParseExecutionContext}; use crate::parse_tree::{parse_source, ParsedSourceRef}; use crate::proc::{job_reap, JobGroupRef, JobList, JobRef, ProcStatus}; use crate::signal::{signal_check_cancel, signal_clear_cancel, Signal}; -use crate::threads::assert_is_main_thread; +use crate::threads::{assert_is_main_thread, MainThread}; use crate::util::get_time; use crate::wait_handle::WaitHandleStore; use crate::wchar::{wstr, WString, L}; @@ -414,19 +414,11 @@ impl Parser { false } - /// Get the "principal" parser, whatever that is. + /// Get the "principal" parser, whatever that is. Can only be called by the main thread. pub fn principal_parser() -> &'static Parser { - // XXX: We use `static mut` as a hack to work around the fact that Parser doesn't implement - // Sync! Even though we are wrapping it in Lazy<> and it compiles without an error, that - // doesn't mean this is safe to access across threads! - static mut PRINCIPAL: Lazy = - Lazy::new(|| Parser::new(EnvStack::principal().clone(), true)); - // XXX: Creating and using multiple (read or write!) references to the same mutable static - // is undefined behavior! - unsafe { - PRINCIPAL.assert_can_execute(); - &PRINCIPAL - } + static PRINCIPAL: Lazy> = + Lazy::new(|| MainThread::new(Parser::new(EnvStack::principal().clone(), true))); + PRINCIPAL.get() } /// Assert that this parser is allowed to execute on the current thread. diff --git a/src/reader.rs b/src/reader.rs index c44514087..12aa9bcdf 100644 --- a/src/reader.rs +++ b/src/reader.rs @@ -1906,7 +1906,7 @@ impl ReaderData { perror("tcsetattr"); // return to previous mode } Outputter::stdoutput() - .get_mut() + .borrow_mut() .set_color(RgbColor::RESET, RgbColor::RESET); } rls.finished.then(|| zelf.command_line.text().to_owned()) @@ -2946,8 +2946,9 @@ impl ReaderData { el.end_edit_group(); } rl::DisableMouseTracking => { - let outp = Outputter::stdoutput().get_mut(); - outp.write_wstr(L!("\x1B[?1000l")); + Outputter::stdoutput() + .borrow_mut() + .write_wstr(L!("\x1B[?1000l")); } rl::ClearScreenAndRepaint => { self.parser().libdata_mut().pods.is_repaint = true; @@ -2958,8 +2959,7 @@ impl ReaderData { // and *then* reexecute the prompt and overdraw it. // This removes the flicker, // while keeping the prompt up-to-date. - let outp = Outputter::stdoutput().get_mut(); - outp.write_wstr(&clear); + Outputter::stdoutput().borrow_mut().write_wstr(&clear); self.screen.reset_line(/*repaint_prompt=*/ true); self.layout_and_repaint(L!("readline")); } @@ -3492,7 +3492,7 @@ fn reader_interactive_init(parser: &Parser) { /// Destroy data for interactive use. fn reader_interactive_destroy() { Outputter::stdoutput() - .get_mut() + .borrow_mut() .set_color(RgbColor::RESET, RgbColor::RESET); } @@ -3575,7 +3575,7 @@ pub fn reader_write_title( } Outputter::stdoutput() - .get_mut() + .borrow_mut() .set_color(RgbColor::RESET, RgbColor::RESET); if reset_cursor_position && !lst.is_empty() { // Put the cursor back at the beginning of the line (issue #2453). @@ -4587,9 +4587,10 @@ fn reader_run_command(parser: &Parser, cmd: &wstr) -> EvalRes { .set_one(L!("_"), EnvMode::GLOBAL, ft.to_owned()); } - let outp = Outputter::stdoutput().get_mut(); reader_write_title(cmd, parser, true); - outp.set_color(RgbColor::NORMAL, RgbColor::NORMAL); + Outputter::stdoutput() + .borrow_mut() + .set_color(RgbColor::NORMAL, RgbColor::NORMAL); term_donate(false); let time_before = Instant::now(); diff --git a/src/screen.rs b/src/screen.rs index 4ed5b51f0..4458fae8c 100644 --- a/src/screen.rs +++ b/src/screen.rs @@ -8,6 +8,7 @@ //! of text around to handle text insertion. use crate::pager::{PageRendering, Pager}; +use std::cell::RefCell; use std::collections::LinkedList; use std::ffi::{CStr, CString}; use std::io::Write; @@ -176,7 +177,7 @@ pub struct Screen { pub autosuggestion_is_truncated: bool, /// Receiver for our output. - outp: &'static mut Outputter, + outp: &'static RefCell, /// The internal representation of the desired screen contents. desired: ScreenData, @@ -208,7 +209,7 @@ pub struct Screen { impl Screen { pub fn new() -> Self { Self { - outp: Outputter::stdoutput().get_mut(), + outp: Outputter::stdoutput(), autosuggestion_is_truncated: Default::default(), desired: Default::default(), actual: Default::default(), @@ -637,9 +638,9 @@ impl Screen { // Either issue a cr to go back to the beginning of this line, or a nl to go to the // beginning of the next one, depending on what we think is more efficient. if new_y <= zelf.actual.cursor.y { - zelf.outp.push(b'\r'); + zelf.outp.borrow_mut().push(b'\r'); } else { - zelf.outp.push(b'\n'); + zelf.outp.borrow_mut().push(b'\n'); zelf.actual.cursor.y += 1; } // Either way we're not in the first column. @@ -674,13 +675,13 @@ impl Screen { }; for _ in 0..y_steps.abs_diff(0) { - zelf.outp.tputs_if_some(&s); + zelf.outp.borrow_mut().tputs_if_some(&s); } let mut x_steps = isize::try_from(new_x).unwrap() - isize::try_from(zelf.actual.cursor.x).unwrap(); if x_steps != 0 && new_x == 0 { - zelf.outp.push(b'\r'); + zelf.outp.borrow_mut().push(b'\r'); x_steps = 0; } @@ -700,10 +701,10 @@ impl Screen { multi_str.as_ref().unwrap(), i32::try_from(x_steps.abs_diff(0)).unwrap(), ); - zelf.outp.tputs_if_some(&multi_param); + zelf.outp.borrow_mut().tputs_if_some(&multi_param); } else { for _ in 0..x_steps.abs_diff(0) { - zelf.outp.tputs_if_some(&s); + zelf.outp.borrow_mut().tputs_if_some(&s); } } @@ -715,7 +716,7 @@ impl Screen { fn write_char(&mut self, c: char, width: isize) { let mut zelf = self.scoped_buffer(); zelf.actual.cursor.x = zelf.actual.cursor.x.wrapping_add(width as usize); - zelf.outp.writech(c); + zelf.outp.borrow_mut().writech(c); if Some(zelf.actual.cursor.x) == zelf.actual.screen_width && allow_soft_wrap() { zelf.soft_wrap_location = Some(Cursor { x: 0, @@ -732,16 +733,16 @@ impl Screen { /// Send the specified string through tputs and append the output to the screen's outputter. fn write_mbs(&mut self, s: &CStr) { - self.outp.tputs(s) + self.outp.borrow_mut().tputs(s); } fn write_mbs_if_some(&mut self, s: &Option>) -> bool { - self.outp.tputs_if_some(s) + self.outp.borrow_mut().tputs_if_some(s) } /// Convert a wide string to a multibyte string and append it to the buffer. fn write_str(&mut self, s: &wstr) { - self.outp.write_wstr(s) + self.outp.borrow_mut().write_wstr(s); } /// Update the cursor as if soft wrapping had been performed. @@ -766,9 +767,9 @@ impl Screen { } fn scoped_buffer(&mut self) -> impl ScopeGuarding { - self.outp.begin_buffering(); + self.outp.borrow_mut().begin_buffering(); ScopeGuard::new(self, |zelf| { - zelf.outp.end_buffering(); + zelf.outp.borrow_mut().end_buffering(); }) } @@ -779,7 +780,7 @@ impl Screen { let mut set_color = |zelf: &mut Self, c| { let fg = color_resolver.resolve_spec(&c, false, vars); let bg = color_resolver.resolve_spec(&c, true, vars); - zelf.outp.set_color(fg, bg); + zelf.outp.borrow_mut().set_color(fg, bg); }; let mut cached_layouts = LAYOUT_CACHE_SHARED.lock().unwrap(); @@ -835,6 +836,7 @@ impl Screen { for line_break in left_prompt_layout.line_breaks { zelf.write_str(&left_prompt[start..line_break]); zelf.outp + .borrow_mut() .tputs_if_some(&term.and_then(|term| term.clr_eol.as_ref())); start = line_break; } @@ -1068,7 +1070,7 @@ impl Screen { /// Issues an immediate clr_eos. pub fn screen_force_clear_to_end() { Outputter::stdoutput() - .get_mut() + .borrow_mut() .tputs_if_some(&term().unwrap().clr_eos); } diff --git a/src/threads.rs b/src/threads.rs index f0c80e01d..0c5d27708 100644 --- a/src/threads.rs +++ b/src/threads.rs @@ -4,6 +4,7 @@ use crate::flog::{FloggableDebug, FLOG}; use crate::reader::ReaderData; use once_cell::race::OnceBox; +use std::marker::PhantomData; use std::num::NonZeroU64; use std::sync::atomic::{AtomicBool, Ordering}; use std::sync::{Arc, Mutex}; @@ -358,6 +359,33 @@ impl ThreadPool { } } +/// A `Sync` and `Send` wrapper for non-`Sync`/`Send` types. +/// Only allows access from the main thread. +pub struct MainThread { + data: T, + // Make type !Send and !Sync by default + _marker: PhantomData<*const ()>, +} + +// Manually implement Send and Sync for MainThread to ensure it can be shared across threads +// as long as T is 'static. +unsafe impl Send for MainThread {} +unsafe impl Sync for MainThread {} + +impl MainThread { + pub const fn new(value: T) -> Self { + Self { + data: value, + _marker: PhantomData, + } + } + + pub fn get(&self) -> &T { + assert_is_main_thread(); + &self.data + } +} + pub struct WorkerThread { /// The data shared with the [`ThreadPool`]. shared: Arc,