Skip to content

Make cacheFile thread-safe using file locks - #520

Merged
fendor merged 5 commits into
haskell:masterfrom
crtschin:crtschin/hie-bios-cache-file
Jul 20, 2026
Merged

Make cacheFile thread-safe using file locks#520
fendor merged 5 commits into
haskell:masterfrom
crtschin:crtschin/hie-bios-cache-file

Conversation

@crtschin

Copy link
Copy Markdown
Contributor

This was mentioned on on Matrix.

Changes include:

  • Adds a cacheFileIn that allows varying the cache directory used, overriding the default behavior of using $XDG_CACHE_HOME.
  • Made the above function thread-safe by using file-locks, and atomic file writes.

As long as downstream consumers were using the srcHash parameter as a content addressed tag, I don't think they have to necessarily use cacheFileIn and can keep using cacheFile as is after this change. I exported cacheFileIn mainly for the test I added.

@fendor

fendor commented Jul 12, 2026

Copy link
Copy Markdown
Collaborator

I think hie-bios caches need to be created from scratch for the tests in HLS. Otherwise, we risk the tests using existing caches (e.g., on a rerun), changing the test run. But we want the tests to be run the same way every time!

While the lock might be useful regardless, e.g. on windows this might be a thing that helps with multiple processes trying to open a file, I think we need to be able to specify the XDG_CACHE_HOME without having to read the global XDG_CACHE_HOME variable. I.e., I think we should add a parameter to runCradle, such as CradleRunConfig (completely made up name), which allows us to optionally set the cache root directory.

@crtschin

Copy link
Copy Markdown
Contributor Author

Otherwise, we risk the tests using existing caches (e.g., on a rerun), changing the test run. But we want the tests to be run the same way every time!

Fair! Do the last two commits correspond to what you had in mind? I kept the previous API-exposed function that default to the environment usage. I can drop those if you don't think they're useful to keep.

@crtschin
crtschin force-pushed the crtschin/hie-bios-cache-file branch from ab71583 to 6b0cf08 Compare July 12, 2026 21:49

@fendor fendor left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, this is what I had in mind!

I think this is what we want. All the FilePaths are making me dizzy though, perhaps a newtype CacheDir = CacheDir FilePath is appropriate?

Comment thread src/HIE/Bios/Process.hs
@crtschin
crtschin force-pushed the crtschin/hie-bios-cache-file branch from e49e624 to 57e9245 Compare July 14, 2026 11:16
@crtschin
crtschin requested a review from fendor July 15, 2026 15:52

@fendor fendor left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, just one improvement to the testsuite, then this is ready to merge :)

Comment thread tests/BiosTests.hs Outdated
@mergify

mergify Bot commented Jul 19, 2026

Copy link
Copy Markdown

Tick the box to add this pull request to the merge queue (same as @mergifyio queue).

  • Queue this pull request

@fendor
fendor merged commit eec5c00 into haskell:master Jul 20, 2026
39 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants