ephemeral port#293
Conversation
| return RSocketFactory | ||
| .connect() | ||
| .transport(TcpClientTransport.create("localhost", 8000)) | ||
| .transport(TcpClientTransport.create(address.getHostString(), address.getPort())) |
There was a problem hiding this comment.
I am considering adding TcpClientTransport.create(InetSocketAddress) here
|
cc @phoad |
| .receive() | ||
| .acceptor((setup, sendingSocket) -> Mono.just(new RSocketProxy(handler))) | ||
| .transport(TcpServerTransport.create("localhost", 8000)) | ||
| .transport(serverTransport) |
There was a problem hiding this comment.
Ideally server would be a better type than Closeable, then I could get address off that.
| RSocket client = buildClient(); | ||
|
|
||
| // TODO without this failure is always a timeout | ||
| Thread.sleep(1000); |
| return out.neverComplete(); | ||
| }); | ||
|
|
||
| serverMono.map(s -> s.address()).subscribe(address); |
There was a problem hiding this comment.
If we provide port value of "0", each time we call start(), it generates a new NettyContext, which has a different "address" - "port" value. So if the caller first calls start(..) and then asks for the "address" value, while another caller also calls start(..) and asks for the address value, then there will be a race condition. Other than that, when we call start(..) at the inside of a single thread, the address value of the initial call will be lost.
It seems we only need the "address" if we pass port value of "0", otherwise address can be created by merging "bindAddress" and "port"? If so, we might have a overload method which also provides the address value back? If caller really needs the address back, the caller can call this overloaded one. So we will not need any class member and it will still be ok for multithreading?
There was a problem hiding this comment.
I hadn't considered that you would reuse a TcpServerTransport. It seems problematic to do so as the code currently stands as you either
- use a static port and clients are responsible for remembering the state e.g. is there an open server port? The start call will fail unless a client has called closed.
- use a free port (0) and then multiple calls to start work but with the race condition and address() is on the wrong class.
Overall this makes me think my original comment in the PR is correct "Alternatively we can make sure that server is a better type than Closeable." So this start method should return something more useful than Closeable (representing the server, not a single connection).
| }) | ||
| .map(NettyContextClosable::new); | ||
| } | ||
| .newHandler((in, out) -> { |
Allow TCP server to use an ephemeral port