Skip to content

Commit ebdfddb

Browse files
soulomoonfendor
andauthored
Apply suggestions from code review
Co-authored-by: fendor <fendor@users.noreply.github.com>
1 parent ca5ce66 commit ebdfddb

2 files changed

Lines changed: 24 additions & 22 deletions

File tree

ghcide/src/Development/IDE/Core/WorkerThread.hs

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -52,17 +52,20 @@ Originally we used various ways to implement this, but it was hard to maintain a
5252
Moreover, we can not stop these threads uniformly when we are shutting down the server.
5353
-}
5454
data TaskQueue a = TaskQueue (TQueue a)
55+
56+
data ExitOrTask t = Exit | Task t
57+
5558
newTaskQueueIO :: IO (TaskQueue a)
5659
newTaskQueueIO = TaskQueue <$> newTQueueIO
57-
data ExitOrTask t = Exit | Task t
5860

5961
-- | 'withWorkerQueue' creates a new 'TQueue', and launches a worker
6062
-- thread which polls the queue for requests and runs the given worker
6163
-- function on them.
6264
withWorkerQueueSimple :: Recorder (WithPriority LogWorkerThread) -> T.Text -> ContT () IO (TaskQueue (IO ()))
63-
withWorkerQueueSimple log title = withWorkerQueue log title id
65+
withWorkerQueueSimple recorder title = withWorkerQueue recorder title id
66+
6467
withWorkerQueue :: Recorder (WithPriority LogWorkerThread) -> T.Text -> (t -> IO ()) -> ContT () IO (TaskQueue t)
65-
withWorkerQueue log title workerAction = ContT $ \mainAction -> do
68+
withWorkerQueue recorder title workerAction = ContT $ \mainAction -> do
6669
q <- newTaskQueueIO
6770
-- Use a TMVar as a stop flag to coordinate graceful shutdown.
6871
-- The worker thread checks this flag before dequeuing each job; if set, it exits immediately,
@@ -80,7 +83,6 @@ withWorkerQueue log title workerAction = ContT $ \mainAction -> do
8083
logWith log Debug (LogThreadEnding title)
8184
logWith log Debug (LogThreadEnded title)
8285
where
83-
-- writerThread :: TaskQueue t -> TMVar () -> (forall a. IO a -> IO a) -> IO ()
8486
writerThread q b =
8587
-- See above: check stop flag before dequeuing, exit if set, otherwise run next job.
8688
do

ghcide/src/Development/IDE/LSP/LanguageServer.hs

Lines changed: 18 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,6 @@ module Development.IDE.LSP.LanguageServer
1313
, runWithWorkerThreads
1414
, Setup (..)
1515
, InitializationContext (..)
16-
, untilMVar'
1716
) where
1817

1918
import Control.Concurrent.STM
@@ -74,15 +73,15 @@ instance Pretty Log where
7473
LogReactorShutdownRequested b ->
7574
"Requested reactor shutdown; stop signal posted: " <+> pretty b
7675
LogReactorShutdownConfirmed msg ->
77-
"Reactor shutdown confirmed: " <+> pretty msg
76+
"Reactor shutdown confirmed: " <+> pretty msg
7877
LogServerExitWith (Right 0) ->
7978
"Server exited successfully"
8079
LogServerExitWith (Right code) ->
8180
"Server exited with failure code" <+> pretty code
82-
LogServerExitWith (Left _) ->
81+
LogServerExitWith (Left ()) ->
8382
"Server forcefully exited due to exception in reactor thread"
8483
LogShutDownTimeout seconds ->
85-
"Shutdown timeout, the server will exit now after waiting for" <+> pretty seconds <+> "seconds"
84+
"Shutdown timeout, the server will exit now after waiting for" <+> pretty seconds <+> "seconds"
8685
LogRegisteringIdeConfig ideConfig ->
8786
-- This log is also used to identify if HLS starts successfully in vscode-haskell,
8887
-- don't forget to update the corresponding test in vscode-haskell if the text in
@@ -103,7 +102,7 @@ instance Pretty Log where
103102
LogSession msg -> pretty msg
104103
LogLspServer msg -> pretty msg
105104

106-
-- | Context for initializing the LSP language server.
105+
-- | Context of the LSP language server.
107106
-- This record encapsulates all the configuration and callback functions
108107
-- needed to set up and run the language server initialization process.
109108
data InitializationContext config = InitializationContext
@@ -117,7 +116,7 @@ data InitializationContext config = InitializationContext
117116
-- ^ Function to create and initialize the IDE state with the given environment
118117
, ctxUntilReactorStopSignal :: IO () -> IO ()
119118
-- ^ Lifetime control: MVar to signal reactor shutdown
120-
, ctxconfirmReactorShutdown :: T.Text -> IO ()
119+
, ctxConfirmReactorShutdown :: T.Text -> IO ()
121120
-- ^ Callback to log/confirm reactor shutdown with a reason
122121
, ctxForceShutdown :: IO ()
123122
-- ^ Action to forcefully exit the server when exception occurs
@@ -291,17 +290,18 @@ handleInit initParams env (TRequestMessage _ _ m params) = otTracedHandler "Init
291290
logWith recorder Info $ LogRegisteringIdeConfig initConfig
292291
ideMVar <- newEmptyMVar
293292

294-
let handleServerExceptionOrShutDown me = do
295-
-- shutdown shake
296-
tryReadMVar ideMVar >>= mapM_ shutdown
297-
case me of
298-
Left e -> do
299-
lifetimeConfirm "due to exception in reactor thread"
300-
logWith recorder Error $ LogReactorThreadException e
301-
ctxForceShutdown initParams
302-
_ -> do
303-
lifetimeConfirm "due to shutdown message"
304-
return ()
293+
let
294+
handleServerExceptionOrShutDown me = do
295+
-- shutdown shake
296+
tryReadMVar ideMVar >>= mapM_ shutdown
297+
case me of
298+
Left e -> do
299+
lifetimeConfirm "due to exception in reactor thread"
300+
logWith recorder Error $ LogReactorThreadException e
301+
ctxForceShutdown initParams
302+
_ -> do
303+
lifetimeConfirm "due to shutdown message"
304+
return ()
305305

306306
exceptionInHandler e = do
307307
logWith recorder Error $ LogReactorMessageActionException e
@@ -329,7 +329,7 @@ handleInit initParams env (TRequestMessage _ _ m params) = otTracedHandler "Init
329329
do
330330
ide <- ctxGetIdeState initParams env root withHieDb' threadQueue'
331331
putMVar ideMVar ide
332-
-- We might be blocked indefinitly at initialization if reactorStop is signaled
332+
-- We might indefinitly at initialization if reactorStop is signaled
333333
-- before we putMVar.
334334
untilReactorStopSignal $ forever $ do
335335
msg <- readChan $ ctxClientMsgChan initParams

0 commit comments

Comments
 (0)