8000 Updating progress pane using timer causes race condition that would randomly crash powershell · Issue #2800 · PowerShell/PowerShell · GitHub
[go: up one dir, main page]
Skip to content

Updating progress pane using timer causes race condition that would randomly crash powershell #2800

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
Closed
daxian-dbw opened this issue Nov 29, 2016 · 4 comments · Fixed by #2806
Closed

Updating progress pane using timer causes race condition that would randomly crash powershell #2800

daxian-dbw opened this issue Nov 29, 2016 · 4 comments · Fixed by #2806
Labels
Issue-Bug Issue has been identified as a bug in the product Resolution-Duplicate The issue is a duplicate. WG-Interactive-Console the console experience

Comments

@daxian-dbw
Copy link
Member

With PR #2640, a timer is used to trigger updating progress pane every 100 ms to improve progress record rendering performance (commit: 760a9c2). Before the change, _progPane.Show and _pendingProgress.Update used to be called on the same thread, and now they will be called on different threads. This causes race conditions -- for all type members that are used by both methods, they might be changed at the same time and the states may be corrupted.

The screenshot below is from a recent CI build on master branch. The exception was raised from RenderHelper, and that was because it was actually enumerating over PendingProgress._topLevelNodes and at the same time new node was being added to/removed from _topLevelNodes by Update.

image

/cc @iSazonov @lzybkr
@daxian-dbw daxian-dbw added WG-Interactive-Console the console experience Issue-Bug Issue has been identified as a bug in the product labels Nov 29, 2016
@iSazonov
Copy link
Collaborator

@daxian-dbw Is the #2771 addressed this?

@iSazonov
Copy link
Collaborator
iSazonov commented Nov 29, 2016 •
edited
Loading

@daxian-dbw I specially made restart the timer after completing _progPane.Show to exclude collisions but missed the need to control in HandleIncomingProgressRecord.
Perhaps it would be easier to add a lock control in the HandleIncomingProgressRecord and ProgressPaneUpdateTimerElapsed.
/cc @SteveL-MSFT

@lzybkr
Copy link
Contributor
lzybkr commented Nov 29, 2016

Does #2771 fix this?

@SteveL-MSFT SteveL-MSFT added the Resolution-Duplicate The issue is a duplicate. label Nov 29, 2016
@daxian-dbw
Copy link
Member Author

Ah, I didn't know we already had an issue for this: #2756.
However, _topLevelNodes is not the only type member that is shared between the main pipeline thread and _progPane.Show running on the timer thread, _rawui and _progressRegion are another 2 examples. I think we need to analyze all those members in case any of them have similar race conditions.

75B6
This was referenced Nov 30, 2016
Merged
Closed
@iSazonov iSazonov mentioned this issue Dec 1, 2016
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
Issue-Bug Issue has been identified as a bug in the product Resolution-Duplicate The issue is a duplicate. WG-Interactive-Console the console experience
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Fix the CI build break daxian-dbw/PowerShell
4 participants
@daxian-dbw @lzybkr @SteveL-MSFT @iSazonov
0