summaryrefslogtreecommitdiffhomepage
diff options
context:
space:
mode:
authorTomas Janousek <tomi@nomi.cz>2020-11-14 12:25:58 +0000
committerjao <jao@gnu.org>2020-11-19 13:38:15 +0000
commitb23fc95a3bc2c0fefba7a44d6f1a4e8e0941d77c (patch)
tree2a0aa316ea0c39f8749cd81a55fb51ed5647001f
parente2d88d6de8447de2e8a7d3573fe4c33e6f01ee60 (diff)
downloadxmobar-b23fc95a3bc2c0fefba7a44d6f1a4e8e0941d77c.tar.gz
xmobar-b23fc95a3bc2c0fefba7a44d6f1a4e8e0941d77c.tar.bz2
StdinReader: Improve exception handling
This corrects a misleading comment "EOF check is necessary for certain systems" which was added without complete understanding of the root cause of #442. That issue was in fact caused by old xmobars not being terminated on early EOF, and is thus necessary on _all_ systems that rely on EOF to terminate old xmobar before starting a new one. (To trigger the bug, one additionally needs to close the xmobar pipe before sending any input to it, which is unusual, but incorrectly configured xmonad might trigger that.) Furthermore, this fixes another execution path that could lead to xmobar not being terminated on EOF: `echo -e '\xff' | xmobar -c '[Run StdinReader]' -t '%StdinReader%'` would terminate the StdinReader thread upon catching the "invalid argument (invalid byte sequence)" so there'd be no thread to detect the subsequent EOF and xmobar would get stuck. Additionally, I believe that terminating either the thread or the entire xmobar upon receiving a single miscoded byte isn't desirable, as this might be an intermittent issue and another input line can be perfectly okay. Therefore I suggest that the original issue @psibi was trying to fix by b7a3d6745817 is worked around by introducing a throttling delay instead of terminating the thread, as I assume that exceptions other than async and EOF are recoverable. Fixes: b7a3d6745817 ("Avoid busy looping by not catching all exceptions") Fixes: 68ac4d3ae6f3 ("Update stderr and the bar on receiving exception") Fixes: ed0663aac942 ("Add EOF check before getLine operation from stdin") Fixes: https://github.com/jaor/xmobar/issues/442 Related: https://github.com/jaor/xmobar/pull/439 Related: https://github.com/jaor/xmobar/pull/448
-rw-r--r--src/Xmobar/Plugins/StdinReader.hs42
-rw-r--r--src/Xmobar/System/Utils.hs11
2 files changed, 25 insertions, 28 deletions
diff --git a/src/Xmobar/Plugins/StdinReader.hs b/src/Xmobar/Plugins/StdinReader.hs
index a29c1ad..4d5c438 100644
--- a/src/Xmobar/Plugins/StdinReader.hs
+++ b/src/Xmobar/Plugins/StdinReader.hs
@@ -1,3 +1,5 @@
+{-# LANGUAGE ViewPatterns #-}
+
-----------------------------------------------------------------------------
-- |
-- Module : Plugins.StdinReader
@@ -22,30 +24,36 @@ import Prelude
import System.Posix.Process
import System.Exit
import System.IO
+import System.IO.Error (isEOFError)
import Xmobar.Run.Exec
import Xmobar.X11.Actions (stripActions)
-import Xmobar.System.Utils (onSomeException)
-import Control.Monad (when)
+import Control.Concurrent (threadDelay)
+import Control.Exception
+import Control.Monad (forever)
data StdinReader = StdinReader | UnsafeStdinReader
deriving (Read, Show)
instance Exec StdinReader where
- start stdinReader cb = do
- -- The EOF check is necessary for certain systems
- -- More details here https://github.com/jaor/xmobar/issues/442
- eof <- isEOF
- when eof $
- do hPrint stderr "xmobar: eof at an early stage"
- exitImmediately ExitSuccess
- s <-
- getLine `onSomeException`
- (\e -> do
- let errorMessage = "xmobar: Received exception " <> show e
- hPrint stderr errorMessage
- cb errorMessage)
- cb $ escape stdinReader s
- start stdinReader cb
+ start stdinReader cb = forever $ (cb . escape stdinReader =<< getLine) `catch` handler
+ where
+ -- rethrow async exceptions like ThreadKilled, etc.
+ handler (fromException -> Just e) = throwIO (e :: SomeAsyncException)
+ -- XMonad.Hooks.DynamicLog.statusBar starts new xmobar on every xmonad
+ -- reload and the old xmobar is only signalled to exit via the pipe
+ -- being closed, so we must unconditionally terminate on EOF, otherwise
+ -- there'd be a pileup of xmobars
+ handler (fromException -> Just e) | isEOFError e = exitImmediately ExitSuccess
+ -- any other exception, like "invalid argument (invalid byte sequence)",
+ -- is logged to both stderr and the bar itself, throttled to avoid
+ -- excessive CPU usage whenever someone pipes garbage into xmobar, and
+ -- then discarded without terminating, so a single charset error doesn't
+ -- break the entire xmobar
+ handler e = do
+ let errorMessage = "xmobar: Received exception " <> show e
+ hPutStrLn stderr errorMessage
+ cb $ stripActions errorMessage
+ threadDelay 1000000
escape :: StdinReader -> String -> String
escape StdinReader = stripActions
diff --git a/src/Xmobar/System/Utils.hs b/src/Xmobar/System/Utils.hs
index 53052ea..24c655e 100644
--- a/src/Xmobar/System/Utils.hs
+++ b/src/Xmobar/System/Utils.hs
@@ -20,7 +20,6 @@
module Xmobar.System.Utils
( expandHome
, changeLoop
- , onSomeException
, safeIndex
) where
@@ -31,7 +30,6 @@ import Data.Maybe (fromMaybe)
import System.Environment
import System.FilePath
-import Control.Exception
expandHome :: FilePath -> IO FilePath
expandHome ('~':'/':path) = fmap (</> path) (getEnv "HOME")
@@ -47,15 +45,6 @@ changeLoop s f = atomically s >>= go
guard (new /= old)
return new)
--- | Like 'finally', but only performs the final action if there was an
--- exception raised by the computation.
---
--- Note that this implementation is a slight modification of
--- onException function.
-onSomeException :: IO a -> (SomeException -> IO b) -> IO a
-onSomeException io what = io `catch` \e -> do _ <- what e
- throwIO (e :: SomeException)
-
(!!?) :: [a] -> Int -> Maybe a
(!!?) xs i
| i < 0 = Nothing