Andreas Klebinger pushed to branch wip/andreask/hadrian_race at Glasgow Haskell Compiler / GHC Commits: 20a0c08f by Andreas Klebinger at 2026-02-20T13:51:25+00:00 Apply 5 suggestion(s) to 2 file(s) Co-authored-by: sheaf <sam.derbyshire@gmail.com> - - - - - 2 changed files: - libraries/ghc-boot/GHC/Unit/Database.hs - utils/ghc-pkg/Main.hs Changes: ===================================== libraries/ghc-boot/GHC/Unit/Database.hs ===================================== @@ -317,17 +317,29 @@ data DbInstUnitId -- | Represents a lock of a package db. newtype PackageDbLock = PackageDbLock Handle --- | Run the action under a lock, then return the result. --- If the mode is R/W the *caller* needs to either free the lock or pass it --- on to code that will. +-- | Take a lock on the package database and then run the action. -- --- If an exception is raised the lock is released. +-- - In read-only mode, this takes and releases a shared lock. +-- - In read-write mode, this takes an exclusive lock, and the caller +-- must arrange for this lock to be released: +-- +-- - either the inner action releases it, or +-- - the inner action returns the lock and the caller +-- of 'withLockedPackageDb' is responsible for releasing it. +-- +-- If an exception escapes the inner action, the lock is released. +-- +-- See Note [ghc-pkg database locking] in ghc-pkg/Main.hs withLockedPackageDb :: DbOpenMode m t -> FilePath -> (PackageDbLock -> IO a) -> IO a -withLockedPackageDb mode file act = do - lock <- lockPackageDbWith (lock_mode mode) file - r <- act lock `onException` unlockPackageDb lock - when (isDbOpenReadMode mode ) $ unlockPackageDb lock - pure r +withLockedPackageDb mode file = + bracket_for_mode + (lockPackageDbWith (lock_mode mode) file) + unlockPackageDb + where + bracket_for_mode = + case mode of + DbOpenReadOnly -> bracket + DbOpenReadWrite -> bracketOnError where lock_mode :: DbOpenMode m t -> LockMode lock_mode DbOpenReadOnly = SharedLock @@ -529,8 +541,8 @@ headerMagic = BS.Char8.pack "\0ghcpkg\0" -- | Feed a 'Get' decoder with data chunks from a file. -- --- The file is already locked when we call this. We only need to pass it on --- if we are in R/W mode. +-- Requires a lock (either shared or exclusive) on the package database, +-- which it returns unchanged. decodeFromFile :: FilePath -> DbOpenMode mode PackageDbLock -> Get pkgs -> IO (pkgs, DbOpenMode mode PackageDbLock) decodeFromFile file mode decoder = case mode of ===================================== utils/ghc-pkg/Main.hs ===================================== @@ -875,31 +875,30 @@ lookForPackageDBIn dir = do {- Note [ghc-pkg database locking] ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ -It's important for ghc-pkg, GHC and hadrian to always see a consistent state for -package databases. To ensure this we generally tacke a lock both for reading and -writing to the package database. - -The general idea is we use `withLockedPackageDb` to lock an already existing -database in both modes. Which behaves differently for reads and modify. - -In read mode we take a shared lock before we start reading, run the argument to -withLockedPackageDb and simply unlock the DB after. We need to lock it to avoid -concurrent invocations from deleting files from the db before we managed to read -them. This occasionally happened in #22870. Not that historically we only used -to lock the package db during reads on windows, for reasons that seem to have been -bogus given the existence of #22870. And it was partially discussed in #16773. - -When modifying a package database we also use withLockedPackageDb but we take -an *exclusive* lock and most importantly we don't unlock the DB after the action -has run unless an exception occurs. -Instead the action we are must ensure the lock is either freed, or returned -as part of the result to be freed later by the caller of withLockedPackageDB. -Typically by storing it inside either a `DbOpenMode` or `PackageDB`. - -While this setup is a bit cumbersome it seemed to be the easiest way to ensure +It's important for ghc-pkg, GHC and Hadrian to always see a consistent state for +package databases. To ensure this, we always take a lock before reading or writing +to a package database. + +The general idea is we use 'withLockedPackageDb' to lock an already existing +database in both modes: + + - In read-only mode, we take a *shared* lock before reading and unlock afterwards. + Locking is needed to avoid a concurrent invocation of ghc-pkg deleting files from + the db before we manage to read them. This occasionally happened in #22870. + (Note that, historically, we used to only lock the package db during reads on + Windows, but the justification seemed insufficient given #22870. See also #16773.) + + - In read-write mode, we take an **exclusive** lock on the package database. + However, we don't automatically release the lock after the inner action completes + (unless an exception occurs). Instead, either the inner action itself releases the + lock, or it returns it, in which case it is the caller of 'withLockedPackageDb' + that is responsible for releasing the lock. Typically, the exclusive lock is stored + in 'PackageDB', with the type informing us that we have a lock we are responsible for + releasing. 'updateDBCache' eventually releases it. + +While this setup is a bit cumbersome, it seemed to be the easiest way to ensure that locks are consistently held over reads/modifications of a database without engaging in a larger refactor of the ghc-pkg code. - -} readParseDatabase :: forall mode t. Verbosity -> Maybe (FilePath,Bool) @@ -923,7 +922,7 @@ readParseDatabase verbosity mb_user_conf mode use_cache path -- Take a lock to use while we read the DB Right fs -> withLockedPackageDb mode cache $ \lock -> do if not use_cache - then ignore_cache (lock) (const $ return ()) + then ignore_cache lock (const $ return ()) else do e_tcache <- tryIO $ getModificationTime cache case e_tcache of View it on GitLab: https://gitlab.haskell.org/ghc/ghc/-/commit/20a0c08f1125e0315d75163dce5155f8... -- View it on GitLab: https://gitlab.haskell.org/ghc/ghc/-/commit/20a0c08f1125e0315d75163dce5155f8... You're receiving this email because of your account on gitlab.haskell.org.