Stream: platform development

Topic: thread-safety in glue

Brian Hicks (Oct 05 2022 at 18:59):

Currently, glue exports things like NonNull<ManuallyDrop<u8>>, which can't be sent between threads (NonNull does not implement Send.) Is this intentional / a long-term design? I assume it'd make doing things like web servers pretty annoying. :dizzy:

(I'm running into it when parallelizing work in rbt. I can get around it for now by having all the glue-y jobs live in the main thread and then converting them to send-safe stuff before shoving 'em in queues.)

Folkert de Vries (Oct 05 2022 at 19:06):

in what sort of scenario does this come up?

Folkert de Vries (Oct 05 2022 at 19:06):

specifically, what types are involved?

Brian Hicks (Oct 05 2022 at 20:05):

well in this particular case, it looks like it's in a RocStr! Here's the causal chain that rustc generated:

error[E0277]: `NonNull<ManuallyDrop<u8>>` cannot be shared between threads safely
   --> src/coordinator.rs:295:18
    |
295 |                 .spawn(move || self.run())
    |                  ^^^^^ `NonNull<ManuallyDrop<u8>>` cannot be shared between threads safely
    |
    = help: within `glue::Command`, the trait `Sync` is not implemented for `NonNull<ManuallyDrop<u8>>`
    = note: required because it appears within the type `std::option::Option<NonNull<ManuallyDrop<u8>>>`
    = note: required because it appears within the type `RocList<u8>`
    = note: required because it appears within the type `ManuallyDrop<RocList<u8>>`
    = note: required because it appears within the type `roc_std::roc_str::RocStrInner`
    = note: required because it appears within the type `RocStr`

Folkert de Vries (Oct 05 2022 at 20:07):

well for that specific case we can just implement unsafe impl Send for RocStr

Brendan Hansknecht (Oct 05 2022 at 20:08):

So Roc types are not thread safe by default. So this is probably correct actually. You can always wrap the type and make it send if you want to override that.

Brendan Hansknecht (Oct 05 2022 at 20:09):

If you we to take a RocStr and pass it into two different Roc functions on different threads that could lead to a double free.

Brendan Hansknecht (Oct 05 2022 at 20:10):

We technically need glue to generate different types based on if roc is configured to run with atomic refcounts or not. When using atomic refcounts, they are now send.

Brendan Hansknecht (Oct 05 2022 at 20:11):

This is all super long term stuff, but the platform should be able to request whether or not roc needs to be thread safe

Brendan Hansknecht (Oct 05 2022 at 20:12):

In a web platform, Roc probably doesn't need to be thread safe. Each thread is calling roc functions in a single threaded way. No communication between the threads.

Folkert de Vries (Oct 05 2022 at 20:13):

hmm then how does this work ? https://doc.rust-lang.org/std/vec/struct.Vec.html#impl-Send-for-Vec%3CT%2C%20A%3E

Folkert de Vries (Oct 05 2022 at 20:14):

send should just work: we can transfer our types between threads

Folkert de Vries (Oct 05 2022 at 20:15):

and Sync seems to say that &RocStr can be shared between threads, which also seems fine? dropping that type does nothing

Brendan Hansknecht (Oct 05 2022 at 20:17):

Technically, as they currently stand, they should be Sync and Send, but only if the refcount is 1.

Brendan Hansknecht (Oct 05 2022 at 20:17):

Rust doesn't have conditional Sync/Send, so you have to label them as neither.

Brendan Hansknecht (Oct 05 2022 at 20:17):

If you send one with multiple references, you can get race conditions between the two threads.

Brian Hicks (Oct 05 2022 at 20:18):

hmm… looks like I'm going to have to do something like this to get any of this to work (or I'll have to convert all my RocStr to String)

Brian Hicks (Oct 05 2022 at 20:19):

thought it'd be fine but it looks like channels are Send iff their items are Send. Makes sense.

Brendan Hansknecht (Oct 05 2022 at 20:20):

If you know you will have the only reference, the best answer is to basically make a sendable wrapper type and use that. It isn't too much work.

Brian Hicks (Oct 05 2022 at 20:21):

yeah, Job already wraps a bunch of stuff whose memory is managed by Roc. unsafe impl send for Job oughta do me. Sigh. Would've liked to avoid it, but I'm at least gonna document it thoroughly.

Brendan Hansknecht (Oct 05 2022 at 20:22):

If you really want to be thorough (which maybe isn't worth it), you would make struct UniqueRocStr(RocStr) That is sendable and ensures either the string is unique or that it gets copied. Maybe we could make glue generate that. Just a check in the constructor.

Brian Hicks (Oct 05 2022 at 20:23):

how would that work? Just look at the refcount in the internals and copy if it's greater than 1?

Brendan Hansknecht (Oct 05 2022 at 20:23):

yep

Brian Hicks (Oct 05 2022 at 20:23):

nice.

Brian Hicks (Oct 05 2022 at 20:24):

not gonna do that right now, but I'll note that it should happen in the future. Currently Job just wraps over a couple glue-generated types wholesale. It's a TODO to eventually not do that, though.

Brendan Hansknecht (Oct 05 2022 at 20:24):

:+1:

Richard Feldman (Oct 05 2022 at 21:21):

roc collections (including string) aren't thread-safe by default because they're non-atomically reference counted

Richard Feldman (Oct 05 2022 at 21:22):

so RocList<T> is not comparable to Vec<T>, but rather to Rc<Vec<T>>

Richard Feldman (Oct 05 2022 at 21:22):

and Rc is neither Send nor Sync

Brendan Hansknecht (Oct 05 2022 at 21:34):

What do you think about glue generating a UniqueRocList<T> type that would have a constructor that took a RocList<T> and cloned it if it was not unique and took ownership if it is unique. Then UniqueRocList<T> should be Send but not Sync.

Richard Feldman (Oct 05 2022 at 21:35):

that seems reasonable :thumbs_up:

Brian Hicks (Oct 05 2022 at 21:57):

well, sigh, looks like I'm just gonna have to do this. How do I get the reference count?

Brian Hicks (Oct 05 2022 at 21:59):

or I guess I could just copy all the memory proactively. I don't love that but it'd get me unstuck here (been spinning my wheels all afternoon.) What would y'all think of that?

Richard Feldman (Oct 05 2022 at 22:02):

@Brendan Hansknecht do you think you could help with adding an implementation of UniqueRocList to roc_std? I'm still on parental leave, so my free time is spotty and unpredictable :sweat_smile:

Brian Hicks (Oct 05 2022 at 22:22):

unblocked for now… I was already converting most of the Job into things like PathBufs and stuff, so I just needed to convert a list, a dict, and a str in addition.

Brian Hicks (Oct 05 2022 at 22:22):

maddeningly, the code is actually now better-factored and easier to work with :mad:

Brian Hicks (Oct 05 2022 at 22:23):

fortunately if we had like RocList.unique or something it'd be just a drop-in, I think?

Brian Hicks (Oct 05 2022 at 22:26):

ok, that's enough concurrency shenanigans for today. I am going to go lie down now.

Brendan Hansknecht (Oct 05 2022 at 22:29):

do you think you could help with adding an implementation of UniqueRocList to roc_std?

Yeah, I can take a look.

Brendan Hansknecht (Oct 05 2022 at 22:30):

@Brian Hicks Feel free to let me know if you have any other questions/ need any specific information related to this.

Brian Hicks (Oct 05 2022 at 22:33):

I might when my brain recovers from the day. :dizzy: For now, lemme know if it ends up being easy and I'll drop it in and test it for you as soon as I can!

Last updated: Jul 06 2025 at 12:14 UTC