Add support for WASILibc #2671

MaxDesiatov · 2024-03-04T14:15:19Z

Dispatch is not supported on WASI, and only Unix domain sockets are supported, which means we have to exclude those APIs on this platform.

There's work in progress to enable tests for this on CI, but nothing I can provide for this PR at the current moment.

Dispatch is not supported on WASI, and only Unix domain sockets are supported, which means we have to exclude those APIs on this platform.

Sources/NIOCore/FileHandle.swift

Sources/NIOCore/BSDSocketAPI.swift

Sources/NIOCore/Interfaces.swift

Sources/NIOCore/SocketAddresses.swift

Package.swift

Lukasa · 2024-03-08T15:40:19Z

@swift-server-bot add to allowlist

Package.swift

Lukasa · 2024-03-08T15:41:50Z

Sources/NIOConcurrencyHelpers/lock.swift

@@ -45,7 47,7 @@ public final class Lock {
 public init() {
 #if os(Windows)
 InitializeSRWLock(self.mutex)
-#else
+#elseif !os(WASI)


I don't think we can merge a PR that silently disables these primitives on WASI without being extremely confident that we'll never need them. Is there any way we can replace this with a feature-detection on threading?

Swift main snapshots support #if _runtime(_multithreaded) checks, which I'm now using here.

Lukasa · 2024-03-08T15:43:03Z

Sources/NIOCore/BSDSocketAPI.swift

@@ -199,12 204,14 @@ extension NIOBSDSocket.ProtocolFamily {
 public static let inet6: NIOBSDSocket.ProtocolFamily =
 NIOBSDSocket.ProtocolFamily(rawValue: PF_INET6)

+#if !os(WASI)


My general preference here would be that the APIs here are present, they just end up failing when actually used in WASI. That would mean hardcoding the syscall value on WASI, or deliberately using a spoiler value. That note applies to the rest of this file.

What would be the benefit of that as compared to compile-time warnings for absent symbols? My worry is that someone ends up adopting these APIs when targeting WASI and then discovering that they fail at the most inconvenient moment.

If you insist on runtime errors, would you prefer a fatalError or a certain sentinel value? In the latter case, how do I pick such value or do we just change NIOBSDSocket.Option.rawValue to have CInt? type instead of CInt?

One other argument for compile-time checks is consistency: Windows did it that way, so why WASI shouldn't follow that precedent?

Sources/NIOCore/EventLoop.swift

Lukasa · 2024-03-08T15:52:06Z

Sources/NIOCore/FileHandle.swift

@@ -54,6 57,7 @@ public final class NIOFileHandle: FileDescriptor {
 assert(!self.isOpen, "leaked open NIOFileHandle(descriptor: \(self.descriptor)). Call `close()` to close or `takeDescriptorOwnership()` to take ownership and close by some other means.")
 }

+#if !os(WASI)


Same note here: generally it's preferable to have APIs not simply be absent, but to misbehave when not present. In this case, duplicate will presumably become available, and it'd be nicer if this just threw instead. The only time that isn't my preference is when an API literally cannot be spelled, e.g. the return or argument types are missing.

Relatedly, it's best if we put the #if os at the lowest possible level. In this case, we should arrange for dup to throw or error out, and let the rest of the code path handle that appropriately, which minimises the OS customisation we have to do.

Ditto, what's the benefit of preferring obscured runtime errors when we can easily make these compile-time errors and make unavailability obvious to a library user?

kateinoigakukun · 2024-03-08T16:04:26Z

Sources/NIOCore/SocketAddresses.swift

@@ -445,6 456,7 @@ public enum SocketAddress: CustomStringConvertible, Sendable {
 self = .v6(.init(address: ipv6Addr, host: ""))
 }

+#if !os(WASI)


Could you make makeAddressResolvingHost just throw exception instead of guarding out the API itself?

I'm unsure why we'd prefer runtime checks as opposed to compile-time checks, but this particular change seems easy, so I pushed it for now.

kateinoigakukun · 2024-03-08T16:11:41Z

Sources/NIOCore/SocketOptionProvider.swift

@@ -140,6 142,7 @@ public protocol SocketOptionProvider: _NIOPreconcurrencySendable {
 //
 // You are welcome to add more helper methods here, but each helper method you add must be tested.
 extension SocketOptionProvider {
+#if !os(WASI)


How about adding a typealias like the following instead of guarding out the whole method definition?

#if canImport(WASILibc) typealias NIOLinger = Never #else typealias NIOLinger = linger #endif

Same comment as before, unsure why we'd prefer runtime failures, but this one seemed trivial so I'd considered it addressed

weissi · 2024-04-09T22:26:52Z

@MaxDesiatov / @Lukasa This looks like a PR that has bit rotting potential because it needs to touch a lot of pieces, most of the issues seem pretty tractable to me so I'd love to see this go in.

w.r.t. disabling locks on WASI: Honestly I think this can go in as is: Today this is fine and if threading really happens this will blow up very obviously and can be fixed.

# Conflicts: # Package.swift # Sources/NIOConcurrencyHelpers/NIOLock.swift # Sources/NIOConcurrencyHelpers/atomics.swift # Sources/NIOConcurrencyHelpers/lock.swift # Sources/NIOCore/BSDSocketAPI.swift # Sources/NIOCore/ByteBuffer-aux.swift # Sources/NIOCore/ByteBuffer-core.swift # Sources/NIOCore/EventLoop.swift # Sources/NIOCore/EventLoopFuture.swift # Sources/NIOCore/FileHandle.swift # Sources/NIOCore/FileRegion.swift # Sources/NIOCore/GlobalSingletons.swift # Sources/NIOCore/IO.swift # Sources/NIOCore/Interfaces.swift # Sources/NIOCore/MulticastChannel.swift # Sources/NIOCore/SystemCallHelpers.swift # Sources/NIOCore/Utilities.swift # Sources/NIOEmbedded/AsyncTestingChannel.swift # Sources/NIOEmbedded/Embedded.swift # Sources/NIOPosix/GetaddrinfoResolver.swift # Sources/NIOPosix/MultiThreadedEventLoopGroup.swift

MaxDesiatov · 2024-08-01T14:56:01Z

Currently blocked on swiftlang/swift#75618, which would make PF_INET and PF_INET6 available.

weissi · 2024-08-01T17:13:02Z

Currently blocked on swiftlang/swift#75618, which would make PF_INET and PF_INET6 available.

You can just hardcode the values for the time being, right? They are #defines and therefore ABI, so they can't change.

MaxDesiatov · 2024-08-01T17:15:22Z

Currently blocked on swiftlang/swift#75618, which would make PF_INET and PF_INET6 available.

You can just hardcode the values for the time being, right? They are #defines and therefore ABI, so they can't change.

If that version bump is easy and we get it done quickly, I'd rather have that one merged and make this PR cleaner at the same time.

# Conflicts: # Package.swift

MaxDesiatov · 2024-09-04T16:20:08Z

With changes I've pushed the build is now successful with latest development snapshots when building NIOCore and NIOHTTP1 targets for single-threaded WASI.

MaxDesiatov · 2024-09-04T16:37:45Z

Pushed more changes so that it uses wasi_pthread when multi-threading is available with wasm32-unknown-wasip1-threads triples.

MaxDesiatov · 2024-09-04T16:38:48Z

w.r.t. disabling locks on WASI: Honestly I think this can go in as is: Today this is fine and if threading really happens this will blow up very obviously and can be fixed.

@weissi In this PR we now conditionally enable locks when those are available in WASI.

weissi · 2024-09-09T14:16:03Z

CC @FranzBusch & @Lukasa

FranzBusch

This LGTM. @Lukasa do you want to review as well?

weissi

LGTM, thanks @MaxDesiatov !

Dispatch is not supported on WASI, and only Unix domain sockets are supported, which means we have to exclude those APIs on this platform. There's work in progress to enable tests for this on CI, but nothing I can provide for this PR at the current moment. --------- Co-authored-by: Franz Busch <[email protected]>

MaxDesiatov added 3 commits March 4, 2024 14:13

Add support for WASILibc

70ff245

Dispatch is not supported on WASI, and only Unix domain sockets are supported, which means we have to exclude those APIs on this platform.

Clean up formatting

66e3f0e

Clean up formatting

9170712

kateinoigakukun reviewed Mar 4, 2024

View reviewed changes

Sources/NIOCore/FileHandle.swift Outdated Show resolved Hide resolved

Sources/NIOCore/BSDSocketAPI.swift Outdated Show resolved Hide resolved

Sources/NIOCore/Interfaces.swift Outdated Show resolved Hide resolved

Sources/NIOCore/SocketAddresses.swift Outdated Show resolved Hide resolved

MaxDesiatov added 3 commits March 4, 2024 14:52

Fix NIOEmbedded for WASI

1958d91

NIOHTTP1 should depend on NIOPosix only on Posix-ish platforms

4cfee57

Clean up naming in Package.swift

e5ccda5

MaxDesiatov marked this pull request as draft March 4, 2024 16:18

MaxDesiatov added 2 commits March 4, 2024 16:19

Bump to later commit of swift-atomics

9fe470a

Address PR feedback

85f140e

MaxDesiatov marked this pull request as ready for review March 8, 2024 15:33

MaxDesiatov requested a review from kateinoigakukun March 8, 2024 15:33

MaxDesiatov commented Mar 8, 2024

View reviewed changes

Package.swift Outdated Show resolved Hide resolved

MaxDesiatov commented Mar 8, 2024

View reviewed changes

Package.swift Outdated Show resolved Hide resolved

Reduce the diff

4ee91c6

MaxDesiatov added 2 commits March 8, 2024 15:46

Merge branch 'main' into maxd/wasi-support

fdd622f

spaces >> tabs

b1b5d7f

Lukasa reviewed Mar 8, 2024

View reviewed changes

kateinoigakukun reviewed Mar 8, 2024

View reviewed changes

MaxDesiatov added 7 commits July 30, 2024 15:41

Apply formatting fixes

20967d7

Fix license header check failure

b2b25db

Merge branch 'main' into maxd/wasi-support

6143a6f

Address PR feedback

92735a5

Address PR feedback

5d64686

Introduce NIOLinger and SocketOptionProviderError

7fd9880

MaxDesiatov requested review from Lukasa and kateinoigakukun August 1, 2024 12:12

MaxDesiatov added 3 commits August 1, 2024 13:23

Merge branch 'main' into maxd/wasi-support

9149bed

Add empty CNIOWASI.c to fix Xcode build issues

98af324

Add licence header to CNIOWASI.c

47cbd67

MaxDesiatov marked this pull request as draft August 1, 2024 14:56

MaxDesiatov added 2 commits September 4, 2024 16:42

Merge branch 'main' of github.com:apple/swift-nio into maxd/wasi-support

a4bbab5

# Conflicts: # Package.swift

Fix NIOCore build with latest snapshots

aadfe72

MaxDesiatov marked this pull request as ready for review September 4, 2024 16:18

Add support for wasi_pthread

ede0d5b

Fix formatting

0b85ea9

FranzBusch approved these changes Sep 11, 2024

View reviewed changes

FranzBusch added the semver/patch No public API change. label Sep 11, 2024

Merge branch 'main' into maxd/wasi-support

9ddcfca

weissi approved these changes Sep 12, 2024

View reviewed changes

Lukasa merged commit 730713e into apple:main Sep 12, 2024
28 of 29 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add support for WASILibc #2671

Add support for WASILibc #2671

MaxDesiatov commented Mar 4, 2024 •

edited

Loading

Lukasa commented Mar 8, 2024

Lukasa Mar 8, 2024

MaxDesiatov Aug 1, 2024

Lukasa Mar 8, 2024

MaxDesiatov Aug 1, 2024

MaxDesiatov Aug 1, 2024

Lukasa Mar 8, 2024

MaxDesiatov Aug 1, 2024

kateinoigakukun Mar 8, 2024

MaxDesiatov Aug 1, 2024

kateinoigakukun Mar 8, 2024

MaxDesiatov Aug 1, 2024

weissi commented Apr 9, 2024

MaxDesiatov commented Aug 1, 2024

weissi commented Aug 1, 2024 •

edited

Loading

MaxDesiatov commented Aug 1, 2024

MaxDesiatov commented Sep 4, 2024 •

edited

Loading

MaxDesiatov commented Sep 4, 2024

MaxDesiatov commented Sep 4, 2024 •

edited

Loading

weissi commented Sep 9, 2024

FranzBusch left a comment

weissi left a comment

Add support for WASILibc #2671

Add support for WASILibc #2671

Conversation

MaxDesiatov commented Mar 4, 2024 • edited Loading

Lukasa commented Mar 8, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

weissi commented Apr 9, 2024

MaxDesiatov commented Aug 1, 2024

weissi commented Aug 1, 2024 • edited Loading

MaxDesiatov commented Aug 1, 2024

MaxDesiatov commented Sep 4, 2024 • edited Loading

MaxDesiatov commented Sep 4, 2024

MaxDesiatov commented Sep 4, 2024 • edited Loading

weissi commented Sep 9, 2024

FranzBusch left a comment

Choose a reason for hiding this comment

weissi left a comment

Choose a reason for hiding this comment

MaxDesiatov commented Mar 4, 2024 •

edited

Loading

weissi commented Aug 1, 2024 •

edited

Loading

MaxDesiatov commented Sep 4, 2024 •

edited

Loading

MaxDesiatov commented Sep 4, 2024 •

edited

Loading