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

If listener.Accept returns an error then the server stops responding #21

Open
stefanba3 opened this issue Aug 10, 2021 · 2 comments
Open
Labels

Comments

@stefanba3
Copy link

stefanba3 commented Aug 10, 2021

For example, here:

func (s *server) run() {
	defer s.wg.Done()
	for {
		client, err := s.netListener.Accept()
		if err != nil {
			break
		}

		s.handle(client)
	}
}

netListener is commonly a TLS listener, which can return an error from Accept for many reasons. Breaking out of the loop stops the server from accepting new connections. And, as far as I can tell, there is no way to know from the outside (since run gets called in a goroutine) that this has happened.

@andrewkroh andrewkroh added the bug label Aug 10, 2021
@stefanba3
Copy link
Author

Actually, I am wrong. TLS wont return handshake errors from Accept. So this issue is less serious than I thought. I don't know if we can assume Accept will only return an error if closed, so it might still be a good idea to guard against that case.

@stefanba3 stefanba3 changed the title If listener.Accept returns an error (e.g. a TLS certificate error) then the server stops responding If listener.Accept returns an error then the server stops responding Aug 10, 2021
@andrewkroh
Copy link
Member

I think this code could use errors.Is(err, net.ErrClosed) to determine if the loop should break. Other errors should probably be logged, but continue to Accept().

net.ErrClosed is newly exported in Go 1.16. golang/go#4373 (comment)

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