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

[Socket] Server::listen() is blocking if given a hostname #181

Closed
clue opened this issue Apr 20, 2013 · 2 comments
Closed

[Socket] Server::listen() is blocking if given a hostname #181

clue opened this issue Apr 20, 2013 · 2 comments
Labels

Comments

@clue
Copy link
Member

clue commented Apr 20, 2013

Calling Socket\Server::listen($port, $host) with a hostname instead of an IPv4 address is a blocking operation.

Example:

$server = new Socket\Server($loop);
$server->listen(1234, 'example.com');

Giving a hostname will result in resolving it via DNS (blocking).

Possible solutions include:

  1. Reject hostnames altogether and only accept IP addresses. Easy temporary solution, but listening on hostnames is a convenient feature, think of listening on localhost:1234
  2. Accept this as a known limitation and block. Not likely that we want to see something like this in an async library though
  3. Async resolving via React\Dns component: Requires passing in a Resolver instance somewhere and will have to change the listen() method to return a Promise instead. Should be pretty straight forward, but changing the API in such a subtle way is likely going to break existing code
  4. Create new Socket\Factory class that does all the work, accept an optional Resolver instance to resolve any hostnames before passing it to a listen() call.

Personally, I'd go for suggestion 4 as we have a number of outstanding issues with Socket\Server anyway (UNIX sockets, SSL sockets, no IPv6 support, API consistency, etc.). I've implemented a very similar factory for clue/socket-react here and here and for clue/socket-raw here. It's been working out quite good (feel free to take a look at each projects' examples) and I'd love to see something similar in react.

In the future this could possibly also allow merging SocketClient and Socket into a single package as they share quite a bit of code and it could allow us to call

Factory::createServer('localhost:1234')->then(function (Server $server) { });
Factory::createClient('localhost:1234')->then(function (Connection $client) {
   $client->end('hello');
});

This provides a very consistent API, is very similar to how stream_socket_server() and stream_socket_client() work and could easily be extended to support IPv6, SSL and UNIX domain sockets.

@igorw
Copy link
Contributor

igorw commented Apr 21, 2013

While a factory seems like a good local solution, I'm not so happy with the fact that the overall code base is turning into factory-land. I'd rather we could find a better solution. Async-DIC might be it, so maybe we should revisit that.

I need to think about this some more. Poke me if you have any ideas.

@clue
Copy link
Member Author

clue commented Jun 14, 2014

Moved to reactphp/socket#7.

@clue clue closed this as completed Jun 14, 2014
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants