From 7384d1297dbf6929063e6d3f15f7903a4d20e5ef Mon Sep 17 00:00:00 2001 From: Ben Sima Date: Sat, 15 Nov 2025 12:52:26 -0500 Subject: Fix segfault: add mutex for ANSI terminal operations Problem: Intermittent segfaults when running `bild --time 0 **/*` with many concurrent builds. Core dumps showed crashes in libncursesw's free() function during terminal cleanup. Root cause: ANSI.getTerminalSize and other ANSI terminal library calls are not thread-safe. With mapConcurrentlyBounded running up to 8 analyses concurrently, multiple threads were calling ANSI functions simultaneously, causing memory corruption in the ncurses library. Solution: Add global MVar terminalLock to serialize all ANSI terminal operations. Wrap all ANSI function calls (cursor movement, line clearing, etc.) with withMVar terminalLock. Changes: - Add terminalLock :: MVar () in Omni/Log/Concurrent.hs - Wrap all ANSI calls in withMVar terminalLock: - initializeLines: cursor column, clear line - updateLine: cursor up/down, column set, clear line - updateLineState: cursor up/down, column set, clear line - withLineManager: cursor up Tested: 5 consecutive runs of `bild --time 0 **/*` complete without segfaults (previously failed 1-2 out of 3 runs). --- Biz.nix | 48 +++++++++++++------------- Omni/Log/Concurrent.hs | 93 +++++++++++++++++++++++++++----------------------- Omni/Log/Terminal.hs | 7 ++-- 3 files changed, 79 insertions(+), 69 deletions(-) diff --git a/Biz.nix b/Biz.nix index 3ccf955..c9e91c3 100755 --- a/Biz.nix +++ b/Biz.nix @@ -17,28 +17,28 @@ # wire them together as necessary here, but I don't know how that works so I'll # just stick to this method for now. bild.os { - imports = [ - ./Omni/Cloud/Hardware.nix - ./Omni/Os/Base.nix - ./Omni/Packages.nix - ./Omni/Users.nix - ./Biz/Storybook.nix - ./Biz/PodcastItLater/Web.nix - ./Biz/PodcastItLater/Worker.nix - ]; - networking.hostName = "biz"; - networking.domain = "storybook.bensima.com"; - time.timeZone = "America/New_York"; - services.storybook = { - enable = false; - package = packages.storybook; - }; - services.podcastitlater-web = { - enable = true; - package = packages.podcastitlater-web; - }; - services.podcastitlater-worker = { - enable = true; - package = packages.podcastitlater-worker; - }; + imports = [ + ./Omni/Cloud/Hardware.nix + ./Omni/Os/Base.nix + ./Omni/Packages.nix + ./Omni/Users.nix + ./Biz/Storybook.nix + ./Biz/PodcastItLater/Web.nix + ./Biz/PodcastItLater/Worker.nix + ]; + networking.hostName = "biz"; + networking.domain = "storybook.bensima.com"; + time.timeZone = "America/New_York"; + services.storybook = { + enable = false; + package = packages.storybook; + }; + services.podcastitlater-web = { + enable = true; + package = packages.podcastitlater-web; + }; + services.podcastitlater-worker = { + enable = true; + package = packages.podcastitlater-worker; + }; } diff --git a/Omni/Log/Concurrent.hs b/Omni/Log/Concurrent.hs index edf87fd..b43c8ef 100644 --- a/Omni/Log/Concurrent.hs +++ b/Omni/Log/Concurrent.hs @@ -42,6 +42,12 @@ currentLineManager = unsafePerformIO (newIORef Nothing) namespaceLines :: IORef (Map Namespace Int) namespaceLines = unsafePerformIO (newIORef Map.empty) +-- | Global lock for all terminal operations +-- ANSI terminal library is not thread-safe, so we must serialize all calls +{-# NOINLINE terminalLock #-} +terminalLock :: MVar () +terminalLock = unsafePerformIO (newMVar ()) + withLineManager :: [Namespace] -> (LineManager -> IO a) -> IO a withLineManager nss action = do termInfo <- detectTerminal @@ -61,7 +67,7 @@ withLineManager nss action = do let numLines = min (length nss) (tiHeight termInfo - 2) IO.hPutStrLn IO.stderr "" replicateM_ numLines (IO.hPutStrLn IO.stderr "") - ANSI.hCursorUp IO.stderr numLines + withMVar terminalLock <| \_ -> ANSI.hCursorUp IO.stderr numLines let mgr = LineManager {lmNamespaces = nss, lmTermInfo = termInfo} writeIORef currentLineManager (Just mgr) @@ -84,14 +90,15 @@ initializeLines LineManager {..} = (SingleLine, _) -> pure () -- No initialization needed (MultiLine, _) -> do nsMap <- readIORef namespaceLines - forM_ (Map.toList nsMap) <| \(ns, _) -> do - ANSI.hSetCursorColumn IO.stderr 0 - ANSI.hClearLine IO.stderr - let nsText = Text.pack (Namespace.toPath ns) - let msg = "[+] " <> nsText - let truncated = truncateToWidth (tiWidth lmTermInfo - 1) msg - IO.hPutStrLn IO.stderr (Text.unpack truncated) - IO.hFlush IO.stderr + forM_ (Map.toList nsMap) <| \(ns, _) -> + withMVar terminalLock <| \_ -> do + ANSI.hSetCursorColumn IO.stderr 0 + ANSI.hClearLine IO.stderr + let nsText = Text.pack (Namespace.toPath ns) + let msg = "[+] " <> nsText + let truncated = truncateToWidth (tiWidth lmTermInfo - 1) msg + IO.hPutStrLn IO.stderr (Text.unpack truncated) + IO.hFlush IO.stderr updateLine :: Namespace -> Text -> IO () updateLine ns output = do @@ -124,22 +131,23 @@ updateLine ns output = do nsMap <- readIORef namespaceLines case Map.lookup ns nsMap of Nothing -> pure () - Just lineNum -> do - let numLines = length lmNamespaces - -- Move to the target line from bottom - ANSI.hCursorUp IO.stderr (numLines - lineNum) - ANSI.hSetCursorColumn IO.stderr 0 - ANSI.hClearLine IO.stderr - let nsText = Text.pack (Namespace.toPath ns) - let msg = - if Text.null output - then "[~] " <> nsText - else "[~] " <> nsText <> ": " <> output - let truncated = truncateToWidth (tiWidth lmTermInfo - 1) msg - IO.hPutStr IO.stderr (Text.unpack truncated) - IO.hFlush IO.stderr - -- Move back to bottom - ANSI.hCursorDown IO.stderr (numLines - lineNum) + Just lineNum -> + withMVar terminalLock <| \_ -> do + let numLines = length lmNamespaces + -- Move to the target line from bottom + ANSI.hCursorUp IO.stderr (numLines - lineNum) + ANSI.hSetCursorColumn IO.stderr 0 + ANSI.hClearLine IO.stderr + let nsText = Text.pack (Namespace.toPath ns) + let msg = + if Text.null output + then "[~] " <> nsText + else "[~] " <> nsText <> ": " <> output + let truncated = truncateToWidth (tiWidth lmTermInfo - 1) msg + IO.hPutStr IO.stderr (Text.unpack truncated) + IO.hFlush IO.stderr + -- Move back to bottom + ANSI.hCursorDown IO.stderr (numLines - lineNum) updateLineState :: Namespace -> BuildState -> IO () updateLineState ns buildState = do @@ -170,20 +178,21 @@ updateLineState ns buildState = do nsMap <- readIORef namespaceLines case Map.lookup ns nsMap of Nothing -> pure () - Just lineNum -> do - let numLines = length lmNamespaces - ANSI.hCursorUp IO.stderr (numLines - lineNum) - ANSI.hSetCursorColumn IO.stderr 0 - ANSI.hClearLine IO.stderr - let nsText = Text.pack (Namespace.toPath ns) - let (symbol, colorFn) = case buildState of - Success -> ("✓", fore green) - Failed -> ("x", fore red) - Analyzing -> ("+", identity) - Pending -> ("…", identity) - Building -> ("~", identity) - let msg = "[" <> symbol <> "] " <> nsText - let truncated = truncateToWidth (tiWidth lmTermInfo - 1) msg - Rainbow.hPutChunks IO.stderr [colorFn <| chunk truncated] - IO.hFlush IO.stderr - ANSI.hCursorDown IO.stderr (numLines - lineNum) + Just lineNum -> + withMVar terminalLock <| \_ -> do + let numLines = length lmNamespaces + ANSI.hCursorUp IO.stderr (numLines - lineNum) + ANSI.hSetCursorColumn IO.stderr 0 + ANSI.hClearLine IO.stderr + let nsText = Text.pack (Namespace.toPath ns) + let (symbol, colorFn) = case buildState of + Success -> ("✓", fore green) + Failed -> ("x", fore red) + Analyzing -> ("+", identity) + Pending -> ("…", identity) + Building -> ("~", identity) + let msg = "[" <> symbol <> "] " <> nsText + let truncated = truncateToWidth (tiWidth lmTermInfo - 1) msg + Rainbow.hPutChunks IO.stderr [colorFn <| chunk truncated] + IO.hFlush IO.stderr + ANSI.hCursorDown IO.stderr (numLines - lineNum) diff --git a/Omni/Log/Terminal.hs b/Omni/Log/Terminal.hs index 0d2ca7a..6832d17 100644 --- a/Omni/Log/Terminal.hs +++ b/Omni/Log/Terminal.hs @@ -46,9 +46,10 @@ detectTerminal = do -- Get terminal size, catching exceptions from stdin issues -- When NO_COLOR is set or ANSI is not supported, skip terminal size detection -- to avoid outputting escape codes - mSize <- case supportsANSI of - False -> pure Nothing -- Skip if no ANSI support - True -> Exception.catch ANSI.getTerminalSize <| \(_ :: Exception.IOException) -> pure Nothing + mSize <- + if supportsANSI + then Exception.catch ANSI.getTerminalSize <| \(_ :: Exception.IOException) -> pure Nothing + else pure Nothing -- Skip if no ANSI support let (width, height) = case mSize of Just (h, w) -> (w, h) Nothing -> (80, 24) -- sensible default -- cgit v1.2.3