Join GitHub today
GitHub is home to over 40 million developers working together to host and review code, manage projects, and build software together.Sign up
x/tools/gopls: context cancellation errors in type-checking #33678
Please answer these questions before submitting your issue. Thanks!
What did you do?
Edit my code normally, adding functions, imports, etc.
What did you expect to see?
Completion, diagnostics, etc, continue to work.
What did you see instead?
After a few edits, I'm stuck with "work queue is full" and am forced to reload the editor.
This is an issue I just introduced with CL 185438. I'm in the process of some pretty large changes, so things may be a bit unstable on master right now. That said, it seems like what's happening here is that we are propagating context cancellation errors from type-checking. I'll try to investigate this a bit further.
Maybe the CL you mentioned reintroduced the problem where we cache the context.Canceled error in the package cache and it gets stuck that way indefinitely. Can we make it never cache the package on type checker errors like before?
Also, it seems like https://go-review.googlesource.com/c/tools/+/190412 maybe have broken some spots checking explicitly for the context.Canceled error. Perhaps we should use the "%w" error wrapping verb and the checking code should use
Updates golang/go#33678 Change-Id: I844d6599a3e0ae9594dda1abaebe938402b65822 Reviewed-on: https://go-review.googlesource.com/c/tools/+/190601 Run-TryBot: Rebecca Stambler <email@example.com> TryBot-Result: Gobot Gobot <firstname.lastname@example.org> Reviewed-by: Ian Cottrell <email@example.com>
The memoize store passes a detached context to the value getter function. This is important since if the value getter experiences a context cancellation it will end up caching context.Canceled, which you never want. When type checking, we were ignoring the detached context and using the "real" request context. This would cause the context.Canceled error to get cached and continue popping up in various situations. Fix by swapping the importer's context to the detached context. It is a little messy since the importer stores the context as a field. I added a defer to restore the original context since it doesn't seem correct to let the detached context escape the memoize function. Updates golang/go#33678 Change-Id: I20dd466b0072ac2e856adbe993364f77e93ab054 Reviewed-on: https://go-review.googlesource.com/c/tools/+/192719 Reviewed-by: Rebecca Stambler <firstname.lastname@example.org> Run-TryBot: Rebecca Stambler <email@example.com>