From 48afd6bdb135177842af593c7f96846b9ddab7d8 Mon Sep 17 00:00:00 2001 From: Ben Sima Date: Sat, 15 Nov 2025 13:39:15 -0500 Subject: Expand terminalLock scope to protect all terminal I/O Problem: Still getting segfaults ("free(): invalid pointer") even with ncurses calls protected. The mutex only covered ANSI calls, but IORef reads and IO.hPutStr operations were happening outside the lock. Root cause: Race conditions in concurrent terminal output: 1. Multiple threads reading namespaceLines IORef concurrently 2. Interleaved IO.hPutStr calls corrupting output buffer 3. Rainbow.hPutChunks not being thread-safe Solution: Expand terminalLock scope to cover entire output operations: - Move IORef reads inside the lock - Protect all IO.hPutStr and Rainbow.hPutChunks calls - Lock both SingleLine and MultiLine modes - Ensure atomicity from IORef read through all I/O to flush This makes each updateLine/updateLineState call atomic, preventing any interleaving of terminal operations between threads. Changes: - updateLine SingleLine: wrap entire output in withMVar terminalLock - updateLine MultiLine: move nsMap read inside lock - updateLineState SingleLine: wrap entire output in withMVar terminalLock - updateLineState MultiLine: move nsMap read inside lock Tested: 10/10 successful runs of `bild --time 0 **/*` without segfaults. --- Omni/Log/Concurrent.hs | 94 +++++++++++++++++++++++++++----------------------- 1 file changed, 50 insertions(+), 44 deletions(-) (limited to 'Omni/Log/Concurrent.hs') diff --git a/Omni/Log/Concurrent.hs b/Omni/Log/Concurrent.hs index d4969eb..8c48f83 100644 --- a/Omni/Log/Concurrent.hs +++ b/Omni/Log/Concurrent.hs @@ -110,30 +110,33 @@ updateLine ns output = do IO.hFlush IO.stderr Just LineManager {..} -> case tiMode lmTermInfo of - SingleLine -> do + SingleLine -> -- Single line: update in place - 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 - -- Clear line and write - IO.hPutStr IO.stderr "\r" - IO.hPutStr IO.stderr (Text.unpack truncated) - -- Pad to clear previous longer text - let padding = replicate (tiWidth lmTermInfo - Text.length truncated - 1) ' ' - IO.hPutStr IO.stderr padding - IO.hPutStr IO.stderr "\r" - IO.hPutStr IO.stderr (Text.unpack truncated) - IO.hFlush IO.stderr - MultiLine -> do + -- Lock all terminal output to prevent interleaved writes + withMVar terminalLock <| \_ -> do + 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 + -- Clear line and write + IO.hPutStr IO.stderr "\r" + IO.hPutStr IO.stderr (Text.unpack truncated) + -- Pad to clear previous longer text + let padding = replicate (tiWidth lmTermInfo - Text.length truncated - 1) ' ' + IO.hPutStr IO.stderr padding + IO.hPutStr IO.stderr "\r" + IO.hPutStr IO.stderr (Text.unpack truncated) + IO.hFlush IO.stderr + MultiLine -> -- Multi-line: use reserved lines with truncation - nsMap <- readIORef namespaceLines - case Map.lookup ns nsMap of - Nothing -> pure () - Just lineNum -> - withMVar terminalLock <| \_ -> do + -- Lock covers IORef read + all terminal operations to prevent races + withMVar terminalLock <| \_ -> 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) @@ -157,30 +160,33 @@ updateLineState ns buildState = do Nothing -> pure () Just LineManager {..} -> case tiMode lmTermInfo of - SingleLine -> do + SingleLine -> -- Single line: show completion, keep visible for success/failure - let nsText = Text.pack (Namespace.toPath ns) - let (symbol, color) = case buildState of - Success -> ("✓", green) - Failed -> ("x", red) - _ -> ("~", white) - let msg = "[" <> symbol <> "] " <> nsText - let truncated = truncateToWidth (tiWidth lmTermInfo - 1) msg - - IO.hPutStr IO.stderr "\r" - Rainbow.hPutChunks IO.stderr [fore color <| chunk truncated] - case buildState of - Success -> IO.hPutStrLn IO.stderr "" -- Keep successes visible - Failed -> IO.hPutStrLn IO.stderr "" -- Keep failures visible - _ -> pure () -- Transient states overwrite - IO.hFlush IO.stderr - MultiLine -> do + -- Lock all terminal output to prevent interleaved writes + withMVar terminalLock <| \_ -> do + let nsText = Text.pack (Namespace.toPath ns) + let (symbol, color) = case buildState of + Success -> ("✓", green) + Failed -> ("x", red) + _ -> ("~", white) + let msg = "[" <> symbol <> "] " <> nsText + let truncated = truncateToWidth (tiWidth lmTermInfo - 1) msg + + IO.hPutStr IO.stderr "\r" + Rainbow.hPutChunks IO.stderr [fore color <| chunk truncated] + case buildState of + Success -> IO.hPutStrLn IO.stderr "" -- Keep successes visible + Failed -> IO.hPutStrLn IO.stderr "" -- Keep failures visible + _ -> pure () -- Transient states overwrite + IO.hFlush IO.stderr + MultiLine -> -- Multi-line: use reserved lines with truncation - nsMap <- readIORef namespaceLines - case Map.lookup ns nsMap of - Nothing -> pure () - Just lineNum -> - withMVar terminalLock <| \_ -> do + -- Lock covers IORef read + all terminal operations to prevent races + withMVar terminalLock <| \_ -> 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 -- cgit v1.2.3