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

Make libuv server api semantically similar to asio #52

Merged
merged 7 commits into from
Dec 27, 2024

Conversation

uatuko
Copy link
Owner

@uatuko uatuko commented Dec 22, 2024

This is essentially building upon the work done in #42 and #44, which allowed the server to run using an external libuv event loop. But instead of using an external loop this change allows the internal uv loop to be run externally or add external events to the loop.

Main advantage of this change is that it makes the libuv server API semantically similar to asio server API, without removing functionality. It also makes the code a bit simpler since we don't have to check if it's an external loop or not.

This is essentially building on #42 and #44, which introduced external
event loops, and making the libuv api semantically similar to asio.
@uatuko uatuko force-pushed the feature/asio-uv-api-parity branch from 02e48b9 to baddf59 Compare December 22, 2024 14:24
@uatuko uatuko marked this pull request as ready for review December 22, 2024 19:40
@uatuko
Copy link
Owner Author

uatuko commented Dec 22, 2024

@tchernobog thoughts?

@tchernobog
Copy link
Contributor

@tchernobog thoughts?

I think in general this is a good change. Thanks! The only item where I am thinking that it is a bit asymmetric, is that:

The socket handler must be already bound to a network address and will be closed when the server stops.

On one hand, we assume the initialization is done by an external entity. On the other, responsibility for de-initialization is moved to this object. I wonder if there are good use cases for reusing the socket or other reasons to avoid passing ownership. In that case closing should be up to the caller.

Otherwise, as a compromise, I would prefer if at least the parameter had type uv_os_sock_t&& sock so that it is clear it is moved at the call site by an explicit operation.

docs/api.md Outdated Show resolved Hide resolved
docs/api.md Outdated Show resolved Hide resolved
@uatuko
Copy link
Owner Author

uatuko commented Dec 24, 2024

Otherwise, as a compromise, I would prefer if at least the parameter had type uv_os_sock_t&& sock so that it is clear it is moved at the call site by an explicit operation.

I did think this would be better to have uv_os_sock_t&& but wasn't sure if there's a case where the caller would want to keep the ownership of the socket. I'll make the change as suggested 👍.

@uatuko uatuko merged commit 859f1ae into main Dec 27, 2024
12 checks passed
@uatuko uatuko deleted the feature/asio-uv-api-parity branch December 27, 2024 16:54
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