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

[Question][HW4] How can I ensure that a struct is not dropped until a function pointed by a field of the struct completes? #905

Closed
bongjunj opened this issue Mar 31, 2024 · 8 comments
Assignees
Labels
homework - boc boc.rs question Further information is requested

Comments

@bongjunj
Copy link

bongjunj commented Mar 31, 2024

This question is related to the function Behavior::resolve_one.

unsafe fn resolve_one(this: *const Self) { ... }

The function is responsible for the followings (as the paper says):

  1. atomically decrement the count of requests which are not scheduled yet,
  2. spawn a thread that executes the thunk of the behavior,
  3. and releases cowns to the next behavior

At step (2), the function should call the function (thunk) behind the pointer to the behavior (*const Behavior).
But as the type *const Behavior does not implement Send, it is not possible to call the function in another thread.
Also, as the function pointer type (dyn FnOnce() + Send + 'static) does not implement Sync, it is not possible to share the function between threads.

Therefore, we need a way to circumvent this constraints. One possible way is to use std::ptr::read, but it has a problem where that the spawning thread can drop Behavior before the thunk completes.

Q: is there a way to overcome the constraints while I can ensure that the function is not destroyed before running?

@bongjunj bongjunj added the question Further information is requested label Mar 31, 2024
@rgbygv
Copy link

rgbygv commented Mar 31, 2024

I have the same problem.
If we want to use rayon::spawm(move ||) in unsafe fn resolve_one(this: *const Self) { ... },
The compiler will complain that trait Send is not implemented for *const Behavior

@m-spitfire
Copy link

m-spitfire commented Mar 31, 2024

What I did to make the compiler happy is make a Box out of this and move that box into spawned thread. But I'm not sure it's the correct solution, since when I run my solution I just get one of process didn't exit successfully: (signal: 6, SIGABRT: process abort signal), (signal: 11, SIGSEGV: invalid memory reference), (signal: 10, SIGBUS: access to undefined memory)

@m-spitfire
Copy link

Apparently what I'm doing is really wrong. Running cargo miri points out why

test boc::boc_vec ... error: Undefined Behavior: trying to retag from <195353> for Unique permission at alloc67683[0x0], but that tag only grants SharedReadOnly permission for this location
    --> /Users/muradb/.rustup/toolchains/nightly-2024-03-13-aarch64-apple-darwin/lib/rustlib/src/rust/library/alloc/src/boxed.rs:1009:9
     |
1009 |         Box(unsafe { Unique::new_unchecked(raw) }, alloc)
     |         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
     |         |
     |         trying to retag from <195353> for Unique permission at alloc67683[0x0], but that tag only grants SharedReadOnly permission for this location
     |         this error occurs as part of retag at alloc67683[0x0..0x30]
     |
     = help: this indicates a potential bug in the program: it performed an invalid operation, but the Stacked Borrows rules it violated are still experimental
     = help: see https://github.com/rust-lang/unsafe-code-guidelines/blob/master/wip/stacked-borrows.md for further information
help: <195353> was created by a SharedReadOnly retag at offsets [0x0..0x28]
    --> src/boc.rs:213:40
     |
213  |         unsafe { Behavior::resolve_one(&self) };
     |                                        ^^^^^
     = note: BACKTRACE (of the first span) on thread `boc::boc_vec`:
note: inside `boc::Behavior::resolve_one`
    --> src/boc.rs:229:33
     |
229  |         let behavior = unsafe { Box::from_raw(this as *mut Self) };
     |                                 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

@ICubE-
Copy link

ICubE- commented Mar 31, 2024

I used Box::leak to prevent drop until I retake the ownership and spawn new thread.

@Lee-Janggun
Copy link
Member

The recommended way is to use Box::from_raw(this.cast_mut()) to obtain the behavior do more stuff. However, when doing this, you should make sure that the current thread is the actually the only one that can access the behavior.

@m-spitfire
Copy link

But when I do that behavior might be dropped early since the function resolve_one doesn't take the ownership of it.

@kingdoctor123
Copy link
Member

Since resolve_one does not take the ownership of a behavior, it is your obligation to reason about lifetime of the behavior. This manual inspection of lifetime is needed before/during execution of resolve_one, so resolve_one is marked as unsafe and you might need some unsafe blocks within the function.

Recall that schedule() function of Behavior takes full ownership of the behavior. Since it is the first place where a behavior becomes visible to other threads, you can view it as a starting point of manual analysis of lifetime of behaviors.

@bongjunj
Copy link
Author

bongjunj commented Apr 4, 2024

Got it. To sum up, as Behavior::schedule() consumes itself, it is the first place to reason the lifetime of Behavior object. The lifetime of the object is managed by functions called in the schedule functions. And we can manage the lifetime using methods implemented for Box struct.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
homework - boc boc.rs question Further information is requested
Projects
None yet
Development

No branches or pull requests

6 participants