Skip to content

Commit

Permalink
Rename FISH_TSAN_WORKAROUNDS and add feature to Cargo.toml
Browse files Browse the repository at this point in the history
rustc 1.80 now complains about features not declared in Cargo.toml and cfg
keys/values not declared by build.rs to protect against typos or misuse (you
think you're using the right condition but you're not). See
rust-lang/cargo#10554 and rust-lang/rust#82450.

(We're not actually using TSAN under CI at this time, but I do want to re-enable
it at some point — especially if we get multithreaded execution going — using
the rust-native TSAN configuration.)

I'll be updating the `rsconf` crate and patching `build.rs` accordingly to also
handle the warnings about unknown cfg values, but tsan is a feature and not a
cfg and these can be dealt with in `Cargo.toml` directly.
  • Loading branch information
mqudsi committed May 9, 2024
1 parent 059b842 commit 35a16e3
Show file tree
Hide file tree
Showing 4 changed files with 6 additions and 5 deletions.
1 change: 1 addition & 0 deletions Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,7 @@ benchmark = []

# The following features are auto-detected by the build-script and should not be enabled manually.
asan = []
tsan = []

[lints]
rust.non_camel_case_types = "allow"
Expand Down
4 changes: 2 additions & 2 deletions src/parse_constants.rs
Original file line number Diff line number Diff line change
Expand Up @@ -442,9 +442,9 @@ pub const FISH_MAX_STACK_DEPTH: usize = 128;

/// Maximum number of nested string substitutions (in lieu of evals)
/// Reduced under TSAN: our CI test creates 500 jobs and this is very slow with TSAN.
#[cfg(feature = "FISH_TSAN_WORKAROUNDS")]
#[cfg(feature = "tsan")]
pub const FISH_MAX_EVAL_DEPTH: usize = 250;
#[cfg(not(feature = "FISH_TSAN_WORKAROUNDS"))]
#[cfg(not(feature = "tsan"))]
pub const FISH_MAX_EVAL_DEPTH: usize = 500;

/// Error message on a function that calls itself immediately.
Expand Down
2 changes: 1 addition & 1 deletion src/signal.rs
Original file line number Diff line number Diff line change
Expand Up @@ -230,7 +230,7 @@ pub fn signal_set_handlers(interactive: bool) {
set_interactive_handlers();
}

if cfg!(feature = "FISH_TSAN_WORKAROUNDS") {
if cfg!(feature = "tsan") {
// Work around the following TSAN bug:
// The structure containing signal information for a thread is lazily allocated by TSAN.
// It is possible for the same thread to receive two allocations, if the signal handler
Expand Down
4 changes: 2 additions & 2 deletions src/topic_monitor.rs
Original file line number Diff line number Diff line change
Expand Up @@ -187,7 +187,7 @@ impl binary_semaphore_t {
// a signal call, so if we're blocked in read() (like the topic monitor wants to be!),
// we'll never receive SIGCHLD and so deadlock. So if tsan is enabled, we mark our fd as
// non-blocking (so reads will never block) and use select() to poll it.
if cfg!(feature = "FISH_TSAN_WORKAROUNDS") {
if cfg!(feature = "tsan") {
let _ = make_fd_nonblocking(pipes.read.as_raw_fd());
}

Expand Down Expand Up @@ -239,7 +239,7 @@ impl binary_semaphore_t {
// Under tsan our notifying pipe is non-blocking, so we would busy-loop on the read()
// call until data is available (that is, fish would use 100% cpu while waiting for
// processes). This call prevents that.
if cfg!(feature = "FISH_TSAN_WORKAROUNDS") {
if cfg!(feature = "tsan") {
let _ =
fd_readable_set_t::is_fd_readable(fd, fd_readable_set_t::kNoTimeout);
}
Expand Down

0 comments on commit 35a16e3

Please sign in to comment.