How to kill a single thread

[TristanCacqueray]

Would it be possible to add a kill :: Thread a -> IO () function? And could this be ignored by the thread scope?

#7 mention being able to cancel a thread, is there a reason not to?

[mitchellwrosen]

I do think are some reasons not to provide killThread but I'm happy to hear out your use case to see if the rationale still holds.

You can currently only kill threads only as a bundle, by falling through the bottom of the associated scoped call.

By omitting a killThread in the API:

• There is only one way to kill a thread, not two.
• A thread always either runs to completion, throws a synchronous exception, or is killed by an asynchronous exception, which is very likely to be due to this library closing its scope (but other possible exceptions include out-of-memory thrown by the RTS).

A benefit of this design is you don't have to worry that a thread which is supposed to run in the background forever, or otherwise has some expectation of being alive at a certain time, has not been silently killed prior by some other thread.

[TristanCacqueray]

My use-case would be to implement a supervisor thread that can cancel other threads dynamically, for example in a game server to terminate miss behaving client. I guess with the current API, such logic can be implemented by wrapping the thread action with a custom exception handler. But I was wondering if that could be done more easily by exposing a killThread function.

[ocharles]

An alternate design here might be to have each client run like:

scoped \s -> do
  fork_ do
    takeMVar stop >>= throwIO Terminated
  runClient

Now your supervisor can be given MVars and put into then to kill clients. It does mean each client now has another fork though.

[TristanCacqueray]

I realize that if a thread can be terminated, then we also need a way to check if a thread is still running. The current API does not seems to allow that, e.g. it is missing something like tryAwait :: Thread a -> STM (Maybe a). Well I'm still exploring how concurrency works, so I don't know if any of this makes sense, but for the purpose of this discussion, here is my current solution:

import Control.Exception (SomeException, Exception, fromException)
import Control.Concurrent (ThreadId, myThreadId, throwTo)
import Control.Concurrent.STM (atomically)
import Control.Concurrent.STM.TMVar
import Ki qualified

data Status a = Running ThreadId | Killed | Died SomeException | Completed a deriving Show
newtype BackgroundThread a = BackgroundThread (TMVar (Status a))

backgroundThread :: Ki.Scope -> IO a -> IO (BackgroundThread a)
backgroundThread scope action = do
  status <- newEmptyTMVarIO
  child <- Ki.forkTry scope $ do
    -- register the action ThreadId
    threadId <- myThreadId
    atomically $ putTMVar status (Running threadId)
    action
  Ki.fork scope $ atomically $ do
    -- thread supervisor
    res <- Ki.await child
    swapTMVar status $ case res of
      Right result -> Completed result
      Left err -> case fromException err of
        Just KillThread -> Killed
        Nothing -> Died err
  pure $ BackgroundThread status

data KillException = KillThread deriving Show
instance Exception KillException

killThread :: BackgroundThread a -> IO ()
killThread (BackgroundThread status) = do
  s <- atomically $ readTMVar status
  case s of
    Running threadId -> throwTo threadId KillThread
    _ -> fail "Can't kill non-running thread"

Like @ocharles solution, this cost an extra fork, and it doesn't seems like this is breaking the structured concurrency guarantees. Assuming this is correct, I guess it's fine to implement such capability on top of the ki's API. Though I wonder if it would possible to have this as part of the ki's Core API, so that we don't need the extra fork.

[mitchellwrosen]

I like @ocharles solution. You probably wouldn't want the thread you ask to die also throw an exception back, though. That might look like:

client :: TMVar () -> IO a -> IO (Maybe a)
client doneVar action =
  scoped \scope -> do
    thread <- fork scope action
    atomically $
      (do
        () <- readTMVar doneVar
        pure Nothing)
      <|>
      (do
        result <- await thread
        pure (Just result))

(whew that was hard to type on mobile; may not compile).

As far a whether it would be worth adding killThread to the API to avoid having to spawn a second thread to act as the hitman, I'm still not sure, but leaning towards no. And as always, happy to be convinced any which way.

[TristanCacqueray]

Alright thank you so much for the feedback. The client solution does compile and works as expected! Thus I agree and it does not seems worth adding killThread to the API.

For what its worth, I'm working on a ki-effectful library, and I'm testing this client solution here: https://github.com/TristanCacqueray/ki-effectful/blob/main/test/Main.hs#L46-L62

[ocharles]

I would personally lean towards leaving killThread out. @snoyberg had good advice on Twitter which is that whenever you can avoid relying on asynchronous exceptions, you should. In this case, there is definitely a solution without using async exceptions.

[TristanCacqueray]

Thanks again for the suggestions, it seems like they solve that issue. Please re-open otherwise.

[mitchellwrosen]

No problem, I'm sure others will have the same question, I think I'll make this into a "discussion" and see if that helps any.

[TristanCacqueray]

Copying from #18, I wanted to share my use-case implementation. In Butler.Processor I wrapped the ki API to keep track of threads and attach a hitman scope. Then in Butler.App.ProcessExplorer I render the process tree and enable killing arbitrary thread.

That seems to work quite well, albeit the need to create intermediary scope to be able to terminate a thread. As mentioned in #18, the libdill provides a kill all thread in scope, and that would work well for my use-case since every "butler process" has a scope. Though I appreciate the proposed hitman solution which already work with the current API.

Hope that helps :)