Skip to content

Confusing TCP connection states #16

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

Closed
chrysn opened this issue Sep 1, 2020 · 4 comments · Fixed by #30
Closed

Confusing TCP connection states #16

chrysn opened this issue Sep 1, 2020 · 4 comments · Fixed by #30

Comments

@chrysn
Copy link
Member

chrysn commented Sep 1, 2020

The current life of a TCP socket looks superfluously complex: It can be created ("open", although it's not really clear what is opened there), but only when it is connected it becomes useful. It can be closed again, but it is not clear from the documentation whether anything can be done with a closed socket other than being dropped. (Can a closed socket be connected again? From the types it could, but that wouldn't work on POSIX sockets AFAIK).

Wouldn't it be more straightforward to have a single awaitable (OK, nb::Result) client connecting function?

When TCP server functionality is added, that'd probably be centered around different types anyway (possibly a TcpServerSocket that has an accept method).

@ryan-summers
Copy link
Member

Related to #27

@ryan-summers
Copy link
Member

The current life of a TCP socket looks superfluously complex: It can be created ("open", although it's not really clear what is opened there), but only when it is connected it becomes useful. It can be closed again, but it is not clear from the documentation whether anything can be done with a closed socket other than being dropped. (Can a closed socket be connected again? From the types it could, but that wouldn't work on POSIX sockets AFAIK).

The close() method actually consumes the socket. For network stacks with a statically-defined socket count, that essentially means that the socket can be returned for re-use by other open() requests later. Since the socket is consumed, ownership semantics prevent it from being used further.

Wouldn't it be more straightforward to have a single awaitable (OK, nb::Result) client connecting function?

I'm in the process of updating TCP and there's some complexity here. Take for example a hardware network stack like the W5500. When you call connect(), you need to internally allocate a socket and start the connection process. Immediately after this, you would return nb::Error::WouldBlock. However, when the next iteration comes along, another connect() request is encountered. Should the driver create a new connection request and allocate a new socket, or should it assume this new connect() is identical to a repeat of the previous? That would imply that the driver has to keep state of previous requests.

From a simplicity perspective, I think it makes more sense to have a"constructor" to allocate the socket resource and then explicitly connect() a specific socket. This allows drivers to then automatically differentiate duplicated connect() calls.

@Sympatron
Copy link
Contributor

Maybe the state of the socket could be encoded in it's type (possibly as a generic type parameter). For example it does not make much sense to connect() an already connected socket again (you should create a new one instead in my opinion). And conversely you can't send or receive from an unconnected socket. So connect() should return a type that supports sending and receiving, while socket() should return a type that only supports connect(). close() should probably be supported in all cases.

@ryan-summers
Copy link
Member

The difficulty is that connect() may be completed asynchronously, but the type conversion would be completed immediately. I'm having difficulty in seeing how that would be managed with a TCP socket.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants