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

Jump to bottom

x/tools/gopls: test cleanup errors during shutdown with ERROR_SHARING_VIOLATION on Windows #53819

Closed
findleyr opened this issue Jul 12, 2022 · 8 comments
Closed

x/tools/gopls: test cleanup errors during shutdown with ERROR_SHARING_VIOLATION on Windows #53819

findleyr opened this issue Jul 12, 2022 · 8 comments
Assignees
@findleyr
Labels
FrozenDueToAge gopls Issues related to the Go language server, gopls. OS-Windows Tools This label describes issues relating to any tools in the x/tools repository.
Milestone
gopls/v0.9.2

Comments

@findleyr
Copy link
Contributor

Following the recent change to await proper shutdown of the regtest environment (CL 416594), we get one new flake on windows:

--- FAIL: TestRegenerateCgo (7.42s)
    --- FAIL: TestRegenerateCgo/experimental (4.02s)
        runner.go:322: closing the sandbox: error(s) cleaning sandbox: cleaning modcache: <nil>; removing files: remove C:\Users\gopher\AppData\Local\Temp\gopls-regtest-2462642078\TestRegenerateCgo\experimental\work: The process cannot access the file because it is being used by another process.

https://build.golang.org/log/27834526d9f9152f3bc200e902c97d8a73d55452
https://build.golang.org/log/b4e52dbee66f77feae2f7fa17e2981087e7fbbcb

If we've learned anything from investigating these flakes, it is that they are often (if not usually) actual races in gopls. Unfortunately the goroutine dump at test failure point does not show anything interesting. It is possible that problematic goroutines had exited before the dump.
@gopherbot gopherbot added Tools This label describes issues relating to any tools in the x/tools repository. gopls Issues related to the Go language server, gopls. labels Jul 12, 2022
@gopherbot gopherbot added this to the Unreleased milestone Jul 12, 2022
@bcmills
Copy link
Contributor

bcmills commented Jul 12, 2022

Note that Windows in particular seems to produce this error spuriously (compare #51442), or at least for reasons we haven't yet been able to diagnose (see also #25965).

In the testing package in the standard library, we're currently working around it with a retry loop:
https://cs.opensource.google/go/go/+/master:src/testing/testing.go;l=1115-1143;drc=de9805c702bcc19bcf3c783d1c2e43fdf4e1d30e

Would it be possible for gopls to use the testing package to create and clean up its sandbox directories? If not, you might consider copying the testing retry loop into the gopls test harness.

@findleyr
Copy link
Contributor Author

Thank you @bcmills! I was about to ping you to ask, because I seemed to recall the exact context you have just produced :)

I'll copy the removeAll logic.

@findleyr findleyr modified the milestones: Unreleased, gopls/v0.9.1 Jul 12, 2022
@gopherbot
Copy link

Change https://go.dev/cl/417117 mentions this issue: internal/lsp/fake: retry spurious file lock errors on windows

gopherbot pushed a commit to golang/tools that referenced this issue Jul 12, 2022
@findleyr
internal/lsp/fake: retry spurious file lock errors on windows
8730184 
Cleaning the regtest sandbox sometimes fails on windows with errors that
may be spurious. Copy logic from the testing package to retry these
errors.

For golang/go#53819

Change-Id: I059fbb5e023af1cd52a5d231cd11a7c2ae72bc92
Reviewed-on: https://go-review.googlesource.com/c/tools/+/417117
Run-TryBot: Robert Findley <rfindley@google.com>
Reviewed-by: Bryan Mills <bcmills@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
gopls-CI: kokoro <noreply+kokoro@google.com>
@findleyr findleyr modified the milestones: gopls/v0.9.1, gopls/v0.9.2 Jul 13, 2022
@bcmills
Copy link
Contributor

bcmills commented Jul 18, 2022 •
edited

This failure mode seems to affect many different tests on Windows, especially on the windows/arm64 and windows/386 builders.

greplogs -l -e 'cleaning .*: The process cannot access the file because it is being used by another process' --since=2022-07-01
2022-07-15T15:11:26-22d1494-2aa473c/windows-386-2008-newcc
2022-07-15T14:27:36-1a4e02f-4651ebf/windows-arm64-10
2022-07-15T14:20:24-db8f89b-4651ebf/windows-arm64-10
2022-07-14T21:03:14-db8f89b-783ff7d/windows-arm64-11
2022-07-14T19:05:09-db8f89b-a906d3d/windows-arm64-10
2022-07-14T15:54:36-db8f89b-266c70c/windows-arm64-10
2022-07-14T01:47:39-db8f89b-558785a/windows-arm64-11
2022-07-13T21:42:33-a7c53b5-558785a/windows-arm64-11
2022-07-13T21:41:37-a7c53b5-f71f3d1/windows-386-2008-newcc
2022-07-13T20:43:22-85173cc-f71f3d1/windows-arm64-11
2022-07-13T20:11:14-85173cc-c006b7a/windows-arm64-10
2022-07-13T20:11:14-85173cc-1ed3c12/windows-arm64-10
2022-07-13T16:06:45-b230791-923740a/windows-386-2008-newcc
2022-07-13T16:06:45-b230791-923740a/windows-arm64-10
2022-07-12T22:26:41-8730184-bf2ef26/windows-arm64-11
2022-07-12T22:14:18-459e2b8-bf2ef26/windows-amd64-2016-newcc
2022-07-12T22:14:18-459e2b8-bf2ef26/windows-arm64-10
2022-07-12T21:31:27-7c06b01-9c2526e/windows-arm64-11
2022-07-12T19:56:08-7c06b01-88a06f4/windows-386-2008-newcc
2022-07-12T16:56:46-6e6f313-fb979a5/windows-386-2008
2022-07-12T16:56:46-6e6f313-fb979a5/windows-amd64-2016
2022-07-12T14:39:29-3db2cdc-913d051/windows-arm64-11
2022-07-12T14:39:19-a5adb0f-913d051/windows-arm64-11
2022-07-12T14:39:04-bc957ec-913d051/windows-arm64-11
2022-07-07T19:06:45-1dfab61-8ac58de/windows-arm64-11

@bcmills bcmills changed the title x/tools/gopls: TestRegenerateCgo/experimental flaking during shutdown on windows. x/tools/gopls: test cleanup errors during shutdown with ERROR_SHARING_VIOLATION on Windows Jul 18, 2022
@bcmills
Copy link
Contributor

bcmills commented Jul 18, 2022

Note that these failures still persist after CL 417117. 🤔

@bcmills bcmills mentioned this issue Jul 18, 2022
Closed
@findleyr
Copy link
Contributor Author

My investigation into the persistent errors suggested that the new problem is simply that the server did not have enough time to shut down: not that there is a bug in shutdown itself, or that the retry logic doesn't work.

This seems to have been fixed by https://go.dev/cl/417583.

@findleyr
Copy link
Contributor Author

Note: this seems to have gone away so I will close this issue with a CL to improve/remove the shutdown timeout, as suggested by @bcmills in a follow-up comment on https://go.dev/cl/417583.

@findleyr findleyr self-assigned this Jul 27, 2022
@gopherbot
Copy link

Change https://go.dev/cl/419503 mentions this issue: internal/lsp/regtest: remove arbitrary timeout for closing the editor

@golang golang locked and limited conversation to collaborators Jul 28, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees

@findleyr findleyr

Labels
FrozenDueToAge gopls Issues related to the Go language server, gopls. OS-Windows Tools This label describes issues relating to any tools in the x/tools repository.
Projects
None yet
Milestone
gopls/v0.9.2
Development

No branches or pull requests
3 participants
@bcmills @gopherbot @findleyr