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

[QUIC] QuicListener.ListenEndPoint returns a port of 0 if Start() hasn't been called #2262

Closed
scalablecory opened this issue Jan 28, 2020 · 7 comments · Fixed by #51512
Closed
Assignees
Labels
area-System.Net.Quic enhancement Product code improvement that does NOT require public API changes/additions
Projects
Milestone

Comments

@scalablecory
Copy link
Contributor

We should probably make this property throw if the listener hasn't been started.

var listener = new QuicListener(new IPEndPoint(IPAddress.Loopback, 0), sslOpts);
IPEndPoint endPoint = listener.ListenEndPoint; // will have a port of 0.
listener.Start();
endPoint = listener.ListenEndPoint; // will now have a valid port.

CC @jkotalik

@scalablecory scalablecory added this to To Do in HTTP/3 via automation Jan 28, 2020
@Dotnet-GitSync-Bot Dotnet-GitSync-Bot added the untriaged New issue has not been triaged by the area owner label Jan 28, 2020
@karelz karelz added enhancement Product code improvement that does NOT require public API changes/additions and removed untriaged New issue has not been triaged by the area owner labels Feb 21, 2020
@karelz karelz added this to the Future milestone Feb 21, 2020
@karelz karelz changed the title QuicListener.ListenEndPoint returns a port of 0 if Listen() hasn't been called [QUIC] QuicListener.ListenEndPoint returns a port of 0 if Listen() hasn't been called Mar 11, 2020
@scalablecory scalablecory modified the milestones: Future, 6.0.0 Aug 11, 2020
@geoffkizer geoffkizer changed the title [QUIC] QuicListener.ListenEndPoint returns a port of 0 if Listen() hasn't been called [QUIC] QuicListener.ListenEndPoint returns a port of 0 if Start() hasn't been called Nov 14, 2020
@geoffkizer

This comment has been minimized.

@geoffkizer
Copy link
Contributor

Having a property throw usually isn't great, but I suppose you can argue that this is incorrect usage and thus reasonable.

I do think it raises the question: why do we have a separate call to Start()?

What would you ever do between calling the QuicListener constuctor and Start()? Seems like the answer is "nothing useful", and as such we should just get rid of Start.

@geoffkizer
Copy link
Contributor

I suppose another option would be to make the constructor do the equivalent of calling Socket.Bind, but not Socket.Listen. This would at least give you a useful value for LocalEndPoint. Then, Start() would do the equivalent of Listen.

@karelz
Copy link
Member

karelz commented Apr 15, 2021

Triage: We want to merge Start into constructor -- no point in having them separate.
Affects API.

@karelz karelz moved this from To Do (Low Priority) to To Do (High Priority) in HTTP/3 Apr 15, 2021
@CarnaViire CarnaViire self-assigned this Apr 19, 2021
@CarnaViire CarnaViire moved this from To Do (High Priority) to In Progress in HTTP/3 Apr 19, 2021
@ghost ghost added the in-pr There is an active PR which will close this issue when it is merged label Apr 19, 2021
@scalablecory
Copy link
Contributor Author

scalablecory commented Apr 20, 2021

@jkotalik @Tratcher we're considering getting rid of Start()/Stop() and just having ctor/Dispose() do these things. This would avoid the 0 port issue as well. How will this impact Kestrel?

@Tratcher
Copy link
Member

@halter73 @JamesNK

@Tratcher
Copy link
Member

Tratcher commented Apr 20, 2021

Not much, Kestrel immediately calls Start:
https://github.com/dotnet/aspnetcore/blob/ea2f5594e4f817c828904421b33bed3f921907ec/src/Servers/Kestrel/Transport.Quic/src/Internal/QuicConnectionListener.cs#L46-L47

HTTP/3 automation moved this from In Progress to Done Apr 21, 2021
@ghost ghost removed the in-pr There is an active PR which will close this issue when it is merged label Apr 21, 2021
@ghost ghost locked as resolved and limited conversation to collaborators May 21, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-System.Net.Quic enhancement Product code improvement that does NOT require public API changes/additions
Projects
No open projects
HTTP/3
  
Done
Development

Successfully merging a pull request may close this issue.

6 participants