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

std::rt improvements #9901

Merged
merged 23 commits into from
Oct 24, 2013
Merged

std::rt improvements #9901

merged 23 commits into from
Oct 24, 2013

Conversation

alexcrichton
Copy link
Member

Large topics:

  • Implemented rt::io::net::unix. We've got an implementation backed by "named pipes" for windows for free from libuv, so I'm not sure if these should be cfg(unix) or whether they'd be better placed in rt::io::pipe (which is currently kinda useless), or to leave in unix. Regardless, we probably shouldn't deny windows of functionality which it certainly has.
  • Fully implemented net::addrinfo, or at least fully implemented in the sense of making the best attempt to wrap libuv's getaddrinfo api
  • Moved standard I/O to a libuv TTY instead of just a plain old file descriptor. I found that this interacted better when closing stdin, and it has the added bonus of getting things like terminal dimentions (someone should make a progress bar now!)
  • Migrate to ~Trait instead of a typedef'd object where possible. There are only two more types which are blocked on this, and those are traits which have a method which takes by-value self (there's an open issue on this)
  • Drop rt::io::support::PathLike in favor of just ToCStr. We recently had a lot of Path work done, but it still wasn't getting passed down to libuv (there was an intermediate string conversion), and this allows true paths to work all the way down to libuv (and anything else that can become a C string).
  • Removes extra::fileinput and extra::io_util

Closes #9895
Closes #9975
Closes #8330
Closes #6850 (ported lots of libraries away from std::io)
cc #4248 (implemented unix/dns)
cc #9128 (made everything truly trait objects)

@alexcrichton
Copy link
Member Author

I tried to keep each commit separate from the others, so this may be best inspected commit-by-commit. I tried to keep this small but things kinda just snowballed into one another.

@thestinger
Copy link
Contributor

By standard I/O do you mean stdout and stdin? Does it still work properly when the program isn't running in a terminal, and output is piped somewhere else?

@alexcrichton
Copy link
Member Author

Ah yes, I do mean stdout/stdin. And yes, turns out libuv is pretty smart and handles them fine if they're redirected as well as attached to a terminal.

@alexcrichton
Copy link
Member Author

This is still a little in-flux as it's not passing make check. Most of the trouble seems to come down to blocking vs non-blocking stdio. When we use the TTY handles from libuv, it sets the file descriptors in non-blocking mode, but then when we making blocking calls with io::native it returns an error EAGAIN. This is pretty normal, but right now there's no condition handler registered which ends up in all sorts of disaster.

There are only 2 users of native::stdio which are logging and dumb_println. I'm moving logging to uv stdio (or whatever the local event loop provides), and then I'm modifying dumb_println to handle EAGAIN, although this shouldn't be called that often (only rterrln! and rtabort! use it currently, and maybe rtdebug!).

I've also finished migrating everything to a trait object instead of using typedefs.

I'll update this once I have everything passing make check

@alexcrichton
Copy link
Member Author

Notable new changes

  • extra::fileinput was removed
  • extra::io_util was removed
  • task failure logging now forcibly goes to the console instead of going through the task logger (see comments in the commit/file)

@bill-myers
Copy link
Contributor

@alexcrichton if libuv indeed sets stdin/stdout to non-blocking, won't that also wreak havoc with C libraries doing printf, etc.? It seems doing such a thing is fundamentally wrong, and libuv should either learn to do async I/O without setting things to non-blocking (i.e. poll before read/write) or use a separate thread for stdin/stdout with blocking calls

@alexcrichton
Copy link
Member Author

I've explicitly chosen to make stdout/stderr as async instead of synchronous, but we can tell libuv to make synchronous writes instead. We're forced to make stdin async, however, but I think that it's less of a problem.

That is an interesting concern though. I doubt any c library would panic or anything (who checks the return value of printf?), but it would mean that some prints could possibly get lost. This doesn't happen 100% of the time (only when another write is currently in-flight I believe), so it's not like it would invalidate all FFI printfs, but it probably wouldn't be great either.

@brson
Copy link
Contributor

brson commented Oct 21, 2013

Heroic effort! 🌊

alexcrichton and others added 19 commits October 24, 2013 14:21
We get a little more functionality from libuv for these kinds of streams (things
like terminal dimentions), and it also appears to more gracefully handle the
stream being a window. Beforehand, if you used stdio and hit CTRL+d on a
process, libuv would continually return 0-length successful reads instead of
interpreting that the stream was closed.

I was hoping to be able to write tests for this, but currently the testing
infrastructure doesn't allow tests with a stdin and a stdout, but this has been
manually tested! (not that it means much)
This moves as many as I could over to ~Trait instead of ~Typedef. The only
remaining one is the IoFactoryObject which should be coming soon...
This removes the PathLike trait associated with this "support module". This is
yet another "container of bytes" trait, so I didn't want to duplicate what
already exists throughout libstd. In actuality, we're going to pass of C strings
to the libuv APIs, so instead the arguments are now bound with the 'ToCStr'
trait instead.

Additionally, a layer of complexity was removed by immediately converting these
type-generic parameters into CStrings to get handed off to libuv apis.
This involved changing a fair amount of code, rooted in how we access the local
IoFactory instance. I added a helper method to the rtio module to access the
optional local IoFactory. This is different than before in which it was assumed
that a local IoFactory was *always* present. Now, a separate io_error is raised
when an IoFactory is not present, yet I/O is requested.
There are no longer any remnants of typedefs, and everything is now built on
true trait objects.
When uv's TTY I/O is used for the stdio streams, the file descriptors are put
into a non-blocking mode. This means that other concurrent writes to the same
stream can fail with EAGAIN or EWOULDBLOCK. By all I/O to event-loop I/O, we
avoid this error.

There is one location which cannot move, which is the runtime's dumb_println
function. This was implemented to handle the EAGAIN and EWOULDBLOCK errors and
simply retry again and again.
Big fish fried here:

    extra::json
    most of the compiler
    extra::io_util removed
    extra::fileinput removed

Fish left to fry

    extra::ebml
The isn't an ideal patch, and the comment why is in the code. Basically uvio
uses task::unkillable which touches the kill flag for a task, and if the task is
failing due to mismangement of the kill flag, then there will be serious
problems when the task tries to print that it's failing.
The general idea is to remove conditions completely from I/O, so in the meantime
remove the read_error condition to mean the same thing as the io_error condition.
I was seeing a lot of weird behavior with stdin behaving as a tty, and it
doesn't really quite make sense, so instead this moves to using libuv's pipes
instead (which make more sense for stdin specifically).

This prevents piping input to rustc hanging forever.
This adds constructors to pipe streams in the new runtime to take ownership of
file descriptors, and also fixes a few tests relating to the std::run changes
(new errors are raised on io_error and one test is xfail'd).
descriptive names
easier-to-use api
reorganize and document
bors added a commit that referenced this pull request Oct 24, 2013
Large topics:

* Implemented `rt::io::net::unix`. We've got an implementation backed by "named pipes" for windows for free from libuv, so I'm not sure if these should be `cfg(unix)` or whether they'd be better placed in `rt::io::pipe` (which is currently kinda useless), or to leave in `unix`. Regardless, we probably shouldn't deny windows of functionality which it certainly has.
* Fully implemented `net::addrinfo`, or at least fully implemented in the sense of making the best attempt to wrap libuv's `getaddrinfo` api
* Moved standard I/O to a libuv TTY instead of just a plain old file descriptor. I found that this interacted better when closing stdin, and it has the added bonus of getting things like terminal dimentions (someone should make a progress bar now!)
* Migrate to `~Trait` instead of a typedef'd object where possible. There are only two more types which are blocked on this, and those are traits which have a method which takes by-value self (there's an open issue on this)
* Drop `rt::io::support::PathLike` in favor of just `ToCStr`. We recently had a lot of Path work done, but it still wasn't getting passed down to libuv (there was an intermediate string conversion), and this allows true paths to work all the way down to libuv (and anything else that can become a C string).
* Removes `extra::fileinput` and `extra::io_util`


Closes #9895 
Closes #9975
Closes #8330
Closes #6850 (ported lots of libraries away from std::io)
cc #4248 (implemented unix/dns)
cc #9128 (made everything truly trait objects)
@bors bors closed this Oct 24, 2013
@bors bors merged commit 188e471 into rust-lang:master Oct 24, 2013
bors added a commit that referenced this pull request Oct 25, 2013
This is more progress towards #9128 and all its related tree of issues. This implements a new `BasicLoop` on top of pthreads synchronization primitives (wrapped in `LittleLock`). This also removes the wonky `callback_ms` function from the interface of the event loop.

After #9901 is taking forever to land, I'm going to try to do all this runtime work in much smaller chunks than before. Right now this will not work unless #9901 lands first, but I'm close to landing it (hopefully), and I wanted to go ahead and get this reviewed before throwing it at bors later on down the road.

This "pausible idle callback" is also a bit of a weird idea, but it wasn't as difficult to implement as callback_ms so I'm more semi-ok with it.
flip1995 pushed a commit to flip1995/rust that referenced this pull request Dec 1, 2022
Don't lint `explicit_auto_deref` when the initial type is neither a reference, nor a receiver

fixes rust-lang#9901
fixes rust-lang#9777
changelog: `explicit_auto_deref`: Don't lint when the initial value is neither a reference, nor a receiver
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
5 participants