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

Suggestion: lock() for an entire context #545

Open
birkenfeld opened this issue Oct 16, 2021 · 10 comments
Open

Suggestion: lock() for an entire context #545

birkenfeld opened this issue Oct 16, 2021 · 10 comments

Comments

@birkenfeld
Copy link

The lock method on tuples is a great idea for getting multiple resources locked.

However, sometimes I find that I need to lock all resources in a cx.shared and was wondering if it could also provide a lock method that returns a struct with the same field names, but all field tpes are &mut T.

Not sure if resources that aren't lockable should be present in that struct or not.

@perlindgren
Copy link
Collaborator

Yes, that should be doable, and quite useful. Thanks for the suggestion we will look into it.

@birkenfeld
Copy link
Author

Nice! I'm really loving the 0.6 changes, btw, it's a big improvement in ergonomics.

@perlindgren
Copy link
Collaborator

I'm looking at a possible implementation. It looks feasible, a struct needs to be created and passed to the closure containing &mut R references. This struct should be declared locally in the generated module (e.g. under the name Shared). I'm not sure the name needs to be visible to the end user if just using the . notation but if one wants to destruct the passed resource struct it needs to.

@perlindgren
Copy link
Collaborator

perlindgren commented Oct 17, 2021

There is a crux still to be solved. For soundness, RTIC ensures that you cannot get multiple (mutable) references by locking a field of the shared resources twice. The way its implemented is zero-cost, where the check is done by the compiler at compile time. The resource proxy is consumed by the lock. I'm playing around with this now, if lock is implemented for the "parenting" structure then we should be able to have this working. Some details regarding passing of cfgs needs to be ironed out as well.

@perlindgren
Copy link
Collaborator

I put together a pre-rfc. rtic-rs/rfcs#53

@perlindgren
Copy link
Collaborator

There is now a corresponding POC:

https://github.com/rtic-rs/cortex-m-rtic/tree/lockall

There is still an outstanding issue regarding the resource proxy for the lock-all API. It currently exposes the priority (Priority type). At some point, we deemed that to be risky, I don't recall now exactly how it can potentially be exploited. In any case that problem should be solvable if we decide to include the lockall API in the RTIC 0.6 release.
Please go ahead an try it. The examples/lockall*.rs should give you an idea how it can be used.

@perlindgren
Copy link
Collaborator

Small fix, the Priority is no longer leaked.
https://github.com/rtic-rs/cortex-m-rtic/tree/lockall

@birkenfeld
Copy link
Author

I've tried the current branch and it works fine for me. Thanks!

@eflukx
Copy link

eflukx commented Sep 26, 2022

Is there a plan to merge this code?

@korken89
Copy link
Collaborator

@eflukx In its current state, no. The code is UB so a redesign is needed if it were to be merged.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants