Notebook Run All mess up with run state spinner

rebornix commented 3 years ago

Issue Type: Bug

Open a long github issue notebook file in OSS
Put cursor in the second code cell
Run all

After all cells finish execution, every cell has a spinner indicating the execution doesn't finsih. However if you log all edits from the extension host, GH NB does set the cell run state to success. Digging into the code a bit I found following checks might be defective:

If there is a spinner, and then it tries to set success state, it will never clear the spinner https://github.com/microsoft/vscode/blob/195779a054748ccdf226c89b4aa8c0385872a534/src/vs/workbench/contrib/notebook/browser/view/renderers/cellRenderer.ts#L1014-L1017
I'm not seeing the RunStateRenderer ever being disposed. Clearing it (RunStateRenderer.clear) seems not enough.

VS Code version: Code - Insiders 1.52.0-insider (a36c68b9ec3d6a0aca9799d7a10be741a6658a51, 2020-11-19T05:38:14.978Z) OS version: Darwin x64 20.1.0

System Info

|Item|Value| |---|---| |CPUs|Intel(R) Core(TM) i9-9980HK CPU @ 2.40GHz (16 x 2400)| |GPU Status|2d_canvas: enabled
flash_3d: enabled
flash_stage3d: enabled
flash_stage3d_baseline: enabled
gpu_compositing: enabled
metal: disabled_off
multiple_raster_threads: enabled_on
oop_rasterization: enabled
opengl: enabled_on
protected_video_decode: unavailable_off
rasterization: enabled
skia_renderer: disabled_off_ok
video_decode: enabled
webgl: enabled
webgl2: enabled| |Load (avg)|3, 2, 2| |Memory (System)|32.00GB (0.47GB free)| |Process Argv|--enable-proposed-api ms-vscode.vscode-notebook-history --enable-proposed-api vscode.notebook-editing --crash-reporter-id 1a907bbd-fa02-4595-81e0-63e1648afb31| |Screen Reader|no| |VM|0%|

Extensions (43)

Extension|Author (truncated)|Version ---|---|--- comment-tagged-templates|bie|0.3.1 github-markdown-preview|bie|0.0.2 markdown-emoji|bie|0.0.9 markdown-preview-github-styles|bie|0.1.6 gitlens|eam|11.0.1 tsl-problem-matcher|eam|0.3.1 vscode-pull-request-github-insiders|Git|2020.11.25442 vscode-fluent-icons|mig|0.0.1 theme-monokai-pro-vscode|mon|1.1.18 vscode-docker|ms-|1.8.0 vscode-language-pack-zh-hans|MS-|1.51.2 csharp|ms-|1.23.6 vscode-dotnet-runtime|ms-|0.2.0 python|ms-|2020.11.367453362 jupyter|ms-|2020.12.367169166-dev notebook-renderers|ms-|0.0.1 remote-containers|ms-|0.150.0 remote-ssh-edit-nightly|ms-|2020.11.24164 remote-ssh-nightly|ms-|2020.11.24164 hexeditor|ms-|1.3.0 js-debug-nightly|ms-|2020.11.1117 powershell-preview|ms-|2020.9.0 vscode-github-issue-notebooks|ms-|0.0.58 vscode-markdown-notebook|ms-|0.0.13 vscode-notebook-history|ms-|0.0.1 vsliveshare|ms-|1.1.0-dev vsonline|ms-|1.0.3075 debugger-for-chrome|msj|4.12.11 toggle|reb|0.0.1 java|red|0.71.0 kuskus-extensions-pack|ros|1.0.1 kuskus-kusto-language-server|ros|1.0.1 kuskus-kusto-syntax-highlighting|ros|1.1.8 vsc-nwjs|rua|1.0.18 rust|rus|0.7.8 sourcekit-lsp|unp|0.0.1 vscode-lldb|vad|1.6.0 vscode-java-debug|vsc|0.29.0 vscode-java-dependency|vsc|0.15.0 vscode-java-pack|vsc|0.10.0 vscode-java-test|vsc|0.26.0 vscode-maven|vsc|0.25.0 notebook-editing|vsc|1.0.0 (5 theme extensions excluded)

A/B Experiments

``` vsliv695:30137379 vsins829:30139715 vsliv368cf:30146710 vsreu685:30147344 openlogontheside:30221882 python763:30178808 python383:30185418 vspor879:30202332 vspor708:30202333 vspor363:30204092 vswsl492cf:30208930 wsl2prompt:30219162 pythonvsdeb440:30224570 unusedprompt:30219164 folderexplorer:30219166 openfilemenucf:30219169 pythonvsded773:30223139 ```

rebornix commented 3 years ago

In OSS, running a single cell can run into this issue as well.

roblourens commented 3 years ago

I can't repro this. The pendingNewState is supposed to be set after the timer expires here https://github.com/microsoft/vscode/blob/195779a054748ccdf226c89b4aa8c0385872a534/src/vs/workbench/contrib/notebook/browser/view/renderers/cellRenderer.ts#L1032

What notebook did you use to see this @rebornix?

rebornix commented 3 years ago

@roblourens I found it hard to reproduce now (when I filed this issue I have 100% reproduce all the time), but I did run into this once with following setup

open .vscode/notebook/endgame.github-issues in OSS
run all
scroll up and down

and then sometimes after all execution finish, I can see some spinner

rebornix commented 3 years ago

@roblourens it seems if you scroll a cell under execution in and out and at the end you might end up with a weird state

spinner

For example, I just tested the endgame.github-issues and run all, as you can see the last second cell has a spinner with 0.0s execution time. After I scrolled up and back, the last cell now has a spinner.

roblourens commented 3 years ago

I still couldn't repro it, but after staring at the logic for a long time, I think I see the issue. Try to verify it tomorrow.

rebornix commented 3 years ago

I guess it's a racing condition.

microsoft / vscode

Notebook Run All mess up with run state spinner #110973