xfce4-terminal window tiling bug

[georgewsinger]

Way to replicate. Launch Simula with at least one xfce4-terminal launched by default, e.g.:

,   _startingApps = { _center = None Text
                    , _right  = Some "./result/bin/xfce4-terminal"
                    , _bottom = None Text
                    , _left   = None Text
                    , _top    = None Text
                    }

Then launch another xfce4-terminal (e.g. via Super + /), and this and all subsequent xfce4-terminals will launch to that starting location (e.g. to SIMULA_STARTING_LOCATION=right). This bug will persist even if xfce4-terminals are launched with SIMULA_STARTING_LOCATION=<some_other_location>, and doesn't matter how else xfce4-termianls are launched (via keyboard shortcut, from an outside terminal, etc).

The issue. The issue is subsequent launches of xfce4-terminals have the same PID and /proc/*/environ as the original xfce4-terminal. For example when When xfce4-terminals are launched via keyboard shortcut:

appLaunch :: GodotSimulaServer -> String -> Maybe String -> IO ProcessID
appLaunch gss appStr maybeLocation = do
  let originalEnv = (gss ^. gssOriginalEnv)
  maybeXwaylandDisplay <- readTVarIO (gss ^. gssXWaylandDisplay)
  case (appStr, maybeXwaylandDisplay) of
        -- ..
        (_, (Just xwaylandDisplay)) -> do
            let envMap = M.fromList originalEnv
            let envMapWithDisplay = M.insert "DISPLAY" xwaylandDisplay envMap
            let envMapWithWaylandDisplay = case maybeLocation of
                                                Just location -> M.insert "SIMULA_STARTING_LOCATION" location (M.insert "WAYLAND_DISPLAY" "simula-0" envMapWithDisplay)
                                                Nothing -> (M.insert "WAYLAND_DISPLAY" "simula-0" envMapWithDisplay)
            let envListWithDisplays = M.toList envMapWithWaylandDisplay
            res <- Control.Exception.try $ createProcess (shell appStr) { env = Just envListWithDisplays, new_session = True } :: IO (Either IOException (Maybe Handle, Maybe Handle, Maybe Handle, ProcessHandle))
            pid <- case res of
                        Left _ -> return $ fromInteger 0
                        Right (_, _, _, processHandle) -> do maybePid <- System.Process.getPid processHandle -- <-- 
                                                             case maybePid of
                                                                  Just pid -> return pid
                                                                  Nothing -> return $ fromInteger $ 0
            return pid

..then System.Process.getPid processHandle retrieves the same pid as the original xfce4-terminal. This forces the new terminals to have the same SIMULA_STARTING_LOCATION as the original one.

[georgewsinger]

Hack solution. Instead of TVar (M.Map ProcessID String), we use TVar (M.Map ProcessID [String]). If e.g. a user launches two instances of firefox (one in left and one in right, then Simula will just choose one of those locations. It's a "hack" because it's possible that a user might want to do something like

,   _startingApps = { _center = None Text
                    , _right  = Some "./result/bin/xfce4-terminal --command='echo right'"
                    , _bottom = None Text
                    , _left   = Some "./result/bin/xfce4-terminal --command='echo left'"
                    , _top    = None Text
                    }

and the wrong one gets mapped to "left".

Code sketch.

data GodotSimulaServer = GodotSimulaServer
  { _gssObj                   :: GodotObject
  -- ..
  , _gssStartingAppPids       :: TVar (M.Map ProcessID [String])
  }

appLaunch :: GodotSimulaServer -> String -> Maybe String -> IO ProcessID
appLaunch gss appStr maybeLocation = do
  let originalEnv = (gss ^. gssOriginalEnv)
  maybeXwaylandDisplay <- readTVarIO (gss ^. gssXWaylandDisplay)
  case (appStr, maybeXwaylandDisplay) of
        -- ..
        (_, (Just xwaylandDisplay)) -> do
            -- ..launch the app and get a pid..
            pid <- case res of
                        Left _ -> return $ (fromInteger 0 :: ProcessID)
                        Right (_, _, _, processHandle) -> do maybePid <- System.Process.getPid processHandle
                                                             case maybePid of
                                                                  Just pid -> return pid
                                                                  Nothing -> return $ fromInteger $ 0
            startingAppPids <- readTVarIO (gss ^. gssStartingAppPids)
            case maybeLocation of
              Nothing -> return ()
              Just location -> do let startingAppPids' = M.insertWith (++) pid [location] startingAppPids -- Jam it into gssStartingAppPids
                                  atomically $ writeTVar (gss ^. gssStartingAppPids) startingAppPids'
            return pid

getSimulaStartingLocationAtomically :: GodotSimulaServer -> ProcessID -> IO (Maybe String)
getSimulaStartingLocationAtomically gss pid = atomically $ do
  startingAppPids <- readTVar (gss ^. gssStartingAppPids)
  case (M.lookup pid startingAppPids) of
    Nothing -> do
      return Nothing
    Just [] -> do
      let startingAppPids' = M.delete pid startingAppPids
      writeTVar (gss ^. gssStartingAppPids) startingAppPids'
      return Nothing
    Just (location:locations) -> do
      let startingAppPids' = M.insert pid locations startingAppPids
      writeTVar (gss ^. gssStartingAppPids) startingAppPids'
      return $ (Just location)

[georgewsinger]

Fixed in https://github.com/SimulaVR/Simula/commit/1d6e83d3269aa93e95f5ed963698f283bde0d06c

We still face the limitation that launching certain apps multiple times in different starting locations can fail (now sending those apps to the center). This afflicts apps which have indirect launch scripts (e.g. firefox). We've decided to triage this issue to focus on other more pressing things.