Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

DefaultFileSystemAccess.snapshot sometimes ignoring filter #29205

Open
mathjeff opened this issue May 17, 2024 · 1 comment · May be fixed by #29206
Open

DefaultFileSystemAccess.snapshot sometimes ignoring filter #29205

mathjeff opened this issue May 17, 2024 · 1 comment · May be fixed by #29206
Assignees
Labels
a:bug in:virtual-file-system VFS, file system watching, snapshots re:reliability sucessfull build with a wrong result

Comments

@mathjeff
Copy link
Contributor

mathjeff commented May 17, 2024

Current Behavior

I’m finding that in certain circumstances, the filter assigned to a FileCollection is ignored.

In DefaultFileSystemAccess.snapshot there is an early exit here to check for existing FileSystemLocationSnapshots, and if one is found, then the snapshot is used, regardless of whether a filter was passed to the snapshot() function

Normally this case doesn’t trigger, because there are other early exits in readSnapshotFromLocation() such as here which do apply the filter, and there is a lock in readSnapshotFromLocation() to try to prevent computing multiple snapshots of the same directory at a time (see comment).

However, if DefaultFileSystemAccess simultaneously tries to load information about a parent and child directory at the same time, then it is possible that updates of the parent directory in the virtual file system will also cause corresponding updates of the child directory, even while DefaultFileSystemAccess holds a lock on the child directory. These concurrent updates can then trigger the ignoring of a FileCollection's filter.

Expected Behavior

It would be nice if the filter assigned to a FileCollection weren't ignored

Context (optional)

In AndroidX we've been observing our compileKotlin tasks to be intermittently out-of-date: https://issuetracker.google.com/issues/321949384 and this seems to be the cause

Steps to Reproduce

I've uploaded a repro case to https://github.com/mathjeff/gradle-samples-2/tree/main/snapshot-wrong

Because reproducing this issue involves a race condition, I've also uploaded a test Gradle change that exacerbates the race condition by adding an additional hard coded sleep for a specific path (the child path) at master...mathjeff:gradle:test-exacerbate-snapshot-race-condition

The test.sh script in the repro case takes care of applying this change before running the test

Gradle version

8.7.0

Build scan URL (optional)

No response

Your Environment (optional)

No response

mathjeff added a commit to mathjeff/gradle that referenced this issue May 17, 2024
Fixes: gradle#29205
Signed-off-by: Jeff Gaston <gastoj3@gmail.com>
@mathjeff mathjeff linked a pull request May 17, 2024 that will close this issue
9 tasks
@mathjeff mathjeff changed the title DefaultFileSystemAccess.snapshot race condition DefaultFileSystemAccess.snapshot sometimes ignoring filter May 17, 2024
@ljacomet ljacomet added in:virtual-file-system VFS, file system watching, snapshots 👋 team-triage Issues that need to be triaged by a specific team re:reliability sucessfull build with a wrong result and removed to-triage labels May 21, 2024
@ljacomet
Copy link
Member

ljacomet commented May 21, 2024

This issue needs a decision from the team responsible for that area. They have been informed. Response time may vary.


Thanks a lot for the detailed investigation and companion PR.

@lptr lptr self-assigned this May 22, 2024
@lptr lptr removed the 👋 team-triage Issues that need to be triaged by a specific team label May 22, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
a:bug in:virtual-file-system VFS, file system watching, snapshots re:reliability sucessfull build with a wrong result
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants