Race condition between `has_manual_override` and `manual_override`

[meng-xu-cs]

NOTE: Possible duplicate of #101 / #201 (but hopefully this issue is closer to the root cause of the symptoms shown in previous issues).

Summary

ShouldColorize stores the manual override as two separate atomics:

has_manual_override: AtomicBool,
manual_override: AtomicBool,

and currently publishes them in this order:

pub fn set_override(&self, override_colorize: bool) {
    self.has_manual_override.store(true, Ordering::Relaxed);
    self.manual_override.store(override_colorize, Ordering::Relaxed);
}

while should_colorize() consumes them in this order:

pub fn should_colorize(&self) -> bool {
    if self.has_manual_override.load(Ordering::Relaxed) {
        return self.manual_override.load(Ordering::Relaxed);
    }
    // ...
}

A legal interleaving is:

1. Initial state: has_manual_override = false, manual_override = false
2. Thread A calls set_override(true) and executes has_manual_override.store(true, ...)
3. Thread B calls should_colorize(), sees has_manual_override == true, then reads manual_override == false
4. Thread A then executes manual_override.store(true, ...)

So should_colorize() can return the stale value even with one writer and one reader, without any competing set_override() calls.

ColoredString::has_colors() calls control::SHOULD_COLORIZE.should_colorize() on the normal formatting path, so the stale result directly affects whether ANSI escapes are emitted.

Expected behavior

Once a thread observes has_manual_override == true, it should also observe the override value that was meant to be published with that flag.

Actual behavior

A formatting thread can observe has_manual_override == true together with the previous manual_override value and make the opposite colorization decision.

Impact

This can transiently:

• suppress ANSI escapes even though set_override(true) was requested, or
• emit ANSI escapes even though set_override(false) was requested.

This is never meant to be a serious bug, just causing some confusions though.

Suggested fix

Publish the value first and then publish the flag with release semantics:

pub fn set_override(&self, override_colorize: bool) {
    self.manual_override.store(override_colorize, Ordering::Relaxed);
    self.has_manual_override.store(true, Ordering::Release);
}

pub fn should_colorize(&self) -> bool {
    if self.has_manual_override.load(Ordering::Acquire) {
        return self.manual_override.load(Ordering::Relaxed);
    }

    if let Some(forced_value) = self.clicolor_force {
        return forced_value;
    }

    self.clicolor
}

Another option would be to replace the pair with a single atomic enum / bitfield representing the full state (Auto, ForcedTrue, ForcedFalse), which avoids cross-atomic coherence issues entirely.