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

bufferevent_replacefd close socket when bufferevent‘s socket fd is eq new fd #1578

Open
mllw opened this issue Mar 18, 2024 · 3 comments
Open

Comments

@mllw
Copy link

mllw commented Mar 18, 2024

in function
int
bufferevent_replacefd(struct bufferevent *bev, evutil_socket_t fd)

line 896
only check
if (old_fd != EVUTIL_INVALID_SOCKET)

but not check
fd is eq old_fd

@mllw mllw changed the title bufferevent_replacefd close socket when bufferevent‘s socket fd is eq release fd bufferevent_replacefd close socket when bufferevent‘s socket fd is eq new fd Mar 18, 2024
@azat
Copy link
Member

azat commented Mar 29, 2024

Thanks for the report.

The check for old_fd is done to check should be call close or not, for new fd we do not need this.
Are you experiencing some issues with this behavior?

@mllw
Copy link
Author

mllw commented Apr 2, 2024

I first exported evhttp_get_request as a public function for logical processing.

After accepting a socket, use bufferevent to read the handshake information, then determine the protocol type, if it is http protocol, it is processed by evhttp_get_request.
This is done by passing the bufferevent and socket fd into evhttp_get_request, however, in the processing of evhttp_get_request, the bufferevent_replacefd function will be called, and at this time, the fd of the bufferevent and the fd of the socket are equal,. bufferevent_replacefd does not determine whether the new fd and old fd are equal, but instead closes the old fd directly, causing the socket to be closed.

@azat
Copy link
Member

azat commented Apr 7, 2024

I first exported evhttp_get_request as a public function for logical processing.

This is not an official API
Thought it had been requested multiple times - #1471
Are you interested?
I'm OK with exporting it if it will have some example that will show that it make sense in the official repo

but not check
fd is eq old_fd

And then I'm ok with this change as well

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

No branches or pull requests

2 participants