-
Notifications
You must be signed in to change notification settings - Fork 107
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
[DRAFT] Initial socket support #30
base: main
Are you sure you want to change the base?
Conversation
#elseif os(Windows) | ||
@_implementationOnly import ucrt | ||
import CSystem | ||
import ucrt |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hmm, Windows is going to be more complicated :-(. It is going to drag in WinSDK (which is effectively Darwin
on macOS). The socket interfaces are not part of the C library but an entirely separate framework. The problem is that the module (as currently setup) will potentially drag in additional unnecessary link dependencies. I wonder if we want to add a local module to pull in the WinSock interfaces as @_implementationOnly
.
We will definitely need an additional shim for adding a module constructor to initialize WinSDK.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hmm, is linking against WinSDK a problem? Could the shim module link against just Ws2_32.lib
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
WinSDK is not a library but rather a hand rolled module that encompasses all of the Windows SDK. Yes, the shim module can just link against Ws2_32.lib
and that is sufficient. I'm just worried about overlinking, but, I suppose that it should be easy to check. The WinSDK module isn't great though and needs to be improved (it has been built up organically and tactically as we dont have good control over the SDK).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we dynamically link to it or dlsym
equivalent? I don't really know Windows best practices or conventions, but my recollection is that it was shockingly in favor of hot loading .dll
s.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yep, forcing unnecessary linkages on people would be a significant obstacle to adoption. (E.g., see corelibs-foundation.) There ought to be no reasonable objection against adopting swift-system.
I expect at some point we will have to take the step of splitting up this package into multiple system-dependent modules, with the top-level SystemPackage
module reexporting each component.
It should be possible to use the core APIs in this package without linking with winsocks.
What modules (if any) need to be spun off is different on every platform, and the package should fully reflect that.
18b96f6
to
c11d8fa
Compare
edit: No, without struct sub typing, trying to separate socket options by level is a lot more machinery and generates confusion. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Some superficial comments
/// Treat `fd` as a socket descriptor, without checking with the operating | ||
/// system that it actually refers to a socket | ||
@_alwaysEmitIntoClient | ||
public init(unchecked fd: FileDescriptor) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I wonder if unchecked
is the right argument label to use here. It makes me think of Range(uncheckedBounds:)
, in which case the checking is a bounds check precondition.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For Range
, is that precondition later checked? Here, the failure mode is "recoverable", i.e. Errno is thrown, but it is still an operation that doesn't check that the result is valid until use. "unsafe" is flat out as this will be checked and does not produce UB.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No -- the unchecked
in Range(uncheckedBounds:)
indicates that failing to fulfill the unchecked precondition will lead to undefined behavior.
This is not the case here at all -- the difference between FileDescriptor and SocketDescriptor is a figment of our own imagination; translating between the two can never lead to UB.
} | ||
if scopeid { | ||
flags.insert(.scopeIdentifier) | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@natecook1000 , does the argument parser allow adding members to an OptionSet directly?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No - it would be really cool to have that as a feature, where a group of flags could all be stored together in an OptionSet
type. You’d need to provide the metadata specifically, since those aren’t typically CaseIterable
the way enums often are.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If this is something you’re using multiple places, you could move the flags to a separate type, vend the option set as a property, and then include it as an @OptionGroup
.
|
||
/// TODO | ||
@frozen | ||
public struct SocketDescriptor: RawRepresentable, Hashable { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
how are we later on doing kqueue/epoll where we can add sockets and pipes (and for kqueue also file descriptors (although they don't work like you'd expect to))?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We can do that either by requiring the user to pass socket.fileDescriptor
to these constructs, or by adding direct overloads that take a SocketDescriptor, or by introducing a protocol for these things. Note that the latter option is highly unlikely, I think.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@lorentey Right. The "issue" with this is that for epoll
for example, you should only pass pipes/sockets/ttys etc. If you pass a regular file, then it won't work (and error, think even EBADF
:) ). So given that we have some form of typing, it's kinda sad to remove it to go to epoll which internally does require the right type. But maybe this is a step too far for System
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I haven't explored kqueue yet, but I suspect we'll follow a similar strategy here with a custom descriptor type. We might end up with rich type hierarchies for the different kinds of descriptors, so epoll
would only receive pipe-family descriptors, but that's TBD.
If Swift had struct subtyping, this would all be way easier to model (as would many other things), but we can instead rely on conventions and protocols.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The distinction between SocketDescriptor
and FileDescriptor
is just a namespacing convenience -- it's not real. If we add a KqueueDescriptor
, it'll also be just a namespacing construct.
The primary goal of System is to eliminate the difficulty of calling syscalls from Swift, and especially to fix the associated memory safety issues.
Building abstractions that don't directly map to the underlying interfaces aggressively works against this goal -- however desirable they might be. Such abstractions are important (dare I say, crucial) to have, but they belong in higher layers such as NIO.
Other than ensuring memory safety, System cannot afford to be strongly opinionated about how people want to use syscalls; otherwise some folks will be forced to continue calling the unsafe C originals, and memory management / buffer overflow issues will be a problem forever.
Making it more difficult to call syscalls just to prevent EBADF/ENOTSOCK from getting thrown is definitely a non-goal to me. It's not unsafe to call epoll on a regular file; we'll just get an error thrown at runtime. This is like calling seek
on a terminal device or socket (in fact, that's even "worse", as seek
doesn't even throw an error).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have also been working on Sockets support for System (mainly for Bluetooth and Netlink on Linux, not IPv4). Just for some context we are building embedded devices with Swift 5 and Yocto / Buildroot, so all our Swift packages are eventually vendored as .so
shared libraries and system frameworks.
I strongly believe that adding a duplicate FileDescriptor
wrapper for sockets (what about pipes) defeats the purpose of this API and will lead to code duplication. IMHO the System
library should be using mostly by library vendors and not most end users directly, and other abstractions like memory management (automatically closing FDs) should be done in another layer.
https://github.com/PureSwift/swift-system/blob/feature/network/Sources/System/SocketOperations.swift
https://github.com/PureSwift/BluetoothLinux
https://github.com/PureSwift/Netlink
I think we can make Codable for None of the constant enums or other structs seem Codable to me. |
|
||
/// Specify the part (or all) of a full-duplex connection to shutdown. | ||
@frozen | ||
public struct ShutdownKind: RawRepresentable, Hashable, Codable, CustomStringConvertible { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we'll want to get rid of Codable
here.
To answer your Codable question: Codable abstracts your serialization format from your type structure, and thus is suitable for all kinds of serialization, including cross-process and cross-machine. (One of its primary use cases is to parse JSON coming from arbitrary servers, for instance.) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Happened upon a couple typos and thought I'd mark them even though that part appears to be WIP.
Add Swifty wrappers for socket and IP addresses
Co-authored-by: Kyle Macomber <[email protected]>
Co-authored-by: Kyle Macomber <[email protected]>
* Implement UNIX domain socket addresses * Implement standalone SocketAddress.Family enum * Add availability comments * [WIP] Implement sendmsg/rcvmsg * SocketAddress: Use _RawBuffer. * ControlMessageBuffer: Fix issues uncovered in testing * Enums: use mutable rawValues This helps simplify code that needs to update these directly. * Update/simplify sendmsg/recvmsg implementations * Mocking: Add support for wildcard argument matching * Add some AncillaryMessageBuffer tests; fix issues. * Apply suggestions from code review Co-authored-by: Michael Ilseman <[email protected]> * Make mock failure output a little easier to follow * Implement support for getaddrinfo * Add an executable with very simple sample code * Add availability marker for new CInterop typealiases * Do not use implicitly unwrapped optionals * Rework sendmsg/recvmsg, make some samples * Updates/cleanups/discussion results Co-authored-by: Michael Ilseman <[email protected]>
* Update getsockopt/setsockopt * Add getsockopt/setsockopt docs * Doc updates
* Update getsockopt/setsockopt * Add getsockopt/setsockopt docs * Doc updates * Add overloads for bind/connect taking concrete address types * ShutdownKind: Remove Codable conformance. * Listen: close the client connection socket before exiting This is supposed to demonstrate acceptable use, we can’t leave resource cleanup to exit() * Make IPv4.Address and IPv6.Address expressible by string literals * Document SocketAddress better.
42b8e22
to
2b0b28c
Compare
Rebased, fixed conflicts, and then extracted everything into a separate |
This would be great! It's really awkward to use the system's socket API via the clang importer - for instance, it seems like every system uses a different name for the union property of an FYI, I'd be happy to contribute those to swift-system. IMO, it's nicer if you can avoid platform dependencies, because you can guarantee more consistent behaviour, and there is not really anything about IP addresses which should be specific to any system. The implementation is fairly well isolated to a single file, outside of a couple of ASCII constants and some pointer utilities for using tuples as fixed-size arrays. |
I’m not speaking for @milseman here, but while I think good IP address types are very valuable, they probably don’t belong in Swift System. In many ways, Swift System is not trying to paper over the differences between OSes, but is instead exposing them. However, I definitely think some suitable currency types here would be valuable. They may belong in the standard library more than they belong here. Relatedly, currency types for the wider “sockaddr” notion would also be useful, as those types are drastically more complex than an IP address alone. |
Well, the API roadmap says:
So that's why I thought they might fit here; papering over OS differences is just a bonus. If I understand you correctly @Lukasa, you're saying that there may be room for a library below swift-system (maybe the stdlib), containing currency types defined by e.g. networking standards? That's an interesting idea. |
It does, but the README also says:
Naturally these two ideas are in tension, but I think it would be good to holistically consider whether swift-system is the right place to define an IPAddress type, or whether it's simply somewhere that should use it. |
I do want to resuscitate this and figure out how to move forwards. @karwa thanks for the offer for IP address parsing code. Is this a desire to avoid making a syscall, or are there concerns about platform availability? Are you able to support more API with a native implementation? We have construction of address from string via System is surfacing platform differences, but in a way that should make it easier to write platform-conscious code (including working in portable subsets). Sort of like the |
I am currently working on the same thing, I am making the Socket address, options and family type-safe wrappers over the C API using generics. https://github.com/PureSwift/swift-system/tree/feature/network |
I wanted to propose some API design changes based on my work to open a discussion on the merits of each:
I think the main take away is taking more advantage of generics for compiler optimizations and extending usage outside of this package, and preferring concrete types and enums with associated values over |
I think it's a mistake to say that IP addresses have a "raw" value of
enums here are very dangerous because they cannot be evolved: adding new enum cases is an API breaking change. We'll consistently need to add new types any time someone wants to add their pet socket option to the API, which is a bit of a mess. Given that socket options are unbounded anyway, we may just want a large family of structures instead.
Presumably we want to be able to construct these values from their C representations? I don't see any particular harm in being able to use initialisers to transform back and forth. |
Thats fair, I guess its a personal preference but I do see the logic of what you are saying. My logic was that since its the preferred way to construct the type (with the C type also able to instantiate it, but via more verbose
I am aware of the ABI aspect and I think we should freeze the enums we ship with this library. I think we need to weigh the pros and cons of having a couple dozen structs littering the type system for each option (and the performance impact of switching enums with associated values vs casting protocol containers), vs making a concrete enum of each family of socket options and the (IHMO small) danger of ABI breakage. If we are using the C / POSIX socket options (e.g.
Yes, but we want to make it as verbose as all the other |
Concretely, my objection here is that
That's good, because we don't have a choice: Swift packages literally cannot have open enums in them.
Additionally, socket options in your design aren't constants: they have associated values, because they encode the type of the value. This is a good part of your design, and I want to see it persist: dealing with non We don't have to "litter the namespace" with them: you can define them in an uninhabited
Why? It's not unsafe. |
@Lukasa I think you misunderstood my pitch for
|
@Lukasa I will try your proposal for struct for socket options, thanks for your feedback. Feel free to keep an eye on my branch, its too WIP to open a PR yet, and honestly there is a lot of code I wish I just copied from this branch to save hours of my time. |
I don't think I did. Here's the code directly from your module (as of eba8454e): public protocol SocketOption {
associatedtype ID: SocketOptionID
var id: ID { get }
func withUnsafeBytes<T>(_: ((UnsafeRawBufferPointer) -> (T))) -> T
}
public enum GenericSocketOption: SocketOption, Equatable, Hashable {
public typealias ID = GenericSocketOptionID
case debug(Bool)
case keepAlive(Bool)
@_alwaysEmitIntoClient
public var id: ID {
switch self {
case .debug: return .debug
case .keepAlive: return .keepAlive
}
}
@_alwaysEmitIntoClient
public func withUnsafeBytes<T>(_ pointer: ((UnsafeRawBufferPointer) -> (T))) -> T {
switch self {
case let .debug(value):
return Swift.withUnsafeBytes(of: value.cInt) { bufferPointer in
pointer(bufferPointer)
}
case let .keepAlive(value):
return Swift.withUnsafeBytes(of: value.cInt) { bufferPointer in
pointer(bufferPointer)
}
}
}
} My point here is that As to POSIXErrorCode, enums in the standard library can add new cases over time, so my objection does not apply to them. POSIXErrorCode, notably, is not a frozen enum. |
@Lukasa So, I agree that using structs for |
For Do you have a link to SocketProtocol? |
SystemPackage: I know it's a small optimization, but the enum will use one byte on the stack vs Int32, and more than that I really like enums for constants (due to switching), especially if they are tied to C std lib or kernel drivers I know will not change for the foreseeable future. It just makes it a tiny bit safer but not allowing invalid constants. |
Just to provide some context, we have been using L2CAP and HCI Bluetooth sockets on Linux for Bluetooth LE (Client and Server) to interact with the Linux BlueZ subsystem via Swift without the C userland library (due to licensing and bugs) and Netlink (with Codable serializer) as a replacement for CoreWLAN on Linux. Outside of Other parts of C stdlib / POSIX API this framework should implement:
Also Foundation (really CoreFoundation) breaks every couple releases on Linux and 32 bit platforms in general, so the more #PureSwift libraries we can have, the more we can use Swift (instead of Rust) for embedded ARM development. |
So the offer was based on:
As for why we should have native IP address types in general:
That being said, I think @Lukasa's idea about this being in a library below swift-system is very attractive. They don't require any system functionality, so a new platform that doesn't support swift-system (e.g. webassembly) shouldn't be excluded from them. At the same time, they don't expose any system functionality, so if there was a theoretical port of swift-system to webassembly, it would have to include more than just IP addresses and any other such types. |
You don't have to construct an IP address from a String. With the WebURL IPv4Address type you can also write: let addr = IPv4Address(octets: (127, 0, 0, 1)) This has some uses for addresses you know at compile-time (loopback addresses, perhaps special services). Since it doesn't require parsing, it isn't a failable initialiser. IPv6 addresses offer the same thing, but they're easier to get wrong because they're bigger. |
Draft of
SocketDescriptor
and support for:Prototyped
socket
, with domains (protocol/address families) and connection types (stream vs datagram)send
/recv
, with message flagslisten
shutdown
getsockopt
andsetsockopt
FileDescriptor
's helpersgetaddrinfo
-like functionalitysockaddr_t
and friends, and syscalls takingsockaddr_t
.TODO
Questions
Codable
?TCP
,IP
,IPv6
namespace for socket options?level
andoption_name
are in effect rolled into one parameterv4
in their names?Deferred
gethostent
,getnetent
,getprotoent
,getservent
, etc., unless needed forsockaddr_t
FileDescriptor
really is a socket (stat
)Bool
or simple value-oriented API for options