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

Thoughts on SocketAddr switchboard #3

Open
dusty-phillips opened this issue Jun 4, 2020 · 6 comments
Open

Thoughts on SocketAddr switchboard #3

dusty-phillips opened this issue Jun 4, 2020 · 6 comments

Comments

@dusty-phillips
Copy link
Contributor

For my use as a test socket, I'm looking for memory-socket to mimic a normal socket as much as possible.

In order to fully mimic them, I'd like the keys in the SWITCHBOARD to be SocketAddr instead of u16s. This would allow me to call it seamlessly with other socket traits. I know that'd be a huge breaking change for memory-socket, and I'm happy to maintain my own fork. But I wanted to run it by you in case a) you'd accept the contribution and b) you had a good reason not to implement it that way so I can avoid making such a mistake. ;-)

@bmwill
Copy link
Owner

bmwill commented Jun 4, 2020

I'd be happy to accept any contributions :)

That would probably be a good change since it would make the type more interchangeable. I wish there was a set of standard socket traits for connecting/listening so that they would be completely interchangeable but such is the current state of things.

The primary reason I implemented the address using a u16 instead of a full SocketAddr was mostly because I wanted to punt the question of what to do with the ip address part since anything but localhost doesn't quite make sense (and even then localhost can be misleading). One way to possibly implement this is to just change the public interfaces of the various connect/listen functions to take in a SocketAddr instead of a u16 and then convert the SocketAddr into a u16 by just grabbing the port, ensuring that the ip address part is equal to some static value like localhost. Or...

Though thinking about this more perhaps the SWITCHBOARD can use a SocketAddr as a key and then you can bind and connect to the whole address/port space fairly easily. This probably sounds closer to what you're trying to achieve correct?

@dusty-phillips
Copy link
Contributor Author

Yeah, I figure the switchboard can have keys for every possible host/port combo, and therefore mimic "The entire internet" instead of just one port.

It wouldn't make sense to listen on 0.0.0.0 in that case, but if we assume one ip per machine, I think it's reasonable to set up test nets with multiple addresses.

I'll throw together a prototype tomorrow and see what issues come up.

@bmwill
Copy link
Owner

bmwill commented Jun 4, 2020

Sounds good! Look forward to seeing it :)

@dusty-phillips
Copy link
Contributor Author

It's not quite working yet, but before I forget: Kudos from a fellow Brandon Sanderson fan. ;-)

dusty-phillips pushed a commit to dusty-phillips/memory-socket that referenced this issue Jun 5, 2020
The basic idea is the same, but it better mimics
the normal socket interface.

The Switchboard can now be thought of as
a parody of the entire internet
instead of a single machine.

Most everything is a direct mapping,
but there are two uncertain situations:
 *  When connect() is called, there is no clear indicator
    as to where the client is connecting *from*.
    So if we ever wanted to implement e.g. peer_addr
    on a MemorySocket,
    the behaviour would be undefined.
 *  It is not clear what address to listen on when
     connecting to 0.0.0.0.

Both of these *could* be solved by mapping to the local
machine address (using e.g the get_if_addrs crate),
but I'm not sure that make sense.
Another option I toyed with was having a "set_connect_address"
global, but that simply lacked elegance.

( Discussion in bmwill#3 )
@dusty-phillips
Copy link
Contributor Author

There it is.

BTW, in a parallel project, I've been trying to make that set of dialer traits you mentioned, but only for async usage. I'll talk to my boss about open sourcing it since you mentioned it as being useful. They're a bit of a mess, I'm afraid; without generic abstract traits, Rust is, in my opinion, an unfinished language!

@Nautigsam
Copy link

If I'm not mistaken, this change was pushed on master but never released. Could you please publish a new version on Cargo so we can use it?

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

No branches or pull requests

3 participants