-
Notifications
You must be signed in to change notification settings - Fork 0
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 RFC #2
base: main
Are you sure you want to change the base?
Conversation
RUST_RFC.md
Outdated
|
||
As stated in the previous section, many of the interceptors "just work". Often, the Rust runtime will call into the libc runtime to allocate memory (`malloc` et al), interface with the networking stack (`socket` et al) or do I/O (`read`, `write`, `open`, `close` et al). | ||
|
||
One place where this is not the case is `Mutex::lock`. Locks are disallowed in real-time contexts but because `Mutex::lock` begins as a spin-lock, the RTsan runtime cannot automatically detect its usage. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's just say because Mutex::lock does not call the pthread functions
or double check that it in fact can be called spin lock.
RUST_RFC.md
Outdated
``` | ||
|
||
### nosanitize | ||
Another approach we could take is similar to the ASan and TSan `nosanitize` attribute. We advocate for the scoped disabler macro, as it allows users to specify a more specific scope to disable the tool in. This means users will not have to extract unsafe code into helper functions to disable them at the function level. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It is called no_sanitize
and we should add it there as well because currently you can specify the precise sanitizer you want to disable: no_sanitize(address)
. Would be nice to add no_sanitize(realtime)
there.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
also see the PR: rust-lang/rust#68164
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I put in a sentence in this next upcoming draft requesting feedback which path to take. The LLVM folks were pretty convinced NOT to add nosanitize, so we will have to see if Rust wants it if we can go back and add the proper attrs
RUST_RFC.md
Outdated
* **Are attributes inheritable on interfaces?** | ||
* **do we make it a requirement of all lock types?** | ||
* **What other details can we provide here??** | ||
* **Are there other obvious ones we want to add this attribute to?** |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Definitely std::sync::RwLock::lock
as it is similar to a Mutex only allowing multiple readers at the same time.
We should check with try_lock if it is hitting anything already when unlocking.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We could also think about the atomic functions that not do a direct atomic operation but trigger a CAS-loop. As far as I know CAS loops have unbounded execution time or am I wrong? I can send you the part where Jon is explaining when the Rust atomics have CAS-loops instead of a direct atomic operation.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
My guess is, that most of the other stuff like file io, network stuff and so on will trigger something in libc.
Maybe a bit out of scope for now, but one unknown I have in my mind is async fn and .await, that should probably not be allowed, but I don't even know if it is possible to somehow call async functions from your process block, as you need an async runtime to do that. Maybe when adding async to a fn it could automatically be marked blocking, just in case someone somehow manages to do that.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok, I will add RwLock, good idea.
CAS loops are tricky - I'm not sure I'd advocate for adding those right now. I think you're right in them being unbounded but I think we should be 100% convinced before putting it in. I know at least using >1 producer or >1 consumer necessitates a loop and is technically unbounded. I am not sure about 1 producer and 1 consumer - is that always unbounded as well because one thread can "spam"?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
My guess is, that most of the other stuff like file io, network stuff and so on will trigger something in libc.
Yes this is my guess too - if you have a little time would you give this a try in the example project and report back? Things like writing to stdout, opening a file, connecting to a socket should all automatically fail.
Maybe a bit out of scope for now, but one unknown I have in my mind is async fn and .await, that should probably not be allowed
Good idea, I think this may be opening up a big can of worms so we may consider leaving it for a v2
RUST_RFC.md
Outdated
|
||
1. RealtimeSanitizer can be enabled in unstable mode - like the other sanitizers | ||
2. The introduction of `nonblocking` (marking a function as real-time constrained) and `blocking` (marking a function as inappropriate for use in a `nonblocking` context) | ||
3. The addition of the `blocking` attribute to `Mutex::lock` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
maybe say directly std::sync::Mutex::lock
to be precise?
@steckes I just put up a new version attempting to address each of your comments, please give it another look when you get a chance. Thanks for the feedback so far |
rtsan_scoped_disabler!( | ||
let audio = vec![1.0; 256]; // report is suppressed | ||
) | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this is a reminder for me, to check if this is possible. Currently it is
rtsan_scoped_disabler!({
let audio = vec![1.0; 256];
});
with curly braces and closing semicolon.
But it does not really matter for the proposal.
RUST_RFC.md
Outdated
|
||
> In aerospace guidance systems if your software doesn't update on a regular tick your simulation of what is happening may diverge from reality. Unfortunately this may also mean your rocket converges with the ground. | ||
|
||
Code in these environments must run in a deterministic amount of time. Allocations, locks, and other OS resource access are disallowed because they don't have bound upper execution time. Unbound execution time leads to blown deadlines, and blown deadlines leads to consequences. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Minor: I think the point in the final sentence in this paragraph has already been made, so could be removed here
RUST_RFC.md
Outdated
|
||
As stated in the previous section, many of the interceptors "just work". Often, the Rust runtime will call into the libc runtime to allocate memory (`malloc` et al), interface with the networking stack (`socket` et al) or do I/O (`read`, `write`, `open`, `close` et al). | ||
|
||
A couple places where this is not the case is `std::sync::Mutex::lock` and `std::sync::RwLock::lock`. Locks are disallowed in real-time contexts but because these methods do not call in to `pthread_mutex_lock` initially, the RTsan runtime cannot automatically detect its usage. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
On "Locks are disallowed in real-time contexts"...
This is difficult to say definitively. Consider two use cases:
- the spin mutex is locked by both non-real-time and real-time threads, vs.
- the spin mutex is locked only by real-time threads, which never try to re-lock it immediately
In the case of 1., we can get the classic priority inversion if the real-time thread tries to acquire while the non-real-time thread has the lock. But in the case of 2., there are situations where all real-time threads can make forward progress (e.g. popping jobs off a thread pool queue).
We kind of get out of jail for free in the C++ spin mutex case, because adding the attribute [[clang::nonblocking]]
is a user decision. But if we were to blanket apply it to std::sync::Mutex::lock
, then we might get into trouble with conflicting user requirements. I'm not sure what to do here - let's discuss it further soon?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, looks good :)
No description provided.