Race condition caching bug with files (content being swapped) 0.19.7-0.19.8

[lyric solar]

Reproduced with my repo here: https://github.com/chrisjpalmer/dagger-file-caching-bug/tree/main

This demo repo shows a race condition when multiple files are passed to a module in parallel. Though the files are different, occassionally the contents of one file is mixed with another.

❯ dagger call -m my-module-tests test-my-module

✔ connect 0.3s
✔ load module: my-module-tests 0.3s

✔ myModuleTests: MyModuleTests! 0.0s
✘ .testMyModule(
  ┆ fixtures: context /Users/christopher.palmer/workspace/dagger-caching-bug/my-module-tests/my-module-tests/fixtures (exclude: [])
  ): Void 1.3s ERROR
✘ MyModule.expectFileContents(
  ┆ expect: "d"
  ┆ file: Directory.file(path: "d"): File!
  ): Void 1.1s ERROR
! expected value d doesn't match actual contents c

https://dagger.cloud/nine/traces/986467472d6fa2f5a1eedbcf7e99a978

Github issue raised here

Tagging you @mellow sinew for visibility 🙂

[dim bloom]

I was doing some debugging to check the contents of all of the files on this branch.

The results end up with a single value after a few runs.

expected: b
actual: c

expected: c
actual: c

expected: d
actual: c

expected: f
actual: c

expected: e
actual: c

expected: a
actual: c

After pruning the engine cache, it seems the values will be correct on the next run.

I also noticed that there is a rootfs call in the successful run, which is not in the failed run.

Success Trace

Failure Trace

[lyric solar]

Thanks Luke for that

Btw guys apologies my demo repo has the code with the bug commented out (my-module-tests/main.go)... to reproduce the condition you'll need to uncomment it 🙂

[lyric solar]

Fixed that on my demo now 🙂

[mellow sinew]

Hey 👋 Trying to take a look a bit later today or tomorrow 🙏 Was digging another perf issue on our end

[lyric solar]

Awesome thank you so much !

[mellow sinew]

This https://github.com/dagger/dagger/pull/11329 seems very suspicious, thanks for tracking the engine version in the thread / issue, very helpful 🙏 Will dig deeper tomorrow 😇

[lyric solar]

Pleasure 🙂 Thanks Guillaume!

[mellow sinew]

cc @digital reef, the corresponding thread

[digital reef]

Thanks for the easy to run reproduction case Chris! I was able to distill it down to https://github.com/alexcb/dagger/commit/3ca1bfc1d74b56009dda1167bbd6dad1cdb02a53 which we'll add in to our integration tests -- now on to the fun part of fixing the bug 🙂

[lyric solar]

No problem! 🙂 Nice integration test

Hahaha enjoy the bug fix 🙂

[digital reef]

turns out the fix was simply deleting some code 😆
https://github.com/dagger/dagger/pull/11575

[lyric solar]

Amazing thanks so much @digital reef !

Best kind of bug fix 🔥

[lyric solar]

Do you know when the next release would be ?

[mellow sinew]

Mmh probably next week i suppose, but feel free to ask on the maintainer channel. What I can say is that none is booked for this week atm

[lyric solar]

That's okay. We are all going on leave in about 2 days so we'll upgrade our engines after Christmas