dotnet / runtimelab

This repo is for experimentation and exploring new ideas that may or may not make it into the main dotnet/runtime repo.
MIT License
1.36k stars 188 forks source link

[NativeAOT-LLVM] support `System.Net.Http.HttpClient` on WASIp2 #2614

Open dicej opened 2 weeks ago

dicej commented 2 weeks ago

This adds WasiHttpHandler, a new implementation of HttpMessageHandler based on wasi:http/outgoing-handler, plus tweaks to System.Threading to allow async Tasks to work in a single-threaded context, with ThreadPool work items dispatched from an application-provided event loop.

WASIp2 supports asynchronous I/O and timers via wasi:io/poll/pollable resource handles. One or more of those handles may be passed to wasi:io/poll/poll, which will block until at least one of them is ready. In order to make this model play nice with C#'s async/await and Task features, we need to reconcile several constraints:

The solution we arrived at looks something like this:

The upshot is that application code can use wit-bindgen (either directly or via the new componentize-dotnet package) to generate async export bindings which will provide an event loop backed by Thread.Dispatch. Additionally, wit-bindgen can transparently convert any pollable handles returned by WASI imports into Tasks via Thread.Register, allowing the component to await them, pass them to a combinator such as Task.WhenEach, etc.

Later, when WASIp3 arrives and we update the .NET runtime to target it, we'll be able to remove some of this code (and the corresponding code in wit-bindgen) without requiring significant changes to the application developer's experience.

This PR contains a few C# source files that were generated by wit-bindgen from the official WASI WIT files, plus scripts to regenerate them as desired.

dicej commented 2 weeks ago

TODO items:

dicej commented 2 weeks ago

Ah, that CI failure was useful; now I see how WASI tests are currently run. Looks like I'll need to make some changes to convert the test module(s) into component(s) and run them on a WASIp2-compatible runtime.

dicej commented 6 days ago

Status update on this: CI is green, and I've manually tested WasiHttpHandler by way of HttpClient to verify it works. However, I could use some guidance on how to automate this testing as part of CI.

AFAICT, CI currently only runs the library smoke tests for both the WASI and browser targets. I can see that e.g. ResponseStreamTest has methods like BrowserHttpHandler_Streaming, so it looks like there is test coverage for TARGET_BROWSER, but I'm not seeing how or where those tests are run, nor how to replicate them for WASI.

I've tried running the System.Net.Http.Functional.Tests suite, but have not had much success with that (many of the tests rely directly or indirectly on TaskAwaiter.GetResult, which leads to an infinite busy wait), and I'm not sure if debugging that is a good use of time.

So my question is: What's the best (read: most practical and efficient) way to add test coverage for WasiHttpHandler such that it can be run as part of CI (or elsewhere if there's some other automated testing infrastructure I'm missing)?

Meanwhile, I'm going to mark this "ready for review" since I believe it's ready for feedback even with the testing question unresolved.

pavelsavara commented 3 days ago

Meanwhile, I'm going to mark this "ready for review" since I believe it's ready for feedback even with the testing question unresolved.

Yeah, enabling more tests should be independent PR.

It takes non-trivial effort in the first pass.

Also because at the same time it will test host's HTTP stack and interop, including HTTP edge cases.

I've tried running the System.Net.Http.Functional.Tests suite

That's one we want to use but we only use tests which are fully async. Meaning tests which don't contain blocking Task.Wait and yield to the host/browser.

There is TARGET_BROWSER and PlatformDetection.IsBrowser with variations. We will need to use the same/similar conditional tests.

[ConditionalTheory(typeof(PlatformDetection), nameof(PlatformDetection.IsBrowserDomSupportedOrNotBrowser))]

[ConditionalFact(typeof(PlatformDetection), nameof(PlatformDetection.IsChromium))]

[ConditionalFact(typeof(PlatformDetection), nameof(PlatformDetection.IsBrowser))]

[ConditionalClass(typeof(PlatformDetection), nameof(PlatformDetection.IsBrowserDomSupportedOrNotBrowser))]

https://github.com/dotnet/runtime/blob/f8bcb89796d3c83264fffd913e5196d29d8d730e/src/libraries/Common/tests/TestUtilities/System/PlatformDetection.cs#L50-L53

https://github.com/dotnet/runtime/blob/f8bcb89796d3c83264fffd913e5196d29d8d730e/src/libraries/Common/tests/TestUtilities/System/PlatformDetection.cs#L147-L151

The HTTP server role is played by NetCoreServer for browser. https://github.com/dotnet/runtime/tree/main/src/libraries/Common/tests/System/Net/Prerequisites/NetCoreServer

https://github.com/dotnet/runtime/blob/f8bcb89796d3c83264fffd913e5196d29d8d730e/src/libraries/Common/tests/System/Net/Prerequisites/LocalEchoServer.props#L15-L19

https://github.com/dotnet/runtime/blob/main/src/libraries/Common/tests/System/Net/Configuration.Http.cs

Also note, that some of the tests assume HTTP server in the same process. With browser that's not possible and so we offload the HTTP server role to xharness (helper which is driving the browser). They communicate via WebSocket, which we don't have for WASI yet. See https://github.com/dotnet/runtime/tree/main/src/libraries/Common/tests/System/Net/Prerequisites/RemoteLoopServer

Things which are broken should be marked with filter for broken scenario and a link to GH issue for it.

[ActiveIssue("https://github.com/dotnet/runtime/issues/50957", typeof(PlatformDetection), nameof(PlatformDetection.IsBrowser), nameof(PlatformDetection.IsMonoAOT))]
dicej commented 17 hours ago

I've added a minimal smoke test for HttpClient using a loopback HTTP server I added to SharedLibraryDriver.mjs. I'll add a few more tests next week.

pavelsavara commented 6 hours ago

There is CI fail

D:/a/_work/1/s/src/native/libs/System.Native/pal_process_wasi.c(78): error 0: Not supported on WASI (false failed) [D:\a\_work\1\s\src\tests\build.proj]
          Assertion failed: false && "assert_msg failed" (D:/a/_work/1/s/src/native/libs/System.Native/pal_process_wasi.c: SystemNative_SysLog: 78)
          wasm://wasm/.tmpp9kfF5-086e5c86:1

          RuntimeError: unreachable
              at .tmpp9kfF5.abort (wasm://wasm/.tmpp9kfF5-086e5c86:wasm-function[27516]:0x137db36)
              at .tmpp9kfF5.__assert_fail (wasm://wasm/.tmpp9kfF5-086e5c86:wasm-function[27570]:0x137f867)
              at .tmpp9kfF5.SystemNative_SysLog (wasm://wasm/.tmpp9kfF5-086e5c86:wasm-function[27284]:0x137716b)
              at .tmpp9kfF5.S_P_CoreLib_Interop_Sys___SysLog_g____PInvoke_133_0 (wasm://wasm/.tmpp9kfF5-086e5c86:wasm-function[2419]:0x185d27)
              at .tmpp9kfF5.S_P_CoreLib_Interop_Sys__SysLog (wasm://wasm/.tmpp9kfF5-086e5c86:wasm-function[17536]:0x10ea28b)
              at .tmpp9kfF5.S_P_CoreLib_System_Diagnostics_DebugProvider__WriteToDebugger (wasm://wasm/.tmpp9kfF5-086e5c86:wasm-function[2226]:0x13cd50)
              at .tmpp9kfF5.S_P_CoreLib_System_Diagnostics_DebugProvider__WriteCore (wasm://wasm/.tmpp9kfF5-086e5c86:wasm-function[17375]:0x10bf00c)
              at .tmpp9kfF5.S_P_CoreLib_System_Diagnostics_DebugProvider__Write (wasm://wasm/.tmpp9kfF5-086e5c86:wasm-function[2090]:0x12564d)
              at .tmpp9kfF5.S_P_CoreLib_System_Diagnostics_DebugProvider__WriteLine (wasm://wasm/.tmpp9kfF5-086e5c86:wasm-function[17235]:0x10a3c44)
              at .tmpp9kfF5.S_P_CoreLib_System_Diagnostics_DebugProvider__WriteAssert (wasm://wasm/.tmpp9kfF5-086e5c86:wasm-function[1970]:0x1119c6)
              at .tmpp9kfF5.S_P_CoreLib_System_Diagnostics_DebugProvider__Fail (wasm://wasm/.tmpp9kfF5-086e5c86:wasm-function[17128]:0x108dd7e)
              at .tmpp9kfF5.S_P_CoreLib_System_Diagnostics_Debug__Fail_0 (wasm://wasm/.tmpp9kfF5-086e5c86:wasm-function[14921]:0xe73e04)
              at .tmpp9kfF5.S_P_CoreLib_System_Diagnostics_Debug__Assert_2 (wasm://wasm/.tmpp9kfF5-086e5c86:wasm-function[4060]:0x330add)
              at .tmpp9kfF5.S_P_CoreLib_System_Diagnostics_Debug__Assert (wasm://wasm/.tmpp9kfF5-086e5c86:wasm-function[14889]:0xe6fa62)
              at .tmpp9kfF5.SharedLibrary_LibraryWorld_LibraryWorldImpl__TestHttpAsync_d__1__MoveNext (wasm://wasm/.tmpp9kfF5-086e5c86:wasm-function[1926]:0x105f20)
              at .tmpp9kfF5.S_P_CoreLib_System_Runtime_CompilerServices_AsyncTaskMethodBuilder_1_AsyncStateMachineBox_1<S_P_CoreLib_System_Threading_Tasks_VoidTaskResult__System___Canon>__ExecutionContextCallback (wasm://wasm/.tmpp9kfF5-086e5c86:wasm-function[17647]:0x1102238)
              at .tmpp9kfF5.S_P_CoreLib_System_Threading_ContextCallback__InvokeOpenStaticThunk (wasm://wasm/.tmpp9kfF5-086e5c86:wasm-function[17648]:0x110246b)
              at .tmpp9kfF5.S_P_CoreLib_System_Threading_ExecutionContext__RunInternal (wasm://wasm/.tmpp9kfF5-086e5c86:wasm-function[4562]:0x39d42b)

You can change SystemNative_SysLog as I did in image

And it will tell us more about the real issue.

dicej commented 23 minutes ago

Strange -- the test passed on Linux. I'll debug when I'm back on Monday.