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

Add EINTR and EAGAIN judge for accept #1438

Merged
merged 3 commits into from
Dec 21, 2022

Conversation

fishpenguin
Copy link
Contributor

This pr add EINTR and EAGAIN process for socket accept function.

When program is blocked by accept, Capturing some signals like SIGUSR2 may interrupt the accept and return error(-1), set errno to EINTR. When this happens, http service shouldn't be shutdown but should continue to call accept.

I propose this pr because I met such problem, I want to shutdown my http service by stop function when I capture signals, but this will shutdown straightly.

@yhirose
Copy link
Owner

yhirose commented Nov 24, 2022

@fishpenguin thanks for the pull request. Could you explain more about EAGAIN? In what situation did you receive EAGAIN?

@fishpenguin
Copy link
Contributor Author

@yhirose I didn't meet EAGAIN error, it usually happens when the socket network resource temporarily unavailable. EAGAIN shouldn't be treat as an error, it won't destroy socket. When EAGAIN happens, socket should retry as man accept said.

@yhirose
Copy link
Owner

yhirose commented Nov 27, 2022

@fishpenguin, thank you for the reply. Could you add at least one unit test case to validate your modification works? Then, I'll do code review. Thanks!

@yhirose
Copy link
Owner

yhirose commented Dec 5, 2022

@fishpenguin any response?

@fishpenguin
Copy link
Contributor Author

@yhirose sorry, I was busy with other things these days and forgot this pr. I added a unit test: the unit test send SIGINT to the thread which is in listening, and the listen_after_bind() won't quit. If the EINTR isn't judged, it will quit when receive the signal.

@yhirose
Copy link
Owner

yhirose commented Dec 16, 2022

@fishpenguin thanks for adding the unit test. Could you take a look at the build error on GitHub Action?

@yhirose yhirose merged commit 227d2c2 into yhirose:master Dec 21, 2022
@yhirose
Copy link
Owner

yhirose commented Dec 21, 2022

@fishpenguin thanks for the fine contribution!

ExclusiveOrange pushed a commit to ExclusiveOrange/cpp-httplib-exor that referenced this pull request May 2, 2023
* Add EINTR and EAGAIN judge for accept

* Add EINTR signal tests

* Cancel win32 and win64 compile on signal unittest

Co-authored-by: yukun.yu <[email protected]>
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.

2 participants