Unsafe getWindowAttributes calls in ~20 modules

[SOwOphie]

I've been running into some xmonad crashes lately, but they've been pretty hard to isolate and reproduce. All of them quit with the same user error in getWindowAttributes message, so I've decided to grep for getWindowAttributes through both xmonad and xmonad-contrib. Here's what I've come up with:

• Multiple comments hint towards getWindowAttributes being an unsafe function. It is implemented as a wrapper around an unsafe foreign C function. However, its documentation doesn't mention anything. (Maybe I should open an issue there as well?)
• The xmonad code was mostly fine, only one unsafe call I fixed in Rare crash fix due to uncaught exception from getWindowAttributes xmonad#76.
• Out of 29 getWindowAttributes calls spread over 21 modules in xmonad-contrib, only two catch the exception (details further down below).

Considering the amount of changes necessary to safely wrap all those, I wanted to ask for advice on how to proceed before doing anything on my own.

P.S.: Sorry for not using your issue template, but it didn't seem appropriate for the kind of problem.

---

Sorted + filtered output of grep -Hn getWindowAttributes **/*.hs

GOOD:

XMonad/Actions/UpdatePointer.hs:71:        Just w  -> do tryAttributes <- io $ try $ getWindowAttributes dpy w
XMonad/Util/DebugWindow.hs:158:-- Graphics.X11.Extras.getWindowAttributes is bugggggggy

BAD:

XMonad/Actions/ConstrainedResize.hs:48:    wa <- io $ getWindowAttributes d w
XMonad/Actions/FlexibleManipulate.hs:83:    [wpos, wsize] <- io $ getWindowAttributes d w >>= return . winAttrs
XMonad/Actions/FlexibleResize.hs:54:    wa <- io $ getWindowAttributes d w
XMonad/Actions/FloatKeys.hs:47:    wa <- io $ getWindowAttributes d w
XMonad/Actions/FloatKeys.hs:65:    wa <- io $ getWindowAttributes d w
XMonad/Actions/FloatKeys.hs:115:    wa <- io $ getWindowAttributes d w
XMonad/Actions/FloatSnap.hs:97:    wa <- io $ getWindowAttributes d w
XMonad/Actions/FloatSnap.hs:125:    wa <- io $ getWindowAttributes d w
XMonad/Actions/FloatSnap.hs:175:    wa <- io $ getWindowAttributes d w
XMonad/Actions/FloatSnap.hs:216:    wa <- io $ getWindowAttributes d w
XMonad/Actions/FloatSnap.hs:256:    wa <- io $ getWindowAttributes d w
XMonad/Actions/FloatSnap.hs:290:    wa <- io $ getWindowAttributes d w
XMonad/Actions/FloatSnap.hs:295:    wla <- filter (collides wa) `fmap` (io $ mapM (getWindowAttributes d) $ filter (/=w) wl)
XMonad/Actions/Navigation2D.hs:594:             <$> getWindowAttributes dpy win
XMonad/Actions/NoBorders.hs:30:        cw <- wa_border_width `fmap` getWindowAttributes d w
XMonad/Actions/Warp.hs:96:            wa <- io $ getWindowAttributes d w
XMonad/Actions/WindowMenu.hs:74:  wa <- io $ getWindowAttributes d w
XMonad/Hooks/ManageDocks.hs:190:    wa <- io $ getWindowAttributes dpy rootw
XMonad/Hooks/PositionStoreHooks.hs:76:        wa <- io $ getWindowAttributes d w
XMonad/Hooks/WorkspaceByPos.hs:46:    wa <- io $ getWindowAttributes d w
XMonad/Layout/AvoidFloats.hs:112:            _ -> do rs <- io $ map toRect `fmap` mapM (getWindowAttributes d) (filter shouldAvoid $ M.keys floating)
XMonad/Layout/DecorationAddons.hs:111:                        wa <- io $ getWindowAttributes d decoWin
XMonad/Layout/FixedColumn.hs:87:    bw <- fmap (fromIntegral . wa_border_width) $ getWindowAttributes d w
XMonad/Layout/LayoutHints.hs:264:    wa <- getWindowAttributes d w
XMonad/Layout/LayoutScreens.hs:88:    do a <- io $ getWindowAttributes d w
XMonad/Layout/SimpleFloat.hs:74:  wa <- io $ getWindowAttributes d w
XMonad/Layout/SimplestFloat.hs:58:  wa <- io $ getWindowAttributes d w

[pjones]

Thanks for starting the conversation about this.

I think the plan we are considering is creating a new set of functions in X11 that return a Maybe a instead of throwing exceptions. This is going to be a lot of work but it's the right thing to do.

[SOwOphie]

Yeah, that seems like a good choice. I'm happy to help with anything that doesn't require massive amounts of X11 knowledge.

By the way, XMonad.Util.DebugWindow seems to already have implemented a few safe wrappers on its own, like safeGetWindowAttributes :: Display -> Window -> IO (Maybe WindowAttributes). Moving them to X11 may or may not save some work.

[pjones]

This needs to start with work on the xmonad/x11 project. You don't need to know about X11, you'll just be updating unsafe functions and making them safe.

[Ongy]

I was looking at making a PR for the xmonad repo that works with the X11 repo from @SirBoonami

Fixing all the type errors is somewhat easy, but what should we do with the error messages? Print to stderr, silently ignore, or are there some where we want to crash/recover?

[SOwOphie]

The changes mentioned in xmonad/X11#51 are still in the "draft" stage. Basing anything on top of them would be a waste of time as I can guarantee that they won't get merged in the state they're in now. But I appreciate your offer to help; I'll come back to you as soon as the X11 part is done.