"[QUIC] Adopted msquic generated interop (#68288)" attempt no 2 #69009

ManickaP · 2022-05-07T11:05:02Z

This reverts commit 4820674.

Brings back #68288

Fixes #67377, #67301, #68954

…" (dotnet#68940)" This reverts commit 4820674.

ghost · 2022-05-07T11:05:10Z

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

Issue Details

This reverts commit 4820674.

Brings back #68288

Author:	ManickaP
Assignees:	ManickaP
Labels:	`area-System.Net.Quic`
Milestone:	-

ManickaP · 2022-05-08T19:08:41Z

So the AOT itself is not failing anymore, but now I get:

Failed to load AOT module '/root/helix/work/correlation/System.Net.Quic.dll.so' ('/root/helix/work/correlation/System.Net.Quic.dll') in aot-only mode.

In Work Item JIT.Regression.JitBlue: https://helixre8s23ayyeko0k025g8.blob.core.windows.net/dotnet-runtime-refs-pull-69009-merge-bd97177d9b0441c6b0/JIT.Regression.JitBlue/1/console.ca962c5e.log?helixlogtype=result

@akoeplinger could help with this or do you know who could?

ManickaP · 2022-05-09T13:52:51Z

I have a fix for #68954 ready, but I don't want to stop the pipeline by pushing it right now.

ManickaP · 2022-05-11T08:13:15Z

src/libraries/Common/tests/System/Net/Http/Http3LoopbackConnection.cs

+            Http3LoopbackStream requestStream = null;

            while (true)
            {
-                stream = await AcceptStreamAsync().ConfigureAwait(false);
+                var stream = await AcceptStreamAsync().ConfigureAwait(false);

+                // Accepted request stream.
                if (stream.CanWrite)
                {
-                    return stream;
+                    // Only one expected.
+                    Assert.True(requestStream is null, "Expected single request stream, got a second");
+
+                    // Control stream is set --> return the request stream.
+                    if (_inboundControlStream is not null)
+                    {
+                        return stream;
+                    }
+
+                    // Control stream not set --> need to accept another stream.
+                    requestStream = stream;
+                    continue;
                }

-                // Must be the control stream
+                // Must be the control stream.
                await HandleControlStreamAsync(stream);
+
+                // We've already accepted request stream --> return it.
+                if (requestStream is not null)
+                {
+                    return requestStream;
+                }


This is fix for the assert problem from #68954

ManickaP · 2022-05-11T08:17:24Z

src/libraries/System.Net.Quic/src/System/Net/Quic/Implementations/MsQuic/MsQuicStream.cs

+
+            // Check if we already got final event.
+            bool releaseHandles = Interlocked.Exchange(ref _state.ShutdownDone, State.ShutdownDone_Disposed) == State.ShutdownDone_NotificationReceived;
            if (releaseHandles)
            {
                _state.Cleanup();
            }



This together with lines 871-877 in the original are a fix for the use after free part in #68954
Note that the removed: _state.ShutdownState = ShutdownState.Finished; is actually set in _state.Cleanup();:

runtime/src/libraries/System.Net.Quic/src/System/Net/Quic/Implementations/MsQuic/MsQuicStream.cs

Line 94 in 6ca8c9b

ShutdownState = ShutdownState.Finished;

ManickaP · 2022-05-11T08:17:58Z

src/tests/build.proj

    <ItemGroup>
      <TestsAndAssociatedAssemblies Include="%(TestDirs.Identity)/*.dll" Exclude="@(NoMonoAotAssemblyPaths)" />
-      <CoreRootDlls Include="$(CORE_ROOT)/*.dll" Exclude="$(CORE_ROOT)/xunit.performance.api.dll" />
+      <CoreRootDlls Include="$(CORE_ROOT)/*.dll" Exclude="$(CORE_ROOT)/xunit.performance.api.dll;$(CORE_ROOT)/System.Net.Quic.dll" />


Excludes us from mono AOT

ManickaP · 2022-05-11T08:18:37Z

src/tests/issues.targets

+
+        <ExcludeList Include="$(XunitTestBinBase)/JIT/Performance/RunBenchmarks/RunBenchmarks/**">
+            <Issue>https://github.com/dotnet/runtime/issues/52977</Issue>
+        </ExcludeList>
+        <ExcludeList Include="$(XunitTestBinBase)/JIT/Regression/JitBlue/GitHub_25468/GitHub_25468/**">
+            <Issue>https://github.com/dotnet/runtime/issues/52977</Issue>
+        </ExcludeList>
+        <ExcludeList Include="$(XunitTestBinBase)/JIT/CheckProjects/CheckProjects/**">
+            <Issue>https://github.com/dotnet/runtime/issues/52977</Issue>
+        </ExcludeList>
+        <ExcludeList Include="$(XunitTestBinBase)/ JIT/Performance/CodeQuality/BenchmarksGame/k-nucleotide/k-nucleotide-9/**">
+            <Issue>https://github.com/dotnet/runtime/issues/52977</Issue>
+        </ExcludeList>


Skips tests failing due to missing mono AOT generated System.Net.Quic.dll.so.

ManickaP · 2022-05-11T08:19:17Z

The pipeline is green and mono llvmfullaot ran: https://dev.azure.com/dnceng/public/_build/results?buildId=1763129&view=results

Can I get a review here? The PR is same as #68288 with the exceptions marked with comments, which are fixes for mono AOT error and #68954

rzikm

LGTM!

Revert "Revert "[QUIC] Adopted msquic generated interop (dotnet#68288)…

b8f3819

…" (dotnet#68940)" This reverts commit 4820674.

ghost assigned ManickaP May 7, 2022

ghost added the area-System.Net.Quic label May 7, 2022

ManickaP changed the title ~~Revert "Revert "[QUIC] Adopted msquic generated interop (#68288)" (#68940)~~ "[QUIC] Adopted msquic generated interop (#68288)" attempt no 2 May 7, 2022

ManickaP force-pushed the mapichov/quic-interop-aot branch 2 times, most recently from fb127ca to bcace7d Compare May 9, 2022 13:09

runfoapp bot mentioned this pull request May 9, 2022

jit.1 work item failing on mono #67888

Closed

ManickaP mentioned this pull request May 10, 2022

[mono] Avoid an assertion when cattrs with array parameters are const… #69077

Merged

ManickaP added 2 commits May 10, 2022 22:03

Exclude S.N.Quic from Mono AOT

6f28426

Fix dotnet#68954

3243fc1

ManickaP force-pushed the mapichov/quic-interop-aot branch from 638702b to 3243fc1 Compare May 10, 2022 20:03

ManickaP commented May 11, 2022

View reviewed changes

ManickaP marked this pull request as ready for review May 11, 2022 08:19

ManickaP requested review from a team, CarnaViire, rzikm and wfurt May 11, 2022 08:19

dotnet deleted a comment from azure-pipelines bot May 11, 2022

rzikm approved these changes May 11, 2022

View reviewed changes

ManickaP merged commit 122c9d0 into dotnet:main May 11, 2022

ManickaP deleted the mapichov/quic-interop-aot branch May 11, 2022 14:29

jakobbotsch mentioned this pull request May 11, 2022

Disable k-nucleotide-9 in llvmfullaot #69173

Merged

ghost locked as resolved and limited conversation to collaborators Jun 10, 2022

karelz added this to the 7.0.0 milestone Jul 19, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

"[QUIC] Adopted msquic generated interop (#68288)" attempt no 2 #69009

"[QUIC] Adopted msquic generated interop (#68288)" attempt no 2 #69009

Uh oh!

ManickaP commented May 7, 2022 •

edited

Loading

Uh oh!

ghost commented May 7, 2022

Uh oh!

ManickaP commented May 8, 2022

Uh oh!

ManickaP commented May 9, 2022

Uh oh!

ManickaP May 11, 2022

Uh oh!

ManickaP May 11, 2022

Uh oh!

ManickaP May 11, 2022

Uh oh!

ManickaP May 11, 2022

Uh oh!

ManickaP commented May 11, 2022 •

edited

Loading

Uh oh!

rzikm left a comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

"[QUIC] Adopted msquic generated interop (#68288)" attempt no 2 #69009

"[QUIC] Adopted msquic generated interop (#68288)" attempt no 2 #69009

Uh oh!

Conversation

ManickaP commented May 7, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ghost commented May 7, 2022

Uh oh!

ManickaP commented May 8, 2022

Uh oh!

ManickaP commented May 9, 2022

Uh oh!

ManickaP May 11, 2022

Choose a reason for hiding this comment

Uh oh!

ManickaP May 11, 2022

Choose a reason for hiding this comment

Uh oh!

ManickaP May 11, 2022

Choose a reason for hiding this comment

Uh oh!

ManickaP May 11, 2022

Choose a reason for hiding this comment

Uh oh!

ManickaP commented May 11, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

rzikm left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

ManickaP commented May 7, 2022 •

edited

Loading

ManickaP commented May 11, 2022 •

edited

Loading