Skip to content

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.

@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.

3 participants