-
Notifications
You must be signed in to change notification settings - Fork 2
Implementing unix socket server and client #145
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
base: main
Are you sure you want to change the base?
Conversation
fd5f861 to
2ee1cb0
Compare
marciniwanicki
left a comment
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.
Big thanks @beefon! Very excited for curie to get the ability to be controlled remotely. Raw sockets are a bit tricky, I wonder if it would make sense to try to use https://github.com/apple/swift-nio.
|
|
||
| var addr = sockaddr_un() | ||
| addr.sun_family = sa_family_t(AF_UNIX) | ||
| strcpy(&addr.sun_path.0, socketPath) |
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.
This is potentially insecure due to sun_path fixed size (https://github.com/apple-oss-distributions/xnu/blob/f6217f891ac0bb64f3d375211650a4c1ff8ca1ea/bsd/sys/un.h#L79C27-L79C30).
| strongSelf.handleClient(clientFileDescriptor: clientFileDescriptor, handler: handler) | ||
| } | ||
| } | ||
| listeningHandle.close() |
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.
Is there chance here that the Darwin.close will be called twice for the same file descriptor?
| public final class ServerListeningHandle { | ||
| public let socketPath: String | ||
| public let fileDescriptor: Int32 | ||
| public private(set) var isListening: Bool |
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.
Is there a possibility of data race, ie the property being accessed from multiple threads?
| throw NSError(domain: "SocketBindError", code: 2) | ||
| } | ||
|
|
||
| listen(serverFileDescriptor, 10) |
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 could potentially check the return code to handle errors.
| ) throws -> Response { | ||
| let clientFileDescriptor = socket(AF_UNIX, SOCK_STREAM, 0) | ||
| guard clientFileDescriptor >= 0 else { | ||
| throw NSError(domain: "SocketError", code: 1) |
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.
Since we're in Swift land, we can probably just throw CoreError.
| clientMessage: "from client", | ||
| expectedServerMessage: "from server" | ||
| ) | ||
| wait(for: [serverHandlerCalled]) |
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.
Might be good to set a timeout.
2ee1cb0 to
1848945
Compare
1848945 to
b3fe9fb
Compare
|
Thanks @marciniwanicki , I think i have addressed comments. Please have a look. Re: swift-nio. I have attempted to do the socket using that library few months ago when i was doing my change, and I wasn't happy with result. It was super overengineered for such simple use case as i want to have in curie. Also, pulling in library that has more code than this project sounds weird. If this is blocker to you though, I can try to re-do the sockets based upon swift-nio. |
marciniwanicki
left a comment
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.
Thanks @beefon for the follow ups. I've seen few more small edge cases which might be problematic, but we can address them later. Great work.
Only implementation for the server socket which listens to the unix socket, and client which lets you talk to the server over socket. Plus some basic tests which describe high level usage.