From 331c97df88a7447ae117510acd7fbabf891fc0b8 Mon Sep 17 00:00:00 2001 From: Joey Hess Date: Mon, 6 Jun 2022 12:16:55 -0400 Subject: [PATCH] fix MVar deadlock when sqlite commit fails The database queue was left empty, which caused subsequent calls to flushDbQueue to deadlock. Sponsored-by: Dartmouth College's Datalad project --- Database/Queue.hs | 11 ++++++---- ..._216aefa4a78d13cc0c46780161921a36._comment | 22 +++++++++++++++++++ 2 files changed, 29 insertions(+), 4 deletions(-) create mode 100644 doc/bugs/get_is_busy_doing_nothing/comment_19_216aefa4a78d13cc0c46780161921a36._comment diff --git a/Database/Queue.hs b/Database/Queue.hs index 46418ef1d8..f4882d2fa8 100644 --- a/Database/Queue.hs +++ b/Database/Queue.hs @@ -1,6 +1,6 @@ {- Persistent sqlite database queues - - - Copyright 2015 Joey Hess + - Copyright 2015-2022 Joey Hess - - Licensed under the GNU AGPL version 3 or higher. -} @@ -20,6 +20,7 @@ module Database.Queue ( import Utility.Monad import Utility.RawFilePath import Utility.DebugLocks +import Utility.Exception import Database.Handle import Database.Persist.Sqlite @@ -54,9 +55,11 @@ flushDbQueue :: DbQueue -> IO () flushDbQueue (DQ hdl qvar) = do q@(Queue sz _ qa) <- debugLocks $ takeMVar qvar if sz > 0 - then do - commitDb hdl qa - debugLocks $ putMVar qvar =<< emptyQueue + then tryNonAsync (commitDb hdl qa) >>= \case + Right () -> debugLocks $ putMVar qvar =<< emptyQueue + Left e -> do + debugLocks $ putMVar qvar q + throwM e else debugLocks $ putMVar qvar q {- Makes a query using the DbQueue's database connection. diff --git a/doc/bugs/get_is_busy_doing_nothing/comment_19_216aefa4a78d13cc0c46780161921a36._comment b/doc/bugs/get_is_busy_doing_nothing/comment_19_216aefa4a78d13cc0c46780161921a36._comment new file mode 100644 index 0000000000..06bbb569ff --- /dev/null +++ b/doc/bugs/get_is_busy_doing_nothing/comment_19_216aefa4a78d13cc0c46780161921a36._comment @@ -0,0 +1,22 @@ +[[!comment format=mdwn + username="joey" + subject="""comment 19""" + date="2022-06-06T15:55:15Z" + content=""" +So in flushDbQueue, which is waiting for all queued writes to complete. +When the write failed with an exception, a previous flushDbQueue +would have left the queue's MVar empty. + +So, now I understand how the original problem can lead to this MVar +problem. And I've fixed that part of it. Now a write failing this way will +refill the queue with what it failed to write. So it will try to write it +again later. + +This flushDbQueue probably also explains the hang at +"recording state in git" since there is also a final flushDbQueue at that +point, and I guess it fails to detect a deadlock at that point so just +hangs forever. So my fix should also avoid that. + +None of which means this is fixed, really, just the fallout from the +write timeout problem will be less severe now. +"""]]