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

use getsockopt to check connection state after poll indicate writable #93

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

Conversation

MOON-CLJ
Copy link
Contributor

No description provided.

@MOON-CLJ MOON-CLJ changed the title use getsockopt to check connection state after poll indicate writable use getsockopt to check connection state after poll indicate writable[WIP] Jan 26, 2019
@MOON-CLJ MOON-CLJ changed the title use getsockopt to check connection state after poll indicate writable[WIP] use getsockopt to check connection state after poll indicate writable Jan 27, 2019
@MOON-CLJ MOON-CLJ requested review from mckelvin and tclh123 January 30, 2019 09:39
@mckelvin
Copy link
Contributor

Could you add more details on why we need this? It'd would best if you could add a test case or a piece of POC code to reproduce the issue you want to fix.

@MOON-CLJ
Copy link
Contributor Author

MOON-CLJ commented Jan 31, 2019

@mckelvin this pr contains two points,

1,as suggestion by https://man.openbsd.org/connect.2 ,also see the https://man.openbsd.org/connect.2#EXAMPLES 。 add getsockopt with option SO_ERROR after poll indicate ok to double check after connect。it's not easy to add a test。
2,again,add more log with connection's pointer address to expose connection's state。

@@ -134,23 +135,44 @@ int Connection::connectPoll(int fd, struct addrinfo* ai_ptr) {
pollfd_t pollfds[n_fds];
pollfds[0].fd = fd;
pollfds[0].events = POLLOUT;
// clean errno
errno = 0;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

not necessary?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@tclh123 https://github.com/douban/libmc/blob/master/include/Common.h#L61 log will use errno,if not clean here。such as EINPROGRESS EALREADY above will show in the log,it's noisy。

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