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

FreeBSD: fix read write kevent #1186

Closed
wants to merge 2 commits into from

Conversation

azlm8t
Copy link
Contributor

@azlm8t azlm8t commented Oct 16, 2018

On FreeBSD, the kevent EVFILT_READ=-1 and EVFILT_WRITE=-2 (rather than 1 and 2), so when they are ORed together you get -1, so we only end up registering READ in tvhpoll.

This means that on an async tcp connect we eventually timeout since we are not registered for WRITE.

So we now delete the special logic and use the existing separate read/write registration logic.

Also no longer return EIO for FreeBSD on tcp_socket_dead for zero bytes available to read, otherwise streaming fails to my tablet since we get no bytes to read so immediately disconnect the client, but no bytes on async socket is not same as eof.

Otherwise code such as webui which does "if (tcp_socket_dead(...))"
fails since we were returning EIO if there is nothing to read, but
nothing to immediately read is not the same as EOF on FreeBSD.
Since what occurs is the async tcp connect returns EINPROGRESS,
the socket has no bytes to read, so is immediately disconnected.
The kevent does not take a bitmask. So if you register for READ|WRITE
then it only registers READ since READ=-1 and WRITE=-2.

This means that with an async socket connect then you do not get a
callback on connect.

So we need to register these separately.
@@ -458,7 +458,7 @@ tcp_socket_dead(int fd)
if (err < 0)
return -errno;
else if (err == 0)
return -EIO;
Copy link
Contributor

Choose a reason for hiding this comment

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

It looks like that 3895c92 should be reverted ?

@perexg
Copy link
Contributor

perexg commented Oct 16, 2018

I applied kevent fix and reverted 3895c92 .

@perexg perexg closed this Oct 16, 2018
@azlm8t
Copy link
Contributor Author

azlm8t commented Oct 16, 2018

Thanks. You're right, that earlier change should be reverted.

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.

None yet

2 participants