"[QUIC] Adopted msquic generated interop (#68288)" attempt no 2#69009
"[QUIC] Adopted msquic generated interop (#68288)" attempt no 2#69009ManickaP merged 3 commits intodotnet:mainfrom
Conversation
…" (dotnet#68940)" This reverts commit 4820674.
|
Tagging subscribers to this area: @dotnet/ncl |
|
So the AOT itself is not failing anymore, but now I get: 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? |
fb127ca to
bcace7d
Compare
|
I have a fix for #68954 ready, but I don't want to stop the pipeline by pushing it right now. |
638702b to
3243fc1
Compare
| 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; | ||
| } |
There was a problem hiding this comment.
This is fix for the assert problem from #68954
|
|
||
| // Check if we already got final event. | ||
| bool releaseHandles = Interlocked.Exchange(ref _state.ShutdownDone, State.ShutdownDone_Disposed) == State.ShutdownDone_NotificationReceived; | ||
| if (releaseHandles) | ||
| { | ||
| _state.Cleanup(); | ||
| } | ||
|
|
There was a problem hiding this comment.
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();:
| <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" /> |
There was a problem hiding this comment.
Excludes us from mono AOT
|
|
||
| <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> |
There was a problem hiding this comment.
Skips tests failing due to missing mono AOT generated System.Net.Quic.dll.so.
|
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 |
This reverts commit 4820674.
Brings back #68288
Fixes #67377, #67301, #68954