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

Potential leak detected in decorations #63467

Closed
mjbvz opened this issue Nov 20, 2018 · 5 comments
Closed

Potential leak detected in decorations #63467

mjbvz opened this issue Nov 20, 2018 · 5 comments
Assignees
@jrieken
Milestone
November 2018

Comments

@mjbvz
Copy link
Contributor

mjbvz commented Nov 20, 2018 •
edited

Issue Type: Bug

While working with VS Code today, I saw these two potential leaks in the dev console:

/Applications/Visual Studio Code - Insiders.app/Contents/Resources/app/out/vs/workbench/workbench.main.js:185 [cf4] potential listener LEAK detected, having 1001 listeners already. MOST frequent listener (501):
e.check @ /Applications/Visual Studio Code - Insiders.app/Contents/Resources/app/out/vs/workbench/workbench.main.js:185
/Applications/Visual Studio Code - Insiders.app/Contents/Resources/app/out/vs/workbench/workbench.main.js:185     at new e (file:///Applications/Visual Studio Code - Insiders.app/Contents/Resources/app/out/vs/workbench/workbench.main.js:2037:828)
    at new e (file:///Applications/Visual Studio Code - Insiders.app/Contents/Resources/app/out/vs/workbench/workbench.main.js:2040:545)
    at file:///Applications/Visual Studio Code - Insiders.app/Contents/Resources/app/out/vs/workbench/workbench.main.js:2047:159
    at file:///Applications/Visual Studio Code - Insiders.app/Contents/Resources/app/out/vs/workbench/workbench.main.js:1926:480
    at file:///Applications/Visual Studio Code - Insiders.app/Contents/Resources/app/out/vs/workbench/workbench.main.js:1926:518
    at V (file:///Applications/Visual Studio Code - Insiders.app/Contents/Resources/app/out/vs/workbench/workbench.main.js:1916:839)
    at t._renderOnce (file:///Applications/Visual Studio Code - Insiders.app/Contents/Resources/app/out/vs/workbench/workbench.main.js:1923:831)
    at t.change (file:///Applications/Visual Studio Code - Insiders.app/Contents/Resources/app/out/vs/workbench/workbench.main.js:1926:254)
    at t.changeViewZones (file:///Applications/Visual Studio Code - Insiders.app/Contents/Resources/app/out/vs/workbench/workbench.main.js:1949:459)
    at file:///Applications/Visual Studio Code - Insiders.app/Contents/Resources/app/out/vs/workbench/workbench.main.js:2046:623
    at o._changeDecorations (file:///Applications/Visual Studio Code - Insiders.app/Contents/Resources/app/out/vs/workbench/workbench.main.js:1207:185)
    at o.changeDecorations (file:///Applications/Visual Studio Code - Insiders.app/Contents/Resources/app/out/vs/workbench/workbench.main.js:1206:478)
    at t.changeDecorations (file:///Applications/Visual Studio Code - Insiders.app/Contents/Resources/app/out/vs/workbench/workbench.main.js:1945:46)
    at e._renderCodeLensSymbols (file:///Applications/Visual Studio Code - Insiders.app/Contents/Resources/app/out/vs/workbench/workbench.main.js:2046:583)
    at file:///Applications/Visual Studio Code - Insiders.app/Contents/Resources/app/out/vs/workbench/workbench.main.js:2044:884
    at <anonymous>
    at process._tickCallback (internal/process/next_tick.js:188:7)
e.check @ /Applications/Visual Studio Code - Insiders.app/Contents/Resources/app/out/vs/workbench/workbench.main.js:185
/Applications/Visual Studio Code - Insiders.app/Contents/Resources/app/out/vs/workbench/workbench.main.js:185 [619] potential listener LEAK detected, having 590 listeners already. MOST frequent listener (187):
e.check @ /Applications/Visual Studio Code - Insiders.app/Contents/Resources/app/out/vs/workbench/workbench.main.js:185
/Applications/Visual Studio Code - Insiders.app/Contents/Resources/app/out/vs/workbench/workbench.main.js:185     at new e (file:///Applications/Visual Studio Code - Insiders.app/Contents/Resources/app/out/vs/workbench/workbench.main.js:1791:516)
    at new e (file:///Applications/Visual Studio Code - Insiders.app/Contents/Resources/app/out/vs/workbench/workbench.main.js:1788:363)
    at t.registerDecorationType (file:///Applications/Visual Studio Code - Insiders.app/Contents/Resources/app/out/vs/workbench/workbench.main.js:1787:605)
    at t._registerDecorationType (file:///Applications/Visual Studio Code - Insiders.app/Contents/Resources/app/out/vs/workbench/workbench.main.js:1955:219)
    at t.setDecorations (file:///Applications/Visual Studio Code - Insiders.app/Contents/Resources/app/out/vs/workbench/workbench.main.js:1945:655)
    at e.setDecorations (file:///Applications/Visual Studio Code - Insiders.app/Contents/Resources/app/out/vs/workbench/workbench.main.js:3238:409)
    at e.$trySetDecorations (file:///Applications/Visual Studio Code - Insiders.app/Contents/Resources/app/out/vs/workbench/workbench.main.js:3354:80)
    at t._doInvokeHandler (file:///Applications/Visual Studio Code - Insiders.app/Contents/Resources/app/out/vs/workbench/workbench.main.js:3293:963)
    at t._invokeHandler (file:///Applications/Visual Studio Code - Insiders.app/Contents/Resources/app/out/vs/workbench/workbench.main.js:3293:635)
    at t._receiveRequest (file:///Applications/Visual Studio Code - Insiders.app/Contents/Resources/app/out/vs/workbench/workbench.main.js:3292:262)
    at t._receiveOneMessage (file:///Applications/Visual Studio Code - Insiders.app/Contents/Resources/app/out/vs/workbench/workbench.main.js:3291:126)
    at file:///Applications/Visual Studio Code - Insiders.app/Contents/Resources/app/out/vs/workbench/workbench.main.js:3289:10
    at e.fire (file:///Applications/Visual Studio Code - Insiders.app/Contents/Resources/app/out/vs/workbench/workbench.main.js:186:466)
    at a (file:///Applications/Visual Studio Code - Insiders.app/Contents/Resources/app/out/vs/workbench/workbench.main.js:342:213)
    at Socket.n._socketDataListener (file:///Applications/Visual Studio Code - Insiders.app/Contents/Resources/app/out/vs/workbench/workbench.main.js:342:434)
    at emitOne (events.js:116:13)
    at Socket.emit (events.js:211:7)
    at addChunk (_stream_readable.js:263:12)
    at readableAddChunk (_stream_readable.js:250:11)
    at Socket.Readable.push (_stream_readable.js:208:10)
    at Pipe.onread (net.js:594:20)
e.check @ /Applications/Visual Studio Code - Insiders.app/Contents/Resources/app/out/vs/workbench/workbench.main.js:185

Sorry, no clear repo steps. I first noticed this when I was in a side-by-side diff view and trying to stage a change the selected range. This operation took quite a while.

VS Code version: Code - Insiders 1.30.0-insider (ebe9ea8, 2018-11-19T19:59:44.607Z)
OS version: Darwin x64 18.2.0

System Info
Item Value
CPUs Intel(R) Core(TM) i7-4770HQ CPU @ 2.20GHz (8 x 2200)
GPU Status 2d_canvas: enabled
checker_imaging: disabled_off
flash_3d: enabled
flash_stage3d: enabled
flash_stage3d_baseline: enabled
gpu_compositing: enabled
multiple_raster_threads: enabled_on
native_gpu_memory_buffers: enabled
rasterization: enabled
video_decode: enabled
video_encode: enabled
webgl: enabled
webgl2: enabled
Load (avg) 3, 3, 3
Memory (System) 16.00GB (0.12GB free)
Process Argv -psn_0_15298198
Screen Reader no
VM 0%
Extensions (30)
Extension Author (truncated) Version
color-info bie 0.5.0
comment-tagged-templates bie 0.2.0
emojisense bie 0.4.1
folder-source-actions bie 0.0.4
github-markdown-preview bie 0.0.2
lit-html bie 1.10.1
markdown-checkbox bie 0.1.1
markdown-emoji bie 0.0.7
markdown-image-size bie 0.0.2
markdown-mermaid bie 1.0.0
markdown-preview-github-styles bie 0.1.4
markdown-yaml-preamble bie 0.0.2
speech bie 0.0.2
vscode-eslint dba 1.7.0
gitlens eam 8.5.6
vscode-npm-script eg2 0.3.5
vscode-pull-request-github Git 0.2.3
vscode-memfs jri 0.0.3
theme-karyfoundation-themes kar 18.2.1
vscode-azurefunctions ms- 0.12.0
vscode-cosmosdb ms- 0.9.0
vscode-language-pack-de MS- 1.29.2
vscode-language-pack-zh-hans MS- 1.29.2
mssql ms- 1.4.0
python ms- 2018.10.1
azure-account ms- 0.7.0
Go ms- 0.7.0
vscode-typescript-tslint-plugin ms- 0.1.0
debugger-for-chrome msj 4.11.1
java red 0.34.0

(16 theme extensions excluded)
@vscodebot vscodebot bot added the insiders label Nov 20, 2018
mjbvz added a commit that referenced this issue Nov 20, 2018
@mjbvz
Always dispose of DecorationCSSRules
d37f7c1 
Found while looking into #63467

There's potential leak for `DecorationCSSRules`. Currently we only dipose of them if `hasContent` is true, which I'm not sure guarantees that these classes are truely non-disposable

With this change, we always dispose of them. At worst, these extra dispose call should be noops
@mjbvz
Copy link
Contributor Author

mjbvz commented Nov 20, 2018

d37f7c1 looked into one potential leak. Not sure if this was the one causing the issue or not since I can't reliably repo the problem

@alexdima
Copy link
Member

Deminified using https://github.com/alexandrudima/vscode-stack-beautifier

First stack:

That listener was introduced via 5005b5b @jrieken

  at onDidChangeConfiguration (./editor/contrib/codelens/codelensWidget.ts:87:38)
  at ./editor/contrib/codelens/codelensWidget.ts:247:24
  at ./editor/contrib/codelens/codelensController.ts:253:23
  at func (./editor/browser/view/viewImpl.ts:589:9)
  at callback (./editor/browser/view/viewImpl.ts:498:18)
  at func (./editor/browser/view/viewImpl.ts:581:9)
  at safeInvokeNoArg (./editor/browser/view/viewImpl.ts:359:10)
  at _renderOnce (./editor/browser/view/viewImpl.ts:478:7)
  at change (./editor/browser/widget/codeEditorWidget.ts:1247:40)
  at changeViewZones (./editor/contrib/codelens/codelensController.ts:222:16)
  at callback (./editor/common/model/textModel.ts:1502:12)
  at _changeDecorations (./editor/common/model/textModel.ts:1467:15)
  at changeDecorations (./editor/browser/widget/codeEditorWidget.ts:1019:31)
  at changeDecorations (./editor/contrib/codelens/codelensController.ts:221:15)
  at _renderCodeLensSymbols (./editor/contrib/codelens/codelensController.ts:121:10)
  at hasListeners (./base/common/event.ts:302:11)

Second stack:

Might be fixed by your change d37f7c1 @mjbvz .

  at ./editor/browser/services/codeEditorServiceImpl.ts:91:28
  at ./editor/browser/services/codeEditorServiceImpl.ts:42:15
  at registerDecorationType (./editor/browser/widget/codeEditorWidget.ts:1495:26)
  at _registerDecorationType (./editor/browser/widget/codeEditorWidget.ts:1060:10)
  at setDecorations (./workbench/api/electron-browser/mainThreadEditor.ts:386:19)
  at setDecorations (./workbench/api/electron-browser/mainThreadEditors.ts:175:42)
  at apply (./workbench/services/extensions/node/rpcProtocol.ts:461:16)
  at _doInvokeHandler (./workbench/services/extensions/node/rpcProtocol.ts:446:31)
  at _invokeHandler (./workbench/services/extensions/node/rpcProtocol.ts:366:18)
  at _receiveRequest (./workbench/services/extensions/node/rpcProtocol.ts:296:9)
  at _receiveOneMessage (./workbench/services/extensions/node/rpcProtocol.ts:171:41)
  at call (./base/common/event.ts:212:15)
  at fire (./base/parts/ipc/node/ipc.net.ts:103:22)
  at acceptChunk (./base/parts/ipc/node/ipc.net.ts:132:3)
  at idx (./base/common/paths.ts:48:5)
  at Promise (./base/common/actions.ts:196:10)
  at _shouldRender (./base/browser/ui/scrollbar/horizontalScrollbar.ts:72:7)
  at _revealTimer (./base/browser/ui/scrollbar/scrollbarVisibilityController.ts:91:7)
  at prototype (./base/browser/ui/tree/objectTreeModel.ts:98:1)
  at Position (./editor/common/model/pieceTreeTextBuffer/pieceTreeBase.ts:437:16)

@alexdima alexdima removed their assignment Nov 20, 2018
@jrieken
Copy link
Member

jrieken commented Nov 22, 2018

the code lens widgets don't leak but each widget is a listener which is maybe not the smartest...

jrieken added a commit that referenced this issue Nov 22, 2018
@jrieken
have just one editor config listener #63467
2448fa4
jrieken added a commit that referenced this issue Nov 22, 2018
@jrieken
use editor mouse listener instead of multiple listeners #63467
d669936
jrieken added a commit that referenced this issue Nov 22, 2018
@jrieken
dispose listener #63467
4852f20
@jrieken
Copy link
Member

jrieken commented Nov 22, 2018

Pushed one more change to fix an actual leak, but that grows with the number of editors only.

@jrieken
Copy link
Member

jrieken commented Nov 22, 2018

Closing this and seeing if this occurs again... Per editor code is now just 1 listener, not one per lens. Unsure what the other 500 listeners are...

@jrieken jrieken closed this as completed Nov 22, 2018
@jrieken jrieken added this to the November 2018 milestone Nov 28, 2018
@jrieken jrieken mentioned this issue Nov 28, 2018
Closed
@vscodebot vscodebot bot locked and limited conversation to collaborators Jan 6, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees

@jrieken jrieken

Labels
None yet
Projects
None yet
Milestone
November 2018
Development

No branches or pull requests
3 participants
@jrieken @alexdima @mjbvz