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

Don't accept more connections if accept fails #12

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

p12tic
Copy link
Contributor

@p12tic p12tic commented Nov 26, 2022

This fixes infinite loop in case application uses an external io_context in which case _app.stop() does nothing.

This fixes infinite loop in case application uses an external io_context
in which case _app.stop() does nothing.
@dfleury2
Copy link
Owner

Hi,
what's happening is internal io_context is used ?
the do_accept will not be called and the acceptor will not be registered to the io_context ?

Using the added unit tests, I am not sure how I can simulate a failure on on_accept, or doing it using the examples.

@p12tic
Copy link
Contributor Author

p12tic commented Nov 28, 2022

You're right. I will create a failing unit test and get back.

@MiSo1289
Copy link

MiSo1289 commented Aug 27, 2024

Hi @dfleury2 - this issue still occurs on the latest version. From what I can see, it's caused by the acceptor handling the accept error by calling app.stop() - but this method does nothing if user's io context is used. This patch would at least alleviate the issue by not causing infinite spam in the console, only logging the error once; ideally, I would prefer if the error from bind() call in the acceptor could be propagated to the user's thread calling server.listen() as an exception. Should I try to submit such a patch?

@dfleury2
Copy link
Owner

Hi,
yes of course.
The use of an external io_context was added later, and it's not the prefered way to use beauty.
If you succeed this issue, you are welcome.

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

Successfully merging this pull request may close these issues.

3 participants