Fix createTempDirectory name generation#11872
Open
Bodigrim wants to merge 1 commit into
Open
Conversation
e53ccd3 to
9d7fc11
Compare
Collaborator
|
I tested this on my machine and it works. |
9d7fc11 to
5770b21
Compare
Collaborator
Author
@ffaf1 done. |
ffaf1
approved these changes
May 24, 2026
zlonast
approved these changes
May 24, 2026
The existing `createTempDirectory` uses a suboptimal schema to generate temporary directory names. Namely, it starts with `foo-$processId`. If there is no such folder that's it, otherwise it tries to create `foo-($processId+1)`, then `foo-($processId+2)`, etc. While on the surface this is OK, in practice it puts strain on the file system. If we need to create N temporary folders, the name assignment will cycle starting from `foo-$processid` every time, totaling N^2 IO calls. There are also seem to be issues on Windows when one thread is removing its temporary folder, while another it trying to create the very same one. It would be so much better if our temporary names were more diverse by construction. Yes, there might be an odd chance of a clash because someone else just accidentally created the very same folder, but we should not routinely make our own life harder. Normally this is achieved by generating random numbers, but Cabal cannot depend on non-boot libraries such as `random`. So this patch uses a global counter. The approach is similar to one used by [`file-io`](https://hackage-content.haskell.org/package/file-io-0.2.0/docs/src/System.File.Platform.html#tempCounter) for temporary files or by [`temporary-ospath`](https://hackage-content.haskell.org/package/temporary-ospath-1.3/docs/src/System.IO.Temp.OsPath.html#tempDirectoryCounter). (Both `file-io` and `temporary-ospath` actually have even more refined schema for name generation, getting entropy from CPU time, but I think in the interest of simplicity Cabal can get away just with a global counter)
5770b21 to
d8ba5e8
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The existing
createTempDirectoryuses a suboptimal schema to generate temporary directory names. Namely, it starts withfoo-$processId. If there is no such folder that's it, otherwise it tries to createfoo-($processId+1), thenfoo-($processId+2), etc.While on the surface this is OK, in practice it puts strain on the file system. If we need to create N temporary folders, the name assignment will cycle starting from
foo-$processidevery time, totaling N^2 IO calls. There are also seem to be issues on Windows when one thread is removing its temporary folder, while another it trying to create the very same one.It would be so much better if our temporary names were more diverse by construction. Yes, there might be an odd chance of a clash because someone else just accidentally created the very same folder, but we should not routinely make our own life harder.
Normally this is achieved by generating random numbers, but Cabal cannot depend on non-boot libraries such as
random. So this patch uses a global counter. The approach is similar to one used byfile-iofor temporary files or bytemporary-ospath.(Both
file-ioandtemporary-ospathactually have even more refined schema for name generation, getting entropy from CPU time, but I think in the interest of simplicity Cabal can get away just with a global counter)Template Α: This PR modifies behaviour or interface
Include the following checklist in your PR:
significance: significantin the changelog file.