practical impl of low-level libuv bindings, beginning work on higher-level bindings #2134
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
lots of changes, here.. should've commited sooner. - added uv::direct module that contains rust fns that map, neatly, to the libuv c library as much as possible. they operate on ptrs to libuv structs mapped in rust, as much as possible (there are some notable exceptions). these uv::direct fns should only take inputs from rust and, as neccesary, translate them into C-friendly types and then pass to the C functions. We want to them to return ints, as the libuv functions do, so we can start tracking status. - the notable exceptions for structs above is due to ref gh-1402, which prevents us from passing structs, by value, across the Rust<->C barrier (they turn to garbage, pretty much). So in the cases where we get back by-val structs from C (uv_buf_init(), uv_ip4_addr(), uv_err_t in callbacks) , we're going to use *ctypes::void (or just errnum ints for uv_err_t) until gh-1402 is resolved. - using crust functions, in these uv::direct fns, for callbacks from libuv, will eschew uv_err_t, if possible, in favor a struct int.. if at all possible (probably isn't.. hm.. i know libuv wants to eventually move to replace uv_err_t with an int, as well.. so hm). - started flushing out a big, gnarly test case to exercise the tcp request side of the uv::direct functions. I'm at the point where, after the connection is established, we write to the stream... when the writing is done, we will read from it, then tear the whole thing down. overall, it turns out that doing "close to the metal" interaction with c libraries is painful (and more chatty) when orchestrated from rust. My understanding is that not much, at all, is written in this fashion in the existant core/std codebase.. malloc'ing in C has been preferred, from what I've gathered. So we're treading new ground, here!
.. but passing sockaddr_in by val back to C is broken, still passing by ptr .. the uv_write_cb is processed, but we have a status -1.. there is also valgrind spew.. so buf passing is broken, still.
ways to go, still..
…RGET have to use ++ sigil in rust-side extern fn decls in order to have rust actually copy the struct, by value, onto the C stack. gotcha, indeed. also adding a helper method to verify/remind how to pass a struct by-val into C... check out the rust fn sig for rust_uv_ip4_test_verify_port_val() for more infos
…-outs so we're now adhering the libuv C api and passing structs by-val where it is expected, instead of pulling pointer trickery (or worse having to malloc structs in c++ to be passed back to rust and then into C again)
.. up next: windows! .. impl'd uv::direct::read_stop() and uv::direct::close() to wrap things up .. demonstrated sending data out of the uv_read_cb via a channel (which we block on to recv all of it, complete w/ EOF notification) that is read from after the loop exits. .. helpers to read the guts of a uv_buf_t .. an idea im kicking around: starting to pile up all of these hideous data accessor functions in uv::direct .. I might make impl/iface pairs for the various uv_* types that I'm using, in order to encapsulate those data access functions and, perhaps, make the access look a little cleaner (it still won't be straight field access, but it'll be a lot better) .. formatting cleanup to satisfy make check
.. im now going to refactor the tcp request and server tests to utilize each other, so no more external network ugliness
..plus whitespace cleanup for make check
.. 32bit linux issues persist.
also println->log(debug,) and assorted cleanup ahead of merge to master
|
OK. This is super-fantastic. It will take me some time to go through it, and I'll probably push it without a full review. First thing I noticed is that the rustdoc headers don't follow the existing conventions (which only exist in my head): function arguments should be under an |
|
I got the I have no issue adjusting either. |
|
uv.rs has some test code that is not conditionally compiled (e.g. |
|
|
|
Oh, this is merged. High-fives all around. |
brson
pushed a commit
that referenced
this pull request
Apr 20, 2012
celinval
pushed a commit
to celinval/rust-dev
that referenced
this pull request
Jun 4, 2024
Kobzol
pushed a commit
to Kobzol/rust
that referenced
this pull request
Dec 30, 2024
bors
pushed a commit
to rust-lang-ci/rust
that referenced
this pull request
Jan 2, 2025
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Contained in this pull request are a few things:
uv_hl.rs). This work is contingent on getting some API for interacting with a global channel/task (where a process-wife libuv loop will live) from @brson.Right now, the tcp server/request test (
test_uv_tcp_server_and_request) is disabled on 32bit linux because of too many minefields that we're encountering because of, it seems, #2064. I will open another ticket to get this test passing.The
uvrust API has also been split up into multiple files, somewhat:std::uvwill continue to be the "top-level" module. It will import and re-export, as needed, all of the other uv submodules, like:std::uv::llakauv_ll.rs-- contains the rust records corresponding to libuv C structs, as well as direct, low-level bindings to various bits of libuv API. Also some helper functions. This needs more docs. Everything in this module is markedunsafe.std::uv::hlakauv_hl.rs-- some initial thoughts, from me, for how the "high-level" API (to be used by libstd developers who want to write rust-idiomatic bindings to libuv facilities) should be structured. It's very WIP and useless, at the moment, pending some contributions from @brson. This module will aim to contain only safe functions for developers to use, but this may not hold up. There are possibly situation where we'll want to give developers "escape hatch" access to the actual libuv pointers, although the preference will be to wrap them in non-exported, opaque enums whenever possible (for use with the safeuv::hlAPI).std::uvstill run and work, as advertised by their tests, but I will deprecate their implementations soon and either remove them entirely or re-implement them usinguv::hl, once the rest of that lands. The top-levelstd::uvdocument notes this fact.Anyways: with this pull request landed, we'll be moving (quickly!) to implement a process-wide libuv loop that will be encapsulated in the
uv::hlAPI. Once that work lands, we'll be able to paralellize adding the rest of the libuv API, as needed, to create high-level modules to meet the various IO needs for libstd.