Fix concurrency issue with ZipFileTree class

[tresat]

When multiple threads are using the getFile method, it is possible for thread 2 to begin reading the file before thread 1 has finished expanding it from the zip.

This adds test to reproduce to issue reliably (I think), and fixes the issue by using a cache to mangae the expansion of zip files using this class's inner DetailsImpl.

This should allow for correct operation, even if multiple Gradle processes are working with the same archive file; or if multiple tasks in different projects work with the file simultaneously.

This fixes both ZipFileTree and TarFileTree.

Fixes #22685

[tresat]

@big-guy - I think the JaCoCo related errors on Windows for this will be fixed by #22840 and #22834

[big-guy]

I haven't looked at everything, but I realize now I may have sent you down a troublesome path.

Using the PersistentCache introduces a sync point for all extracted archives. We can only extract one file at a time. I don't know if we're introducing a perf overhead by locking the cache, extracting a single file and unlocking. I made the suggestion to move this locking outside of the visitor, but I think this might just move the problem around so that we're now locking while extracting the entire archive vs a single file.

Since we are using a single cache for everything, the subdirectories under the cache are important. This PR re-uses the same convention as the existing code (file name + hash of file), but we could make this just the hash of the file so we don't need to extract the same contents if only the file name is different. We could also make this cache cross-version so we don't need to re-extract an archive if the Gradle version changes.

With the cache shared across builds and projects vs being inside buildDir, this could lead to a few weird things due to the shared expanded directory.

Imagine you had two projects that did something like:

// some task in project A
tree = zipTree("/path/to/the.zip")
tree.visit { file ->
    if (file.name == "foo") {
        file.text = "foo"
    }
}

// some task in project B
tree = zipTree("/path/to/the.zip")
tree.visit { file ->
    if (file.name == "foo") {
        file.text = "not foo"
    }
}

With existing behavior, this produces two separate expand directories where foo has the contents "foo" in project A and "not foo" in project B.
With this PR, the contents of foo would depend on which task ran first, I think.

There might be other things that you can do with the file visitor that might subtly break things. e.g., you can call stopVisiting() in a file visitor and the contents of the expanded directory will be incomplete. Is there some way that someone would get the root directory of the expanded dir and expect the incomplete extraction?

[tresat]

We can only extract one file at a time. I don't know if we're introducing a perf overhead by locking the cache, extracting a single file and unlocking. I made the suggestion to move this locking outside of the visitor, but I think this might just move the problem around so that we're now locking while extracting the entire archive vs a single file.

Do we need to be able to extract multiple files simultaneously? This seems like it typically wouldn't offer any performance gains, as it would be limited by the file system.

but we could make this just the hash of the file so we don't need to extract the same contents if only the file name is different.

OK

We could also make this cache cross-version so we don't need to re-extract an archive if the Gradle version changes.

OK

With the cache shared across builds and projects vs being inside buildDir...

I'll change this to use a per-project cache under the buildDir then.

[tresat]

@bot-gradle test this

[bot-gradle]

OK, I've already triggered the following builds for you:

• PullRequestFeedback build

[big-guy]

@bot-gradle test and merge

[bot-gradle]

Your PR is queued. See the queue page for details.

[bot-gradle]

Your PR is queued. See the queue page for details.

[bot-gradle]

OK, I've already triggered a build for you.