Lock store creation against concurrent processes

[crtschin]

Fixes #11329.

Used the hypothesis in #12111 (comment) to create a reproducer. Extracted a helper from the code that used fd locks to make concurrent store additions safe, and use that when initializing the store as well.

Template Α: This PR modifies behaviour or interface

Include the following checklist in your PR:

• Patches conform to the coding conventions.
• Any changes that could be relevant to users have been recorded in the changelog.
  • Is the change significant? If so, remember to add significance: significant in the changelog file.
• The documentation has been updated, if necessary.
• Manual QA notes have been included.
• Tests have been added. (Ask for help if you don’t know how to write them! Ask for an exemption if tests are too complex for too little coverage!)

[Bodigrim]

Thanks a lot for working on this!

I was thinking about the issue in the background and came up with a different approach. Namely,

• Distribution.Simple.Program.HcPkg.init should read stderr of ghc-pkg and raise alreadyExistsErrorType if ghc-pkg says so.
• Then createPackageDBIfMissing can catch the error from createPackageDB. If the error isAlreadyExistsError then return success, otherwise rethrow.

Theoretically this is slightly better in a sense that it works regardless on who else is trying to create package DB (which could be an older Cabal version, Stack, Shake, anyone) and there is no need to agree on the name of the lock file, because there is no locking.

But I understand if you prefer your implementation, which is already written :)

[crtschin]

Appreciate you taking a look! Nice idea, taking more of a optimistic approach to avoid locking.

It's probably benign, but I'm not too big of a fan of the idea of logic depending on reading stderr. So I do prefer this as is 😄, but I'm open to changing the PR to your approach though if that's generally preferred.

[Bodigrim]

I think withFileLock should go into Distribution.Simple.Utils or another utility module.

Should not we remove the lock file afterwards?

[crtschin]

I think withFileLock should go into Distribution.Simple.Utils or another utility module.

Will do.

Should not we remove the lock file afterwards?

I think in this case of the store-init, it would be fine. As the check falls back to the existence of the store directory. I haven't taken a good look into withIncomingUnitIdLock to judge if that's safe as well.

I don't think this is safe in the general sense. Deleting the lockfile would disjoint visibility of the lock. Consider the following sequence of events.

- A acquires lock 1.
- B tries to acquire lock 1 and get's blocked.
- A finishes deletes lock 1.
- B proceeds holding lock 1.
- C sees no file, creates a new lockfile and acquires lock 2.
* B and C run concurrently