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

RUST_THREADS=1 must stay with a single thread, which the runtime doesn't #9373

Closed
glycerine opened this issue Sep 21, 2013 · 4 comments
Closed

Comments

@glycerine
Copy link

rustc 0.8-pre (570431f 2013-09-19 15:56:04 -0700)

RUST_THREADS=1 must not allow the rust runtime to start a second thread, which it does now. This is really bad.

Background: If I wish to fork(2) and use the resulting image, only the thread that calls fork(2) gets duplicated. If any other thread has locked a mutex (say for printf, or malloc) and was in the middle of a critical section, then those locks remain locked, the other threads vanish, and my new child process is hosed.

From the rust-dev thread on rusti - the - repl renovation: (20 sept 2013):

I'm trying some sanity checks. This one had a curious result. I did
$ export RUST_THREADS=1
and then started rusti under gdb. Expected: only one thread going. Observed: I have two threads going instead.

(This is troublesome, because fork will never work if Rust doesn't honor the request of RUST_THREADS=1; you can't mix threads and fork; explanation: http://www.linuxprogrammingblog.com/threads-and-fork-think-twice-before-using-them )

Q: Is there a way to really just get one thread in the rust runtime? Best case, I'm hoping the two threads observed is just a bug that can be fixed.

Jason

From: Alex Crichton alex@crichton.co
Date: Fri, Sep 20, 2013 at 4:24 PM
Subject: Re: [rust-dev] rusti - the - repl renovation
To: "Jason E. Aten" j.e.aten@gmail.com

Q: Is there a way to really just get one thread in the rust runtime? Best
case, I'm hoping the two threads observed is just a bug that can be fixed.
Right now the runtime will always spawn at least one thread, so
without turning off the runtime you'll have at least two threads.
That's arguably a bug in the runtime though...

jaten@fre:/usr/cn/rust/debug-build/rust/x86_64-unknown-linux-gnu/stage2/bin$ ./rustc -v
./rustc 0.8-pre (570431f 2013-09-19 15:56:04 -0700)
host: x86_64-unknown-linux-gnu
jaten@fre:/usr/cn/rust/debug-build/rust/x86_64-unknown-linux-gnu/stage2/bin$ 



jaten@fre:/usr/cn/rust/debug-build/rust/x86_64-unknown-linux-gnu/stage2/bin$ env|grep RUST
RUST_THREADS=1
jaten@fre:/usr/cn/rust/debug-build/rust/x86_64-unknown-linux-gnu/stage2/bin$ gdb ./rusti
GNU gdb (Ubuntu/Linaro 7.4-2012.04-0ubuntu2.1) 7.4-2012.04
Copyright (C) 2012 Free Software Foundation, Inc.
License GPLv3+: GNU GPL version 3 or later <http://gnu.org/licenses/gpl.html>
This is free software: you are free to change and redistribute it.
There is NO WARRANTY, to the extent permitted by law.  Type "show copying"
and "show warranty" for details.
This GDB was configured as "x86_64-linux-gnu".
For bug reporting instructions, please see:
<http://bugs.launchpad.net/gdb-linaro/>...
Reading symbols from /usr/cn/rust/debug-build/rust/x86_64-unknown-linux-gnu/stage2/bin/rusti...(no debugging symbols found)...done.
(gdb) run
Starting program: /usr/cn/rust/debug-build/rust/x86_64-unknown-linux-gnu/stage2/bin/rusti
[Thread debugging using libthread_db enabled]
Using host libthread_db library "/lib/x86_64-linux-gnu/libthread_db.so.1".
[New Thread 0x7ffff7fd3700 (LWP 11639)]
WARNING: The Rust REPL is experimental and may be
unstable. If you encounter problems, please use the
compiler instead. Type :help for help.
rusti>   C-c C-c
Program received signal SIGINT, Interrupt.
0x00007ffff4356148 in pthread_join () from /lib/x86_64-linux-gnu/libpthread.so.0
(gdb) bt
#0  0x00007ffff4356148 in pthread_join () from /lib/x86_64-linux-gnu/libpthread.so.0
#1  0x00007ffff49369d4 in rust_thread::join (this=0x7ffff001f2e0) at src/rt/sync/rust_thread.cpp:65
#2  0x00007ffff4937469 in rust_raw_thread_join (thread=0x7ffff001f2e0) at src/rt/rust_builtin.cpp:417
#3  0x00007ffff768f2e9 in rt::thread::Thread::join::hf3525925b944a51ZTas::v0.8$x2dpre ()
   from /usr/cn/rust/debug-build/rust/x86_64-unknown-linux-gnu/stage2/bin/../lib/libstd-6c65cf4b443341b1-0.8-pre.so
#4  0x00007ffff77cddd3 in rt::run_::h82e8c355ab8d949faz::v0.8$x2dpre ()
   from /usr/cn/rust/debug-build/rust/x86_64-unknown-linux-gnu/stage2/bin/../lib/libstd-6c65cf4b443341b1-0.8-pre.so
#5  0x00007ffff77cb694 in rt::run::hd3cab0f3a053bc41ab::v0.8$x2dpre ()
   from /usr/cn/rust/debug-build/rust/x86_64-unknown-linux-gnu/stage2/bin/../lib/libstd-6c65cf4b443341b1-0.8-pre.so
#6  0x00007ffff770968e in rt::start::h98ebfd32a7b8f1ad::v0.8$x2dpre ()
   from /usr/cn/rust/debug-build/rust/x86_64-unknown-linux-gnu/stage2/bin/../lib/libstd-6c65cf4b443341b1-0.8-pre.so
#7  0x00007ffff77095f7 in unstable::lang::start::h76d6c774aa357c7aaj::v0.8$x2dpre ()
   from /usr/cn/rust/debug-build/rust/x86_64-unknown-linux-gnu/stage2/bin/../lib/libstd-6c65cf4b443341b1-0.8-pre.so
#8  0x0000000000400c2b in main ()
(gdb) i th
  Id   Target Id         Frame
  2    Thread 0x7ffff7fd3700 (LWP 11639) "rusti" 0x00007ffff46508cd in read () from /lib/x86_64-linux-gnu/libc.so.6
* 1    Thread 0x7ffff7fd5780 (LWP 11636) "rusti" 0x00007ffff4356148 in pthread_join () from /lib/x86_64-linux-gnu/libpthread.so.0
(gdb) thread 2
[Switching to thread 2 (Thread 0x7ffff7fd3700 (LWP 11639))]
#0  0x00007ffff46508cd in read () from /lib/x86_64-linux-gnu/libc.so.6
(gdb) bt
#0  0x00007ffff46508cd in read () from /lib/x86_64-linux-gnu/libc.so.6
#1  0x00007ffff45e4ff8 in _IO_file_underflow () from /lib/x86_64-linux-gnu/libc.so.6
#2  0x00007ffff45e603e in _IO_default_uflow () from /lib/x86_64-linux-gnu/libc.so.6
#3  0x00007ffff45da18a in _IO_getline_info () from /lib/x86_64-linux-gnu/libc.so.6
#4  0x00007ffff45d906b in fgets () from /lib/x86_64-linux-gnu/libc.so.6
#5  0x00007ffff4946358 in linenoise (prompt=0x7fffefc58058 "rusti> ") at src/rt/linenoise/linenoise.c:1405
#6  0x00007ffff70a8061 in rl::rustrt::linenoise::h64cc97493178b67aa3::v0.8$x2dpre ()
   from /usr/cn/rust/debug-build/rust/x86_64-unknown-linux-gnu/stage2/bin/../lib/libextra-a7c050cfd46b2c9a-0.8-pre.so
#7  0x00007ffff70a8b26 in rl::read::anon::expr_fn::a1 () from /usr/cn/rust/debug-build/rust/x86_64-unknown-linux-gnu/stage2/bin/../lib/libextra-a7c050cfd46b2c9a-0.8-pre.so
#8  0x00007ffff70a8aba in c_str::CString::with_ref::hb23b2a52bcdd1fsya0::v0.8$x2dpre ()
   from /usr/cn/rust/debug-build/rust/x86_64-unknown-linux-gnu/stage2/bin/../lib/libextra-a7c050cfd46b2c9a-0.8-pre.so
#9  0x00007ffff70a89b4 in c_str::ToCStr::with_c_str::hb23b2a52bcdd1faZ::v0.8$x2dpre ()
   from /usr/cn/rust/debug-build/rust/x86_64-unknown-linux-gnu/stage2/bin/../lib/libextra-a7c050cfd46b2c9a-0.8-pre.so
#10 0x00007ffff70a8936 in rl::read::h55e92cb2e4e46fea8::v0.8$x2dpre ()
   from /usr/cn/rust/debug-build/rust/x86_64-unknown-linux-gnu/stage2/bin/../lib/libextra-a7c050cfd46b2c9a-0.8-pre.so
#11 0x00007ffff4d691a2 in get_line::hacc817425f24a23caR::v0.8$x2dpre ()
   from /usr/cn/rust/debug-build/rust/x86_64-unknown-linux-gnu/stage2/bin/../lib/librusti-53e0ef2ae196aaff-0.8-pre.so
#12 0x00007ffff4d7b1d2 in main_args::h37a11c4051c2827aO::v0.8$x2dpre ()
   from /usr/cn/rust/debug-build/rust/x86_64-unknown-linux-gnu/stage2/bin/../lib/librusti-53e0ef2ae196aaff-0.8-pre.so
#13 0x00007ffff4d7a947 in main::h3a346db0adc4cf51aB::v0.8$x2dpre ()
   from /usr/cn/rust/debug-build/rust/x86_64-unknown-linux-gnu/stage2/bin/../lib/librusti-53e0ef2ae196aaff-0.8-pre.so
#14 0x0000000000400bb9 in main::h4eb1c8bbff1fac2ag::v0.0 ()
#15 0x00007ffff7709758 in unstable::lang::start::anon::expr_fn::a1 ()
   from /usr/cn/rust/debug-build/rust/x86_64-unknown-linux-gnu/stage2/bin/../lib/libstd-6c65cf4b443341b1-0.8-pre.so
#16 0x00007ffff7719814 in rt::task::__extensions__::build_start_wrapper::anon::anon::expr_fn::ab ()
   from /usr/cn/rust/debug-build/rust/x86_64-unknown-linux-gnu/stage2/bin/../lib/libstd-6c65cf4b443341b1-0.8-pre.so
#17 0x00007ffff761849c in unstable::finally::Finally$__extensions__::finally::h199ab8d6eb226980ECan::v0.8$x2dpre ()
   from /usr/cn/rust/debug-build/rust/x86_64-unknown-linux-gnu/stage2/bin/../lib/libstd-6c65cf4b443341b1-0.8-pre.so
#18 0x00007ffff77170d5 in rt::task::__extensions__::run::anon::expr_fn::at ()
   from /usr/cn/rust/debug-build/rust/x86_64-unknown-linux-gnu/stage2/bin/../lib/libstd-6c65cf4b443341b1-0.8-pre.so
#19 0x00007ffff7719ce9 in rt::task::Unwinder::try::try_fn::__rust_abi::Vc ()
   from /usr/cn/rust/debug-build/rust/x86_64-unknown-linux-gnu/stage2/bin/../lib/libstd-6c65cf4b443341b1-0.8-pre.so
#20 0x00007ffff7719c47 in rt::task::Unwinder::try::try_fn::hae27117228cab98fVca9::v0.8$x2dpre ()
   from /usr/cn/rust/debug-build/rust/x86_64-unknown-linux-gnu/stage2/bin/../lib/libstd-6c65cf4b443341b1-0.8-pre.so
#21 0x00007ffff4937787 in rust_try (f=0x7ffff7719bf0 <rt::task::Unwinder::try::try_fn::hae27117228cab98fVca9::v0.8$x2dpre>, fptr=0x7ffff7717080, env=0x7ffff0236348)
    at src/rt/rust_builtin.cpp:523
#22 0x00007ffff7716fa2 in rt::task::Unwinder::try::h199ab8d6eb226980Vcas::v0.8$x2dpre ()
   from /usr/cn/rust/debug-build/rust/x86_64-unknown-linux-gnu/stage2/bin/../lib/libstd-6c65cf4b443341b1-0.8-pre.so
#23 0x00007ffff7716e36 in rt::task::Task::run::h199ab8d6eb226980iXar::v0.8$x2dpre ()
   from /usr/cn/rust/debug-build/rust/x86_64-unknown-linux-gnu/stage2/bin/../lib/libstd-6c65cf4b443341b1-0.8-pre.so
#24 0x00007ffff7719467 in rt::task::__extensions__::build_start_wrapper::anon::expr_fn::a2 ()
   from /usr/cn/rust/debug-build/rust/x86_64-unknown-linux-gnu/stage2/bin/../lib/libstd-6c65cf4b443341b1-0.8-pre.so
#25 0x00007ffff77b0db5 in rt::context::Context::new::task_start_wrapper::__rust_abi::se ()
   from /usr/cn/rust/debug-build/rust/x86_64-unknown-linux-gnu/stage2/bin/../lib/libstd-6c65cf4b443341b1-0.8-pre.so
#26 0x00007ffff77b0d67 in rt::context::Context::new::task_start_wrapper::h1b9fdc38dc3bcfa4sea8::v0.8$x2dpre ()
   from /usr/cn/rust/debug-build/rust/x86_64-unknown-linux-gnu/stage2/bin/../lib/libstd-6c65cf4b443341b1-0.8-pre.so
#27 0x0000000000000000 in ?? ()
(gdb)
@brson
Copy link
Contributor

brson commented Sep 21, 2013

It's intentional. By default the runtime doesn't put any tasks on the main thread. Using the unstable runtime API it would not be hard to construct a runtime that ran tasks only on the main thread, though you will still run into situations where, e.g. the I/O subsystem starts other threads on its own.

The code for initializing the runtime is https://github.com/mozilla/rust/blob/master/src/libstd/rt/mod.rs#L234

With some tweaking this could be adjusted to only put tasks on the main thread: the start_on_main_thread function already does put a task on the main thread, but it also spins up other threads.

The way to invoke the runtime API directly is by using the #[start] attribute to create a runtimeless application entry point. See this test case for an example: https://github.com/mozilla/rust/blob/master/src/test/run-pass/rt-start-main-thread.rs

@glycerine
Copy link
Author

I didn't understand this was intentional when I wrote up this ticket. (From #rust irc I got the impression that RUST_THREADS=1 was supposed to keep threads down to 1.)

Nonetheless, for my use case where I can't really tolerate more than one thread (using fork(2) calls to get inexpensive + comprehensive transaction semantics for rusti), it would be really great to have a global policy flag somewhere in Rust that is easily settable, that can be easily checked by all programmers, and that can be expected to be honored everywhere. What is desired is some easy way to tell the runtime (and any implementors implementing new, additional features), "Yeah, really, don't spawn any threads, thank you very much; if a background thread is holding a mutex for malloc, this will really hose us if we happen to fork at that moment." Under this one-thread regime, multiple coroutines are okay, but they should be cooperatively scheduled on the single main thread of the application.

Does this seem viable?

It also seems desirable if you are writing embedded systems that do not offer OS level threads.

@glycerine
Copy link
Author

I tried using start_on_main_thread, and Rust still spawns an additional thread. (RUST_THREADS=1 is set).
Example:

// we want a single thread... so we can fork.

use std::libc::size_t;
use std::libc::sleep;

#[link_args = "-lsnappy"]
extern {
    fn snappy_max_compressed_length(source_length: size_t) -> size_t;
}

#[fixed_stack_segment]
fn single_threaded_main() {
    // some computation here...                                                                                                         
    unsafe {
        std::libc::sleep(5);
        snappy_max_compressed_length(100);
        std::libc::sleep(5);
    };
}

#[start]
#[fixed_stack_segment]
fn start(argc: int, argv: **u8, crate_map: *u8) -> int {
    std::rt::start_on_main_thread(argc, argv, crate_map, single_threaded_main)
}

@thestinger
Copy link
Contributor

This isn't fixable, because threads are needed to make large writes non-blocking. See #9568 for a feasible but unlikely solution.

Jarcho pushed a commit to Jarcho/rust that referenced this issue Aug 31, 2022
Initial implementation `result_large_err`

This is a shot at rust-lang#6560, rust-lang#4652, and rust-lang#3884. The lint checks for `Result` being returned from functions/methods where the `Err` variant is larger than a configurable threshold (the default of which is 128 bytes). There has been some discussion around this, which I'll try to quickly summarize:

* A large `Err`-variant may force an equally large `Result` if `Err` is actually bigger than `Ok`.
* There is a cost involved in large `Result`, as LLVM may choose to `memcpy` them around above a certain size.
* We usually expect the `Err` variant to be seldomly used, but pay the cost every time.
* `Result` returned from library code has a high chance of bubbling up the call stack, getting stuffed into `MyLibError { IoError(std::io::Error), ParseError(parselib::Error), ...}`, exacerbating the problem.

This PR deliberately does not take into account comparing the `Ok` to the `Err` variant (e.g. a ratio, or one being larger than the other). Rather we choose an absolute threshold for `Err`'s size, above which we warn. The reason for this is that `Err`s probably get `map_err`'ed further up the call stack, and we can't draw conclusions from the ratio at the point where the `Result` is returned. A relative threshold would also be less predictable, while not accounting for the cost of LLVM being forced to generate less efficient code if the `Err`-variant is _large_ in absolute terms.

We lint private functions as well as public functions, as the perf-cost applies to in-crate code as well.

In order to account for type-parameters, I conjured up `fn approx_ty_size`. The function relies on `LateContext::layout_of` to compute the actual size, and in case of failure (e.g. due to generics) tries to come up with an "at least size". In the latter case, the size of obviously wrong, but the inspected size certainly can't be smaller than that. Please give the approach a heavy dose of review, as I'm not actually familiar with the type-system at all (read: I have no idea what I'm doing).

The approach does, however flimsy it is, allow us to successfully lint situations like

```rust
pub union UnionError<T: Copy> {
    _maybe: T,
    _or_perhaps_even: (T, [u8; 512]),
}

// We know `UnionError<T>` will be at least 512 bytes, no matter what `T` is
pub fn param_large_union<T: Copy>() -> Result<(), UnionError<T>> {
    Ok(())
}
```

I've given some refactoring to `functions/result_unit_err.rs` to re-use some bits. This is also the groundwork for rust-lang#6409

The default threshold is 128 because of rust-lang/rust-clippy#4652 (comment)

`lintcheck` does not trigger this lint for a threshold of 128. It does warn for 64, though.

The suggestion currently is the following, which is just a placeholder for discussion to be had. I did have the computed size in a `span_label`. However, that might cause both ui-tests here and lints elsewhere to become flaky wrt to their output (as the size is platform dependent).

```
error: the `Err`-variant returned via this `Result` is very large
  --> $DIR/result_large_err.rs:36:34
   |
LL | pub fn param_large_error<R>() -> Result<(), (u128, R, FullyDefinedLargeError)> {
   |                                  ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ The `Err` variant is unusually large, at least 128 bytes
```

changelog: Add [`result_large_err`] lint
flip1995 pushed a commit to flip1995/rust that referenced this issue Sep 9, 2022
Use `approx_ty_size` for `large_enum_variant`

This builds upon rust-lang#9373 to use the approximate size of each variant for `large_enum_variant`. This allows us to lint in situations where an `enum` contains generics but is still guaranteed to have a large variant on an at-least basis, e.g. with `(T, [u8; 512])`.

* I've changed the wording from "is ... bytes" to "contains at least" because
  * the size is now an approximate lower bound (e.g. `512` in the example above). The actual size is larger due to `T`, including due to `T`'s memory layout.
  * the discriminant is not taken into account in the message. This comes up with variants like `A(T)`, which are "is at least 0 bytes" otherwise, which may be misleading.
* If the second-largest variant has no fields, there is a special case "carries no data" instead of "is at least 0 bytes".
* A variant like `A(T)` is "at least 0 bytes", which is technically true, yet we don't distinguish between "indeterminate" and truly "ZST".
* The generics-tests that were there before now lint while they didn't lint before. AFAICS this is correct.

I guess the above is correct-ish. However, I use the `SubstsRef` that I got via `cx.tcx.type_of(item.def_id)` to solve for generics in the variants. Is this even applicable, since we start from an - [ ] `ItemKind`?

changelog: none
flip1995 pushed a commit to flip1995/rust that referenced this issue Apr 23, 2023
Add size-parameter to unecessary_box_returns

Fixes rust-lang#10641

This adds a configuration-knob to the `unecessary_box_returns`-lint which allows _not_ linting a `fn() -> Box<T>` if `T` is "large". The default byte size above which we no longer lint is 128 bytes (due to rust-lang/rust-clippy#4652 (comment), also used in rust-lang#9373). The overall rational is given in rust-lang#10641.

---

changelog: Enhancement: [`unnecessary_box_returns`]: Added new lint configuration `unnecessary-box-size` to set the maximum size of `T` in `Box<T>` to be linted
[rust-lang#10651](rust-lang/rust-clippy#10651)
<!-- changelog_checked -->
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

No branches or pull requests

3 participants