concurrently' looks a tad leaky #102

Activity

1. treeowl commented on Oct 3, 2019

   

   treeowl

   on Oct 3, 2019

   ContributorAuthor

   More actions

   Perhaps there's a simpler way. I haven't looked at whether this would work for concurrently', but I think it would work for its applications. For example, I think we could use something vaguely like

   data Status a = Started !ThreadId | Done !(Either SomeException a)
   
   concurrently :: IO a -> IO b -> IO (a, b)
   concurrently left right = do
     mva <- newEmptyMVar
     ta <- forkIO $ (do
       ar' <- left
       swapMVar mva $ Done (Right ar'))
          `catch` \e -> swapMVar mva $ Done (Left e)
     putMVar mva (Started ta)
     br <- right `onException` takeMVar mva >>= \case
       Started tid -> throwTo tid ThreadCancelled
       Done{} -> pure ()
     ar <- takeMVar mva
     pure (ar, br)

   Unlike the current code, nothing is uninterruptible.
2. simonmar commented on Oct 3, 2019

   

   simonmar

   on Oct 3, 2019

   Owner

   More actions

   Is this really a problem? A finished ThreadId will only keep alive the topmost stack chunk, which is 1KB. I'm worried the cost of fixing this "leak" could be worse than the leak itself.

   👀React with 👀1Reacted by Elaine
3. treeowl commented on Oct 3, 2019

   

   treeowl

   on Oct 3, 2019

   ContributorAuthor

   More actions

   Dunno! I'll let you know if I come up with a solution I like. This stuff is terribly subtle.