Skip to content

[release/6.0] [wasm][debugger] Fixing the race condition while modifying pending_ops - #80190

Merged
carlossanlop merged 2 commits into
dotnet:release/6.0from
thaystg:thays_backport_65752_partial
Jan 6, 2023
Merged

[release/6.0] [wasm][debugger] Fixing the race condition while modifying pending_ops#80190
carlossanlop merged 2 commits into
dotnet:release/6.0from
thaystg:thays_backport_65752_partial

Conversation

@thaystg

@thaystgthaystg commented Jan 4, 2023

Copy link
Copy Markdown
Member

Fixing the race condition, modifying pending_ops, as discussed here:
#76900

Fixes: #68402

Backport of fdc0d16 to release/6.0

Customer Impact

There isn't a easy way to reproduce it, but the customer is debugging his app, then suddenly this error message is showed, and the debugging session is aborted, the customer needs to start the app again to continue debugging.
fail: Microsoft.WebAssembly.Diagnostics.DevToolsProxy[0] DevToolsProxy::Run: Exception System.ArgumentException: The tasks argument included a null value. (Parameter 'tasks') at System.Threading.Tasks.Task.WhenAny(Task[] tasks) at Microsoft.WebAssembly.Diagnostics.DevToolsProxy.Run(Uri browserUri, WebSocket ideSocket)
There was a racing condition while adding and removing items from pending_ops, that is the list with the messages which were sent from the BrowserDebugProxy, @radical refactored this code and now it's using ChannelWriter and ChannelReader to avoid this racing condition on the pending_ops list.

Testing

Manually tested.

Risk

Low risk, applying a change that is already landed on .net7.

IMPORTANT: Is this backport for a servicing release? If so and this change touches code that ships in a NuGet package, please make certain that you have added any necessary package authoring and gotten it explicitly reviewed.

@thaystg
thaystg requested review from lewing and radicalJanuary 4, 2023 19:11
@ghostghost added the area-Debugger-mono label Jan 4, 2023
@ghostghost assigned thaystgJan 4, 2023
@ghost

ghost commented Jan 4, 2023

Copy link
Copy Markdown

Tagging subscribers to this area: @thaystg
See info in area-owners.md if you want to be subscribed.

Issue Details

Fixing the race condition, modifying pending_ops, as discussed here:
#76900

Backporting: fdc0d16

Author:thaystg
Assignees:-
Labels:

area-Debugger-mono

Milestone:-

@thaystg

Copy link
Copy Markdown
MemberAuthor

@lewing@radical if you think this makes sense please let me know. Then I can send the email to tactics.

@lewinglewing left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Active customer issue on LTS with a well tested fix, makes sense to me if you're confident in the backport.

@thaystgthaystg added the Servicing-consider Issue for next servicing release review label Jan 5, 2023
@thaystgthaystg added Servicing-approved Approved for servicing release and removed Servicing-consider Issue for next servicing release review labels Jan 6, 2023
@carlossanlopcarlossanlop added this to the 7.0.3 milestone Jan 6, 2023
@carlossanlop

Copy link
Copy Markdown
Contributor

Approved by Tactics (7.0.3).
Signed off by area owner.
No OOB changes needed (wasm).
CI failures:

@thaystg

thaystg commented Jan 6, 2023

Copy link
Copy Markdown
MemberAuthor

The wasm failure is not related to the PR. The debugger-tests I ran locally and they are passing.

@carlossanlop

Copy link
Copy Markdown
Contributor

Thanks for checking. Let's get this merged. :shipit:

@carlossanlop
carlossanlop merged commit 2e45bc7 into dotnet:release/6.0Jan 6, 2023
@mwasson74

Copy link
Copy Markdown

Hello, @thaystg!! I do not know the correct terminology to use but how can I tell which version of .NET 6 and when this will be released to the public?

Thank you!!

@carlossanlopcarlossanlop modified the milestones: 7.0.3, 6.0.14Jan 12, 2023
@carlossanlop

Copy link
Copy Markdown
Contributor

@mwasson74 Version 6.0.14 will contain this fix, which is the February Servicing Release.

@mwasson74

Copy link
Copy Markdown

Excellent, thanks so much @carlossanlop!! I saw the milestone was .NET 7 and wasn't sure if it was on the right "track" to being released to a new .NET 6 version.

@carlossanlop

Copy link
Copy Markdown
Contributor

Apologies. I applied the wrong milestone but it's corrected now.

@ghostghost locked as resolved and limited conversation to collaborators Feb 11, 2023
Sign up for freeto subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

area-Debugger-monoServicing-approvedApproved for servicing release

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants

@thaystg@carlossanlop@mwasson74@radical@lewing