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

libtock_platform static asynchronous Read-Only Allow API Design #334

Closed
jrvanwhy opened this issue Sep 17, 2021 · 4 comments
Closed

libtock_platform static asynchronous Read-Only Allow API Design #334

jrvanwhy opened this issue Sep 17, 2021 · 4 comments
Assignees

Comments

@jrvanwhy
Copy link
Collaborator

I'm having some trouble designing a sound asynchronous API for the Read-Only Allow system call, and could use some help.

Here are what I believe to be the requirements for the API:

  1. Must have a safe API that is sound (i.e. prevents UB)
  2. Must allow for asynchronous operation. This means the Read-Only Allow function must be able to return while the kernel still has access to the buffer.
  3. Must be able to coexist with synchronous APIs (i.e. ones that take non-'static buffers).
  4. Must support buffers in read-only memory
  5. Must support buffers in read-write memory, which are written when they are not shared with the kernel (i.e. app code writes a buffer, then shares it with the kernel via Read-Only Allow).

The obvious API is (omitting driver number and buffer number for simplicity):

fn allow_ro_static(new_buffer: &'static [u8]) -> &'static [u8];

However, this fails either requirement 1 or requirement 5. There is no way to write a RefCell-like type that provides access to a buffer with 'static lifetime that provides a safe, sound API. You can write something like TakeCell with the following API:

struct AllowCell<const LEN: usize> {
    borrowed: Cell<bool>,
    buffer: [u8; LEN]
}

impl AllowCell {
    fn get_ref(&self) -> Option<&'static [u8]>;  // None if this AllowCell is borrowed
    fn return_ref(&self, reference: &'static [u8]);  // Un-borrows this AllowCell by returning the reference.
    
    fn get_mut(&self) -> Option<&'static mut [u8]>;  // None if this AllowCell is borrowed
    fn return_mut(&self, reference: &'static mut [u8]);  // Un-borrows this AllowCell by returning the reference
}

except this has two problems:

  1. It is unsound, because safe code can copy a &'static [u8] and call return_ref on one copy while holding the other (leading to UB when return_mut is called).
  2. It doesn't allow a subslice of the buffer to be shared via Allow, as return_ref would need to fail if the reference doesn't match the entire buffer.

Problem 1 can be solved in an awkward manner by introducing a new type that wraps a &'static [u8] but is not copyable, but I'm having trouble solving problem 2.

The best API I've thought of so far (and I need to think about it more to be sure it is sound) is:

fn allow_ro_static(new_buffer: &'static [u8]);  // For buffers in read-only memory
fn allow_ro_static_cell(new_buffer: &'static [Cell<u8>]);  // For buffers in read-write memory
fn unallow();

Note that this API does not return the previous buffer! Instead, code should call unallow before re-using a mutable buffer. The biggest drawback, though, is the ergonomics of using [Cell<u8>], which I've been trying to avoid since Tock 2.0 started...

Does anyone have a better idea?

@jettr
Copy link
Contributor

jettr commented Sep 17, 2021

Do we have to have 'static lifetime always? Could we expose a lease like interface:

fn lease_ro_buffer<'a>(buffer: &'a [u8]) -> ReadOnlyLease<'a> {
    // allow with kernel and create Lease object
}

impl Drop on ReadOnlyLease<'_> {
    fn drop(&self) {
        // unallow from kernel
    }
}

@jrvanwhy
Copy link
Collaborator Author

Do we have to have 'static lifetime always? Could we expose a lease like interface:

fn lease_ro_buffer<'a>(buffer: &'a [u8]) -> ReadOnlyLease<'a> {
    // allow with kernel and create Lease object
}

impl Drop on ReadOnlyLease<'_> {
    fn drop(&self) {
        // unallow from kernel
    }
}

I don't see how we can avoid requiring a 'static lifetime for the asynchronous interface. The event loop will likely be in main():

fn main() {
    initialize_things();
    start_things();
    loop {
        Syscalls::yield_wait();
    }
}

After each lease_ro_buffer call, the stack will unwind back up to main() while the buffer is still shared with the kernel. That means that every lifetime attached to a stack frame below main() will end, and the memory may be re-used for something else. So I think the interface will require a 'static buffer.

@jrvanwhy jrvanwhy self-assigned this Sep 17, 2021
@jrvanwhy
Copy link
Collaborator Author

There are multiple ways to solve this problem. We could build the API on &[Cell<u8>], or we could write a wrapper around &'static [u8] that is non-copyable and can be subsliced. These have different tradeoffs, both in terms of maintainability and in terms of program size (code + RAM usage).

Until now, I haven't run into any such tradeoffs for supporting highly-asynchronous apps in libtock-rs. Making these tradeoffs correctly requires knowing how the APIs will be used, which is almost impossible unless we have highly-asynchronous apps.

However, there has yet to be a highly-asynchronous app built on top of libtock-rs. OpenSK, Ti50, and Manticore are all synchronous codebases. They use some local asynchrony in the form of waiting for multiple events (mostly a primary event + a timeout), but they never jump from one task into an unrelated task. It's very possible that every app that runs two independent tasks in parallel will put them into different processes.

What that means is that I don't have the data I need to design this API correctly. I certainly don't want to put a lot of work into a highly-asynchronous API if it will never be used. At this point, I think I will focus on APIs that look synchronous but support some mild asynchrony (to cover the "wait for an event with some form of cancellation" use case).

For now, I will leave the libtock_platform and libtock_unittest modules that support highly-asynchronous apps in place, so that we can revive these APIs as needed.

@jrvanwhy jrvanwhy changed the title libtock_platform asynchronous Read-Only Allow API Design libtock_platform static asynchronous Read-Only Allow API Design Oct 19, 2021
@jrvanwhy
Copy link
Collaborator Author

Closing this based on my explanation at #334 (comment). We'll look at supporting a higher degree of asynchrony if a use case appears that needs it. The APIs I designed at #341 and have merged into libtock-rs are sufficient for all currently-known use cases of libtock-rs.

bors bot added a commit that referenced this issue Jan 4, 2022
353: Remove `libtock_platform`'s asynchronous API traits. r=jrvanwhy a=jrvanwhy

These traits were designed to support a style of code I was calling "`'static` asynchrony", where upcalls are not tied to lifetimes and execution can potentially jump between unexpected modules. However [we never implemented `'static` asynchrony](#334), and I'm not sure it will ever be implemented.

Because I don't expect these traits to ever be used, I think they should be removed from `libtock_platform`.

Co-authored-by: Johnathan Van Why <[email protected]>
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

2 participants