Skip to content

load msquic early to avoid interference with RemoteExecutor - #75424

Merged
wfurt merged 6 commits into
dotnet:mainfrom
wfurt:quicLoad
Sep 12, 2022
Merged

load msquic early to avoid interference with RemoteExecutor#75424
wfurt merged 6 commits into
dotnet:mainfrom
wfurt:quicLoad

Conversation

@wfurt

Copy link
Copy Markdown
Member

When msquic is loaded it will also initialize lttng. That calls wait and it can reap processes from RemoteExecutor and that will cause Assert in Process code. (long story and evidence is in #74795)

While the problem really lives in lttng there is no quick fix for that. To void the problem this change tries to load MsQuic early via static variable and that will initialize lttng before any tests actually start.

I wrote GetIsQuicSupported so we can do more there if needed. e.g. call some msquic API to load everything permanently. It is not needed at this point so I left it out.

I had test runs over nigh and I did not see any problems on system where it was failing before.
fixes#74795

@wfurtwfurt added test-enhancement Improvements of test source code area-System.Net.Quic labels Sep 11, 2022
@wfurt
wfurt requested review from a team and jkotasSeptember 11, 2022 22:37
@wfurtwfurt self-assigned this Sep 11, 2022
@ghost

Copy link
Copy Markdown

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

Issue Details

When msquic is loaded it will also initialize lttng. That calls wait and it can reap processes from RemoteExecutor and that will cause Assert in Process code. (long story and evidence is in #74795)

While the problem really lives in lttng there is no quick fix for that. To void the problem this change tries to load MsQuic early via static variable and that will initialize lttng before any tests actually start.

I wrote GetIsQuicSupported so we can do more there if needed. e.g. call some msquic API to load everything permanently. It is not needed at this point so I left it out.

I had test runs over nigh and I did not see any problems on system where it was failing before.
fixes #74795

Author:wfurt
Assignees:wfurt
Labels:

test-enhancement, area-System.Net.Quic

Milestone:-

@jkotas

Copy link
Copy Markdown
Member

Could you please re-enable disabled tests?

[assembly:ActiveIssue("https://github.com/dotnet/runtime/issues/74795",typeof(PlatformDetection),nameof(PlatformDetection.IsMonoLinuxArm64))]

Comment threadsrc/libraries/System.Net.Requests/tests/System.Net.Requests.Tests.csproj Outdated
Comment threadsrc/libraries/Common/tests/System/Net/Capability.Quic.cs Outdated
Comment threadsrc/libraries/System.Net.Requests/tests/QuicLoad.cs Outdated

@jkotasjkotas 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.

LGTM otherwise

Comment threadsrc/libraries/System.Net.Requests/tests/QuicLoad.cs
@karelzkarelz added this to the 8.0.0 milestone Sep 12, 2022
@wfurt

Copy link
Copy Markdown
MemberAuthor

Run tests in loop since yesterday and still looks good. macOS test failures are unrelated.

@wfurt
wfurt merged commit c4fa886 into dotnet:mainSep 12, 2022
@wfurt
wfurt deleted the quicLoad branch September 12, 2022 17:43
@wfurt

Copy link
Copy Markdown
MemberAuthor

/backport to release/7.0

@github-actions

Copy link
Copy Markdown
Contributor

Started backporting to release/7.0: https://github.com/dotnet/runtime/actions/runs/3039384318

Sign up for freeto subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

area-System.Net.Quictest-enhancementImprovements of test source code

Projects

None yet

Development

Successfully merging this pull request may close these issues.

assert in ProcessWaitState on Linux arm64

4 participants

@wfurt@jkotas@liveans@karelz