diff options
| author | Ben Sima <ben@bsima.me> | 2025-11-15 13:39:15 -0500 |
|---|---|---|
| committer | Ben Sima <ben@bsima.me> | 2025-11-15 13:39:15 -0500 |
| commit | 48afd6bdb135177842af593c7f96846b9ddab7d8 (patch) | |
| tree | b1be373909d90e17c9ab158f365ff8bf11f5b9ca /Omni/Log/Concurrent.hs | |
| parent | d2253e15dd405b3fbf15663aa02ab1f5ffa56306 (diff) | |
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.
Diffstat (limited to 'Omni/Log/Concurrent.hs')
| -rw-r--r-- | Omni/Log/Concurrent.hs | 94 |
1 files changed, 50 insertions, 44 deletions
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 |
