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.

Sign up for GitHub

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

Jump to bottom

cmd/go: 'Access is denied' when renaming module cache directory [1.14 backport] #37800

Closed
gopherbot opened this issue Mar 11, 2020 · 7 comments
Closed

cmd/go: 'Access is denied' when renaming module cache directory [1.14 backport] #37800

gopherbot opened this issue Mar 11, 2020 · 7 comments
Labels
CherryPickApproved Used during the release process for point releases FrozenDueToAge
Milestone
Go1.14.2

Comments

@gopherbot
Copy link

@jayconrod requested issue #36568 to be considered for backport to the next 1.14 minor release.

@gopherbot Please open backport issue for 1.14.
@gopherbot gopherbot added the CherryPickCandidate Used during the release process for point releases label Mar 11, 2020
@gopherbot gopherbot mentioned this issue Mar 11, 2020
Closed
@gopherbot gopherbot added this to the Go1.14.1 milestone Mar 11, 2020
@jayconrod
Copy link
Contributor

CL 221820 mitigates #36568, an issue where antivirus (or a similar program) prevents modules from being extracted into the module cache. That CL makes two changes:

  1. os.Rename (the operation that fails) is re-attempted multiple times over 2000 ms. This should make the error less frequent, but doesn't eliminate it entirely.
  2. Adds compatibility with a future change, added in CL 221157 but disabled by default, where modules are extracted in place without requiring os.Rename.

CL 221157 is the proper solution for this problem, but it can't be enabled yet because older versions of the Go command (1.14 and earlier) assume that if a module directory is present, it is fully extracted and ready to use. So if Go 1.14 and 1.15 access the same module directory concurrently, 1.14 might see a partially extracted module. CL 221820 prevents that from happening.

The flag in CL 221157 can be flipped safely when 1.14 and older versions are no longer supported and no longer in use. That would probably be true for 1.17.

I'd like to speed up the migration by backporting CL 221820 to 1.14, probably 1.14.2. That would allow us to flip the flag in 1.16.

@networkimprov
Copy link

Older releases remain in use well after they're no longer supported, so we should also backport to 1.13. It's possible that many sites will skip 1.14 entirely due to its major runtime changes and bumpy launch.

@jayconrod
Copy link
Contributor

I've just opened #37802 for 1.13 backport.

@gopherbot
Copy link
Author

Change https://golang.org/cl/223146 mentions this issue: [release-branch.go1.13] cmd/go: make module zip extraction more robust

@gopherbot
Copy link
Author

Change https://golang.org/cl/223147 mentions this issue: [release-branch.go1.14] cmd/go: make module zip extraction more robust

@cagedmantis cagedmantis modified the milestones: Go1.14.1, Go1.14.2 Mar 19, 2020
@cagedmantis
Copy link
Contributor

Approved. This is a serious issue without a reasonable workaround.

@cagedmantis cagedmantis added CherryPickApproved Used during the release process for point releases and removed CherryPickCandidate Used during the release process for point releases labels Mar 20, 2020
@gopherbot
Copy link
Author

Closed by merging bec8e9c to release-branch.go1.14.

gopherbot pushed a commit that referenced this issue Apr 7, 2020
@andybons
[release-branch.go1.14] cmd/go: make module zip extraction more robust
bec8e9c 
Currently, we extract module zip files to temporary directories, then
atomically rename them into place. On Windows, this can fail with
ERROR_ACCESS_DENIED if another process (antivirus) has files open
before the rename. In CL 220978, we repeated the rename operation in a
loop over 500 ms, but this didn't solve the problem for everyone.

A better solution will extract module zip files to their permanent
locations in the cache and will keep a ".partial" marker file,
indicating when a module hasn't been fully extracted (CL 221157).
This approach is not safe if current versions of Go access the module
cache concurrently, since the module directory is detected with a
single os.Stat.

In the interim, this CL makes two changes:

1. Flaky file system operations are repeated over 2000 ms to reduce
the chance of this error occurring.
2. cmd/go will now check for .partial files created by future
versions. If a .partial file is found, it will lock the lock file,
then remove the .partial file and directory if needed.

After some time has passed and Go versions lacking this CL are no
longer supported, we can start extracting module zip files in place.

Updates #37800

Change-Id: I467ee11aa59a90b63cf0e3e761c4fec89d57d3b6
Reviewed-on: https://go-review.googlesource.com/c/go/+/221820
Run-TryBot: Jay Conrod <jayconrod@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Michael Matloob <matloob@golang.org>
Reviewed-by: Bryan C. Mills <bcmills@google.com>
(cherry picked from commit 093049b)
Reviewed-on: https://go-review.googlesource.com/c/go/+/223147
@golang golang locked and limited conversation to collaborators Apr 7, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
CherryPickApproved Used during the release process for point releases FrozenDueToAge
Projects
None yet
Milestone
Go1.14.2
Development

No branches or pull requests
4 participants
@cagedmantis @jayconrod @networkimprov @gopherbot