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

debug/pe: TestDefaultLinkerDWARF failure on Windows due to "Access is denied" during cleanup #45672

Closed
bcmills opened this issue Apr 21, 2021 · 3 comments
Labels
FrozenDueToAge NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. OS-Windows Testing An issue that has been verified to require only test changes, not just a test failure.
Milestone

Comments

@bcmills
Copy link
Contributor

bcmills commented Apr 21, 2021

2021-04-21T13:27:17-381252f/windows-amd64-2012 looks like it may have failed despite the 5ms sleep:

--- FAIL: TestDefaultLinkerDWARF (1.25s)
    file_test.go:395: Testprog output:
        base=0x240000
        main=0x2cf440
        offset=0x8f440
    file_test.go:442: Found main.main
    testing.go:947: TempDir RemoveAll cleanup: CreateFile C:\Users\gopher\AppData\Local\Temp\1\TestDefaultLinkerDWARF1597920265\001\a.exe: Access is denied.
FAIL
FAIL	debug/pe	4.436s

This is probably due to the same underlying problem as #25965. We had a related issue in the cmd/go tests (#19491), which we worked around by explicitly calling robustio.RemoveAll instead of the usual os.RemoveAll.

I'm not sure whether it would be better to change debug/pe to use robustio.RemoveAll explicitly, or to change the testing package to retry RemoveAll on Windows in case of Access is denied errors, or perhaps to change something even lower in the stack on Windows (perhaps os.RemoveAll itself?) to make it more robust.

CC @bradfitz @alexbrainman

@bcmills bcmills added Testing An issue that has been verified to require only test changes, not just a test failure. OS-Windows NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. labels Apr 21, 2021
@bcmills bcmills added this to the Backlog milestone Apr 21, 2021
@alexbrainman
Copy link
Member

This is probably due to the same underlying problem as #25965.

There is also recent

https://go-review.googlesource.com/c/go/+/309352

change. There could be bug in this CL. There could be bug in testing.TempDir that this CL uses.

I am just guessing. I did not investigate the issue properly.

I'm not sure whether it would be better to change debug/pe to use robustio.RemoveAll explicitly, or to change the testing package to retry RemoveAll on Windows in case of Access is denied errors, or perhaps to change something even lower in the stack on Windows (perhaps os.RemoveAll itself?) to make it more robust.

I can be convinced to use robustio.RemoveAll in testing package, if it helps.

I don't see how we can make os.RemoveAll retry until file is deleted. In some situations it is impossible to the delete file. For example, when you don't have right credentials to the delete file. How do you expect robustio.RemoveAll to break its loop in this situation?

We should not use robustio.RemoveAll in debug/pe. In my opinion robustio.RemoveAll is an unfortunate hack, and we should minimise its spread if we can.

Alex

@bcmills
Copy link
Contributor Author

bcmills commented Dec 8, 2021

Let's treat this as a testing package issue.

@bcmills
Copy link
Contributor Author

bcmills commented Dec 8, 2021

Duplicate of #50051

@bcmills bcmills marked this as a duplicate of #50051 Dec 8, 2021
@bcmills bcmills closed this as completed Dec 8, 2021
@golang golang locked and limited conversation to collaborators Dec 8, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
FrozenDueToAge NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. OS-Windows Testing An issue that has been verified to require only test changes, not just a test failure.
Projects
None yet
Development

No branches or pull requests

3 participants