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

Certain C++ files completely freeze VSCode #78769

Closed
oktonion opened this issue Aug 9, 2019 · 23 comments · Fixed by #80974
Closed

Certain C++ files completely freeze VSCode #78769

oktonion opened this issue Aug 9, 2019 · 23 comments · Fixed by #80974
Assignees
Labels
candidate Issue identified as probable candidate for fixing in the next release freeze-slow-crash-leak VS Code crashing, performance, freeze and memory leak issues grammar Syntax highlighting grammar verified Verification succeeded

Comments

@oktonion
Copy link

oktonion commented Aug 9, 2019

  • VSCode Version: 1.37.0
  • OS Version: Windows 7 x64

Steps to Reproduce:

  1. Open this C++ header with or without any extensions
  2. Scroll through the file content
  3. VSCode is frozen and never responds again except for no-respond-dialogue pop-up message.

Does this issue occur when all extensions are disabled?: Yes

P.S. the problem exists in previous version of VSCode.

@vscodebot vscodebot bot added the new release label Aug 9, 2019
@oktonion oktonion changed the title Certain C++ files completely freeze VsCode Certain C++ files completely freeze VSCode Aug 9, 2019
@vaetas
Copy link

vaetas commented Aug 9, 2019

I have similar problem on Arch Linux with new VS Code update (1.37). Editing C header file will completely crash VS Code, even when the file has only few lines. I noticed that it crashes always when I try to autocomplete type definition in function arguments.

Disabling extensions does not help and the problem persists.

@octref octref added the *caused-by-extension Issue identified to be caused by an extension label Aug 9, 2019
@vscodebot
Copy link

vscodebot bot commented Aug 9, 2019

This issue is caused by an extension, please file it with the repository (or contact) the extension has linked in its overview in VS Code or the marketplace for VS Code. See also our issue reporting guidelines.

Happy Coding!

@vscodebot vscodebot bot closed this as completed Aug 9, 2019
@octref
Copy link
Contributor

octref commented Aug 9, 2019

@oktonion
Copy link
Author

oktonion commented Aug 9, 2019

Does this issue occur when all extensions are disabled?: Yes

How is this caused by extension?

@octref octref added candidate Issue identified as probable candidate for fixing in the next release grammar Syntax highlighting grammar freeze-slow-crash-leak VS Code crashing, performance, freeze and memory leak issues and removed *caused-by-extension Issue identified to be caused by an extension new release labels Aug 9, 2019
@octref
Copy link
Contributor

octref commented Aug 9, 2019

@oktonion I'm very sorry. This seems to be a textmate grammar issue. We'll try to fix that in a recovery build.

@octref octref reopened this Aug 9, 2019
@octref octref added this to the July 2019 Recovery milestone Aug 9, 2019
@matter123
Copy link

matter123 commented Aug 9, 2019

@alexr00 this has been fixed in v1.13.0, upstream issue jeff-hykin/better-cpp-syntax#343

@alexr00
Copy link
Member

alexr00 commented Aug 12, 2019

@matter123 and @jeff-hykin I've updated the grammar to get your fix for Insiders. However, I'm going to roll back to the last version of the C grammars we released with, which is actually from two releases ago since I rolled back last time too.

Going forward I don't want to lose 2-3 months worth of grammar improvements in the VS Code stable build and I'm guessing that you aren't thrilled with that either. I know it is overhead for you, but would you consider having a release branch system? That way, when a hang like this happens you can pull the fix into the appropriate release branch. From there, I can pull the update from your release branch and get the single targeted fix.

@alexr00 alexr00 reopened this Aug 12, 2019
alexr00 added a commit that referenced this issue Aug 12, 2019
Recovery build part of #78769
matter123 pushed a commit to jeff-hykin/better-cpp-syntax that referenced this issue Aug 12, 2019
replace maybe(spaces) with std_space to fix
Ref: microsoft/vscode#78769
@matter123
Copy link

@alexr00 I created the branch stable(https://github.com/jeff-hykin/cpp-textmate-grammar/tree/stable) Which is intended to track the version that vscode uses. It currently is 1.12.18 plus a cherry-pick of the fix for this issue.

@jeff-hykin
Copy link

jeff-hykin commented Aug 12, 2019

I'm all for a more helpful release system. I'm okay with doing rollbacks as needed though, I know its really important to keep the stable release stable. I'm not sure how the release system would work, so let me know if this is what you had in mind. @matter123 might have a better idea of what a standard release branch system looks like.

@alexr00 If you wanted to let us know whenever a new version is pulled, we could make a branch at that point, and then we could have merge requests for small bugs or large-but-isolated changes (basically what @matter123 has just done, but just continual). I'd imagine we'd limit the fixes to only ones that new issues were created for (rather than including fixes bugs nobody had noticed).

On the repo right now, there's not that many "features". Most changes are either simple fixes or complex fixes. We can make merge requests against the stable branch for the simple fixes, but a merge request for a complex fix would be the same as just merging with the master branch. It is realistic to make this separation between simple and complex though, so we can move forward with that.

@jeff-hykin
Copy link

For this issue in particular I do think in general there should be a better fail safe on the VS Code textmate grammar side. Catastrophic backtracking can happen with very simple expressions, they can hard to notice even for experienced regex users, and because VS Code 100% freezes they can be very hard to debug. Should I open up a separate issue for that though?

@alexr00
Copy link
Member

alexr00 commented Aug 13, 2019

@jeff-hykin and @matter123 I think a stable branch that tracks the version of the grammar that VS Code uses like that is a good, simple way to manage releases. In VS Code, we have a branch for each release, but I don't think that level of complexity is needed here.

I update the grammars as far from release as is convenient so that insiders have a chance to help find bugs. I'll start tagging you on the commit where I update the C grammars.

For about a week after each release we consider "candidate" fixes for a recovery build. This is the time period where issues like this one tend to be found. The kind of fixes that I really care about being cherry-picked back are fixes for perf issues and fixes for issues that hit a large number of people. When fixes for those issues require a large or risky change, then a full rollback is safer.

Thank you for being so willing to adopt a release processes! If it becomes too onerous we can go back to only doing rollbacks, but I want to start getting more users on a newer grammar so we can get more coverage and have a better builtin experience.

@alexr00
Copy link
Member

alexr00 commented Aug 13, 2019

And @jeff-hykin, this does seem like an issue that can be helped by VS Code textmate. Filing and issue over in https://github.com/Microsoft/vscode-textmate would help get that started.

alexr00 added a commit that referenced this issue Aug 13, 2019
Recovery build part of #78769
@alexr00 alexr00 closed this as completed Aug 13, 2019
@jeff-hykin
Copy link

jeff-hykin commented Aug 13, 2019

Small/isolated fix => cherry pick
Complex fix => rollback
That sounds like a good plan to me!

I'll start tagging you on the commit where I update the C grammars.

Thank you, and thanks for explaining the integration/update/release process thats helpful to know.

@jeff-hykin
Copy link

jeff-hykin commented Aug 13, 2019

And @jeff-hykin, this does seem like an issue that can be helped by VS Code textmate. Filing and issue over in https://github.com/Microsoft/vscode-textmate would help get that started.

I posted an issue there, microsoft/vscode-textmate#104 However, I think an initial fix might need to be done in a more core area of VS Code.

Even if VS Code used an entirely different parser, like the tree sitter, if the parser is taking too long to tokenize the code (due to file size or any other cause) it should probably cancel/ignore the tokenization and allow the user to edit the file as plaintext. I don't know enough about VS Code's internals to know where that change would happen though (in terms of a repo to post an issue to).

@matter123
Copy link

@alexr00 This issue is present even in 1.8.15. Do you want the solution cherry-picked into 1.8.15?

@alexr00
Copy link
Member

alexr00 commented Aug 14, 2019

@matter123 The specific example given in this issue doesn't cause a hang in 1.8.15 and no one else has noticed this issue in that version. The rollback is safe enough, thank you though!

@sean-mcmanus
Copy link
Contributor

sean-mcmanus commented Aug 29, 2019

@alexr00 This bug still repros with VS Code 1.37.1 (reported at microsoft/vscode-cpptools#4158 ). Using the latest Better C++ Syntax extension fixes it. Well, the freezes last for only about 45 seconds...maybe that is "fixed"? VS Code Insiders has the same issue.

@matter123
Copy link

matter123 commented Aug 29, 2019

@sean-mcmanus Only about a third of the time spent syntax highlighting seems to be the evaluation of Regular expressions (6.369s out of 16.819s for vscode-textmate to tokenize every line). If that ratio holds for your 45s time, there are 28 seconds that cannot be fixed on the grammar side.

Edit: Is the 45 seconds on 1.37.1 without "Better C++ Syntax"?

@alexr00 I'll have the stable branch be v1.8.15 with the cherry-pick for vscode 1.38.1

@sean-mcmanus
Copy link
Contributor

Yeah, 45 seconds is without Better C++ Syntax (which gives no freeze at all).

@jeff-hykin
Copy link

there are 28 seconds that cannot be fixed on the grammar side.

@matter123 I don't think it is something beyond the grammar. The C grammar (on Insiders) doesn't freeze on it.

@matter123
Copy link

Yeah, the 28 seconds was if the 45 was with the extensions.

matter123 added a commit to jeff-hykin/better-cpp-syntax that referenced this issue Aug 29, 2019
@matter123
Copy link

matter123 commented Aug 29, 2019

The stable branch at jeff-hykin/cpp-textmate-grammar now contains a fix for this based of v1.8.15, as well as switching from $base to $self as Objective-C no longer needs that.

@alexr00
Copy link
Member

alexr00 commented Sep 2, 2019

@matter123 since we've shipped with v1.8.15 as is for several months and since we are close to release again I'm fine with just leaving it as is without the patch. I'm updating insiders with 1.14.3 though, so we'll have all the fixes there.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
candidate Issue identified as probable candidate for fixing in the next release freeze-slow-crash-leak VS Code crashing, performance, freeze and memory leak issues grammar Syntax highlighting grammar verified Verification succeeded
Projects
None yet
Development

Successfully merging a pull request may close this issue.

8 participants