Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Closed
wants to merge 20 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
20 commits
Select commit Hold shift + click to select a range
beabe5a
support `System.Net.Http.HttpClient` on WASIp2
dicej Jun 12, 2024
28d758f
make `HttpClient.Timeout` actually work on WASI
dicej Aug 26, 2024
e32a223
fix file count confusion in SharedLibraryDriver.mjs
dicej Aug 26, 2024
e79642e
fix Linux tests
dicej Aug 27, 2024
2f3ce60
use `TaskCompletionSource.SetCanceled` instead of `SetException`
dicej Aug 27, 2024
18f9f6c
use `Stopwatch` in `HttpClient` test to verify timeout works as expected
dicej Aug 28, 2024
ebdbabf
provide more informative exception messages in `WasiHttpHandler`
dicej Aug 28, 2024
9d9ed59
remove obsolete TODO-LLVM comments
dicej Aug 28, 2024
4879365
update wit-bindgen and regenerate bindings
dicej Aug 29, 2024
137614e
to match https://github.com/dotnet/runtime/pull/107096
pavelsavara Aug 28, 2024
b25bc9e
fix disposal of pollable
pavelsavara Aug 29, 2024
5f35285
update wit-bindgen and regenerate bindings (again)
dicej Aug 29, 2024
ddbd68a
specify zero expected exit code for WASIp2 smoke test
dicej Aug 29, 2024
e989a63
convert response headers after setting content
dicej Aug 29, 2024
98bf4dc
exit `DispatchWasiEventLoop` without polling if tasks have been canceled
dicej Aug 29, 2024
757eeb9
assert HTTP test in `SharedLibrary` completes promptly
dicej Aug 29, 2024
dfdd8bc
support reading trailers in `WasiInputStream`
dicej Sep 3, 2024
1736bdd
increase max elapsed time in SharedLibrary test
dicej Sep 3, 2024
dd5f8ae
work around possible `wasmtime-wasi-http` bug regarding trailing headers
dicej Sep 5, 2024
68ec226
remove temporary http-p2 smoke test
dicej Sep 13, 2024
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions eng/pipelines/common/global-build-job.yml
Original file line number Diff line number Diff line change
Expand Up @@ -232,6 +232,8 @@ jobs:
displayName: Install wasi-sdk
- script: pwsh $(Build.SourcesDirectory)/eng/pipelines/runtimelab/install-wasmtime.ps1 -CI -InstallDir $(Build.SourcesDirectory)/wasm-tools
displayName: Install wasmtime
- script: pwsh $(Build.SourcesDirectory)/eng/pipelines/runtimelab/install-jco.ps1 $(Build.SourcesDirectory)
displayName: Install Jco

- ${{ if or(eq(parameters.platform, 'browser_wasm_win'), and(eq(parameters.platform, 'wasi_wasm_win'), not(eq(parameters.runtimeFlavor, 'coreclr')))) }}:
# Update machine certs
Expand Down
9 changes: 9 additions & 0 deletions eng/pipelines/runtimelab/install-jco.ps1
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
$RootPath = $Args[0]

$NpmExePath = $Env:NPM_EXECUTABLE

Set-Location -Path $RootPath

& $NpmExePath install @bytecodealliance/jco
& $NpmExePath install @bytecodealliance/preview2-shim

11 changes: 11 additions & 0 deletions eng/pipelines/runtimelab/install-nodejs.ps1
Original file line number Diff line number Diff line change
Expand Up @@ -51,11 +51,13 @@ if ($IsWindows)
{
Expand-Archive -LiteralPath "$InstallPath\$NodeJSZipName" -DestinationPath $InstallPath -Force
$NodeJSExePath = "$InstallPath\$NodeJSInstallName\node.exe"
$NpmExePath = "$InstallPath\$NodeJSInstallName\npm.cmd"
}
else
{
tar xJf $InstallPath/$NodeJSZipName -C $InstallPath
$NodeJSExePath = "$InstallPath/$NodeJSInstallName/bin/node"
$NpmExePath = "$InstallPath/$NodeJSInstallName/bin/npm"
}

if (!(Test-Path $NodeJSExePath))
Expand All @@ -64,5 +66,14 @@ if (!(Test-Path $NodeJSExePath))
exit 1
}

if (!(Test-Path $NpmExePath))
{
Write-Error "Did not find NPM at: '$NpmExePath'"
exit 1
}

Write-Host Setting NODEJS_EXECUTABLE to $NodeJSExePath
Write-Host "##vso[task.setvariable variable=NODEJS_EXECUTABLE]$NodeJSExePath"

Write-Host Setting NPM_EXECUTABLE to $NpmExePath
Write-Host "##vso[task.setvariable variable=NPM_EXECUTABLE]$NpmExePath"
7 changes: 4 additions & 3 deletions eng/testing/FindWasmHostExecutable.sh
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@ exename=$(basename "$exename" .js)
dirname=$(dirname "$1")

node="node --stack_trace_limit=100"
WASM_HOST_ARGS_SEPERATOR=""

if [ -e "${dirname}/${exename}.js" ]; then
WASM_HOST_EXECUTABLE=$node
Expand All @@ -21,7 +20,9 @@ elif [ -e "${dirname}/main.mjs" ]; then
WASM_HOST_EXECUTABLE=$node
WASM_BINARY_TO_EXECUTE="${dirname}/main.mjs"
elif [ -e "${dirname}/${exename}.wasm" ]; then
WASM_HOST_EXECUTABLE=$WASMTIME_EXECUTABLE
WASM_HOST_ARGS_SEPERATOR="--"
if [ -z "$WASMTIME_EXECUTABLE" ]; then
WASMTIME_EXECUTABLE=wasmtime
fi
WASM_HOST_EXECUTABLE="$WASMTIME_EXECUTABLE run -S http"
WASM_BINARY_TO_EXECUTE="${dirname}/${exename}.wasm"
fi
4 changes: 2 additions & 2 deletions eng/testing/FindWasmHostExecutableAndRun.sh
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,8 @@ source $SCRIPT_DIR/FindWasmHostExecutable.sh "$1"

if [ -n "${WASM_HOST_EXECUTABLE}" ]; then
shift
echo $WASM_HOST_EXECUTABLE "$WASM_BINARY_TO_EXECUTE" $WASM_HOST_ARGS_SEPERATOR "$@"
$WASM_HOST_EXECUTABLE "$WASM_BINARY_TO_EXECUTE" $WASM_HOST_ARGS_SEPERATOR "$@"
echo $WASM_HOST_EXECUTABLE "$WASM_BINARY_TO_EXECUTE" "$@"
$WASM_HOST_EXECUTABLE "$WASM_BINARY_TO_EXECUTE" "$@"
else
echo WASM_HOST_EXECUTABLE not set.
exit 1
Expand Down
8 changes: 5 additions & 3 deletions src/coreclr/build-runtime.cmd
Original file line number Diff line number Diff line change
Expand Up @@ -349,9 +349,11 @@ for /f "delims=" %%a in ("-%__RequestedBuildComponents%-") do (
set __CMakeTarget=!__CMakeTarget! nativeaot

if "%__TargetArch%"=="wasm" (
if not defined EMSDK (
echo %__ErrMsgPrefix%%__MsgPrefix%Error: The EMSDK environment variable pointing to emsdk root must be set.
goto ExitWithError
if "%__TargetOS%"=="browser" (
if not defined EMSDK (
echo %__ErrMsgPrefix%%__MsgPrefix%Error: The EMSDK environment variable pointing to emsdk root must be set.
goto ExitWithError
)
)
)
)
Expand Down
2 changes: 1 addition & 1 deletion src/coreclr/jit/compiler.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -9686,7 +9686,7 @@ void dumpConvertedVarSet(Compiler* comp, VARSET_VALARG_TP vars)
{
printf(" ");
}
printf("V%02u", varNum);
printf("V%02u", (unsigned int) varNum);
first = false;
}
}
Expand Down
1 change: 1 addition & 0 deletions src/coreclr/jit/llvmlssa.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1639,6 +1639,7 @@ class ShadowStackAllocator
#ifdef __clang__
#pragma clang diagnostic push
#pragma clang diagnostic ignored "-Wformat-security"
#pragma clang diagnostic ignored "-Wdeprecated-declarations"
#endif // __clang__
if (pBuffer == nullptr)
{
Expand Down
1 change: 0 additions & 1 deletion src/coreclr/nativeaot/Runtime/Portable/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -40,4 +40,3 @@ install_static_library(standalonegc-disabled aotsdk nativeaot)
if (NOT CLR_CMAKE_TARGET_ARCH_WASM)
install_static_library(standalonegc-enabled aotsdk nativeaot)
endif()

Original file line number Diff line number Diff line change
Expand Up @@ -163,8 +163,8 @@ internal PhysicalFilesWatcher CreateFileWatcher()

FileSystemWatcher? watcher;
#if NET
// For browser/iOS/tvOS we will proactively fallback to polling since FileSystemWatcher is not supported.
if (OperatingSystem.IsBrowser() || (OperatingSystem.IsIOS() && !OperatingSystem.IsMacCatalyst()) || OperatingSystem.IsTvOS())
// For WASI/browser/iOS/tvOS we will proactively fallback to polling since FileSystemWatcher is not supported.
if (OperatingSystem.IsWasi() || OperatingSystem.IsBrowser() || (OperatingSystem.IsIOS() && !OperatingSystem.IsMacCatalyst()) || OperatingSystem.IsTvOS())
Comment on lines +166 to +167

Choose a reason for hiding this comment

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

Most library changes here should be upstreamed (we will merge them here before that, but it should only be temporary).

Copy link
Member

Choose a reason for hiding this comment

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

I have many such changes in messy PR here dotnet/runtime#105838
I don't expect to merge it in current form.
The problem is that attributes are visible on the runtime API, so we better do it right on the first attempt.
But I don't know exactly what would be PNSE and what would eventually work. And that will change with next WASI preview.

Copy link
Author

Choose a reason for hiding this comment

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

I'll admit I've been using this PR as a dumping ground for miscellaneous WASI fixes as I build demo apps (this one came up when testing AspNetCore). Happy to open separate, upstream PRs as appropriate, but I figured I'd collect them all here first, at least temporarily.

@pavelsavara: yeah, I've been avoiding attributes for the time being since I also don't know which features might be supported in the future. I'm pretty sure WASI will never support signal handling, but I could imagine it might support file notification someday.

{
UsePollingFileWatcher = true;
UseActivePolling = true;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,10 +15,13 @@ public partial class ConsoleLifetime : IHostLifetime

private partial void RegisterShutdownHandlers()
{
Action<PosixSignalContext> handler = HandlePosixSignal;
_sigIntRegistration = PosixSignalRegistration.Create(PosixSignal.SIGINT, handler);
_sigQuitRegistration = PosixSignalRegistration.Create(PosixSignal.SIGQUIT, handler);
_sigTermRegistration = PosixSignalRegistration.Create(PosixSignal.SIGTERM, handler);
if (!OperatingSystem.IsWasi())
{
Action<PosixSignalContext> handler = HandlePosixSignal;
_sigIntRegistration = PosixSignalRegistration.Create(PosixSignal.SIGINT, handler);
_sigQuitRegistration = PosixSignalRegistration.Create(PosixSignal.SIGQUIT, handler);
_sigTermRegistration = PosixSignalRegistration.Create(PosixSignal.SIGTERM, handler);
}
}

private void HandlePosixSignal(PosixSignalContext context)
Expand All @@ -31,9 +34,12 @@ private void HandlePosixSignal(PosixSignalContext context)

private partial void UnregisterShutdownHandlers()
{
_sigIntRegistration?.Dispose();
_sigQuitRegistration?.Dispose();
_sigTermRegistration?.Dispose();
if (!OperatingSystem.IsWasi())
{
_sigIntRegistration?.Dispose();
_sigQuitRegistration?.Dispose();
_sigTermRegistration?.Dispose();
}
}
}
}
9 changes: 5 additions & 4 deletions src/libraries/System.Net.Http/src/System.Net.Http.csproj
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,6 @@
<EmitCompilerGeneratedFiles Condition="'$(Configuration)' == 'Debug' and '$(TargetPlatformIdentifier)' == 'browser'">true</EmitCompilerGeneratedFiles>
</PropertyGroup>

<!-- TODO-LLVM: This is not upstreamable because it makes the build runtime-specific. -->
<PropertyGroup>
<DefineConstants Condition="'$(RuntimeFlavor)' == 'CoreCLR'">$(DefineConstants);NATIVE_AOT</DefineConstants>
</PropertyGroup>
Expand Down Expand Up @@ -461,10 +460,12 @@
Link="Common\System\Net\Http\HttpHandlerDefaults.cs" />
</ItemGroup>

<!-- TODO-LLVM: This is not upstreamable because it makes the build runtime-specific. -->
<ItemGroup Condition="'$(TargetPlatformIdentifier)' == 'wasi' and '$(RuntimeFlavor)' != 'CoreCLR'">
<ItemGroup Condition="'$(TargetPlatformIdentifier)' == 'wasi'">
<Reference Include="System.Threading.Thread" />
<Compile Include="System\Net\Http\WasiHttpHandler\WasiHttpHandler.cs" />
<Compile Include="System\Net\Http\WasiHttpHandler\WasiHttpInterop.cs" />
<Compile Include="System\Net\Http\WasiHttpHandler\WasiOutputStream.cs" />
<Compile Include="System\Net\Http\WasiHttpHandler\WasiInputStream.cs" />
<Compile Include="System\Net\Http\WasiHttpHandler\WasiHttp.cs" />
<Compile Include="System\Net\Http\WasiHttpHandler\WasiHttpWorld.wit.imports.wasi.clocks.v0_2_1.MonotonicClockInterop.cs" />
<Compile Include="System\Net\Http\WasiHttpHandler\WasiHttpWorld.wit.imports.wasi.http.v0_2_1.ITypes.cs" />
Expand All @@ -481,7 +482,7 @@
<ItemGroup>
<Reference Include="Microsoft.Win32.Primitives" />
<Reference Include="System.Collections.NonGeneric" />
<Reference Include="System.Console" Condition="'$(Configuration)' == 'Debug'" />
<Reference Include="System.Console" />
<Reference Include="System.Diagnostics.DiagnosticSource" />
<Reference Include="System.IO.Compression" />
<Reference Include="System.Net.NameResolution" />
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@
using System.Threading.Tasks;
using System.Diagnostics.Metrics;
// TODO-LLVM: This is not upstreamable and should be deleted when https://github.com/dotnet/runtimelab/pull/2614 is merged
#if TARGET_WASI && !NATIVE_AOT
#if TARGET_WASI
Comment on lines 13 to +14

Choose a reason for hiding this comment

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

I presume the TODOs here should be removed?

Copy link
Member

Choose a reason for hiding this comment

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

I'm rewriting the whole file upstream to respect child resources and more.

Copy link
Author

Choose a reason for hiding this comment

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

I wasn't sure if the TODO-LLVM comments were still relevant (i.e. did they refer to just the && !NATIVE_AOT part, or the whole #if conditional?) Sounds like they're no longer relevant, so I'll remove them.

Choose a reason for hiding this comment

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

There were introduced very recently in https://github.com/dotnet/runtimelab/pull/2605/files. I assumed it was a fix for some build break.

using System.Diagnostics;
using System.Net.Http.Metrics;
using HttpHandlerType = System.Net.Http.WasiHttpHandler;
Expand All @@ -29,8 +29,7 @@ public partial class HttpClientHandler : HttpMessageHandler
{
private readonly HttpHandlerType _underlyingHandler;

// TODO-LLVM: This is not upstreamable and !NATIVE_AOT should be reverted when https://github.com/dotnet/runtimelab/pull/2614 is merged
#if TARGET_BROWSER || (TARGET_WASI && !NATIVE_AOT)
#if TARGET_BROWSER || TARGET_WASI
private IMeterFactory? _meterFactory;
private HttpMessageHandler? _firstHandler; // DiagnosticsHandler or MetricsHandler, depending on global configuration.

Expand Down Expand Up @@ -100,8 +99,7 @@ protected override void Dispose(bool disposing)
[CLSCompliant(false)]
public IMeterFactory? MeterFactory
{
// TODO-LLVM: This is not upstreamable and !NATIVE_AOT should be reverted when https://github.com/dotnet/runtimelab/pull/2614 is merged
#if TARGET_BROWSER || (TARGET_WASI && !NATIVE_AOT)
#if TARGET_BROWSER || TARGET_WASI
get => _meterFactory;
set
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ private void CheckDisposedOrStarted()
/// Gets a value that indicates whether the handler is supported on the current platform.
/// </summary>
[UnsupportedOSPlatformGuard("browser")]
public static bool IsSupported => !OperatingSystem.IsBrowser();
public static bool IsSupported => !(OperatingSystem.IsBrowser() || OperatingSystem.IsWasi());

public bool UseCookies
{
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
// Generated by `wit-bindgen` 0.29.0. DO NOT EDIT!
// Generated by `wit-bindgen` 0.30.0. DO NOT EDIT!
// <auto-generated />
#nullable enable
using System;
Expand Down
Loading
Loading