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

SCM Decorations missing #100524

Closed
joaomoreno opened this issue Jun 18, 2020 · 9 comments
Closed

SCM Decorations missing #100524

joaomoreno opened this issue Jun 18, 2020 · 9 comments
Assignees
@joaomoreno @jrieken
Labels
bug Issue identified by VS Code Team member as probable bug file-decorations insiders-released Patch has been released in VS Code Insiders scm General SCM compound issues verified Verification succeeded
Milestone
June 2020

Comments

@joaomoreno
Copy link
Member

OK I was able to find 100% repro steps out of sources on master. I can't repro on Insiders tho... 🙈

There's a bit of setup:

git init foo
git init bar
echo hello > foo/file.json
  1. Then, open both folders in a single workspace window
  2. Go to the SCM viewlet, file.json should appear as untracked with the letter U
  3. Click file.json and it will open a diff editor, now in the SCM view it appears as 1,U due to the JSON syntax error
  4. Reload

🐛 It only shows 1 instead of U. As soon as you hit Refresh on the repository, the U appears!

This could also very well be an issue in Git and not decorations. @jrieken Can you check whether the decorations are reaching your model at least?
@joaomoreno joaomoreno added scm General SCM compound issues file-decorations labels Jun 18, 2020
@joaomoreno joaomoreno added this to the June 2020 milestone Jun 18, 2020
@jrieken
Copy link
Member

jrieken commented Jun 19, 2020

Does this repo consistently? Otherwise you can enable trace logging. Whenever I retrieve data it's being logged as trace-message

@jrieken
Copy link
Member

jrieken commented Jun 19, 2020

Go to the SCM viewlet, file.json should appear as untracked with the letter U

This is just in one window, right? The bar windows doesn't show anything. I have followed your steps but wasn't able to reproduce....

@joaomoreno joaomoreno mentioned this issue Jun 21, 2020
Closed
@joaomoreno
Copy link
Member Author

joaomoreno commented Jun 21, 2020 •
edited

OK it seems I can't get 100% repro but I'm more on 50% now. It's also irrelevant to the error decoration, as I am able to repro without that decoration.

After enabling trace logging, that message doesn't show up for those decorations at all. Will dig deeper.

@joaomoreno
Copy link
Member Author

Oh man, it's related to the ignore decorator. When I comment that one out, not only git decorations come much faster but also the bug does not reproduce.

@joaomoreno
Copy link
Member Author

Caught it:

vscode/extensions/git/src/decorationProvider.ts

Line 47 in 1b0f706

queueItem!.queue.set(uri.fsPath, { resolve, reject });

If two provideDecoration calls for the same uri overlap, the second one's resolve and reject will overwrite the first one's, so that first one will never complete! This points to a potential issue in decorations in which if some providers do not complete, this affects other provider's decorations as well. This might also be a pretty ugly memory leak of pending provideDecoration calls, since we might listen on them forever and ever. @jrieken Leaving this investigation to you.

I've fixed it in fad4d8d and now I can't repro no more! 💪

@joaomoreno joaomoreno added the bug Issue identified by VS Code Team member as probable bug label Jun 21, 2020
@jrieken
Copy link
Member

jrieken commented Jun 22, 2020

Caught it:

Hmmm... You saw this in source or actually for real? The refetch while pending fetch shouldn't happen and it would be pretty horrible if it does.

@jrieken
Copy link
Member

jrieken commented Jun 22, 2020

I think what's more likely is that this is happening because the we don't honour the cancellation token that is passed to the provide call. Internally we cancel requests re-fetching them while they are in flight

@joaomoreno
Copy link
Member Author

You saw this in source or actually for real?

Saw it running out of sources, but I have some users reporting similar issues: #98241, #53260, #93701

I think what's more likely is that this is happening because the we don't honour the cancellation token that is passed to the provide call. Internally we cancel requests re-fetching them while they are in flight

Makes sense! Should we still do some work in the core in case people don't respect the token? (Remember you implemented the original ignore decoration provider 😆)

@jrieken
Copy link
Member

jrieken commented Jun 22, 2020

Should we still do some work in the core in case people don't respect the token

We do which makes me wonder how this reproduced... I'll need to step through this to get a repo and an understanding

jrieken added a commit that referenced this issue Jun 23, 2020
@jrieken
more logging for #100524
933e3fa
jrieken added a commit that referenced this issue Jun 23, 2020
@jrieken
send requests per provider so that a hanging provider doesn't block o…
3fd09e6 
…ther providers, #100524
@roblourens roblourens added the verified Verification succeeded label Jul 6, 2020
@github-actions github-actions bot locked and limited conversation to collaborators Aug 5, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees

@joaomoreno joaomoreno

@jrieken jrieken

Labels
bug Issue identified by VS Code Team member as probable bug file-decorations insiders-released Patch has been released in VS Code Insiders scm General SCM compound issues verified Verification succeeded
Projects
None yet
Milestone
June 2020
Development

No branches or pull requests
4 participants
@joaomoreno @roblourens @jrieken and others