JamesMunns[m] has quit [Quit: Idle timeout reached: 172800s]
M9names[m] has quit [Quit: Idle timeout reached: 172800s]
dinkelhacker_ has joined #rust-embedded
Beregond[m] has joined #rust-embedded
<Beregond[m]>
Hey, got question regarding no_std and config files - what are you using/would you use as config file format? I see both toml and yaml libs are std only 🫠
mameluc[m] has joined #rust-embedded
<mameluc[m]>
Beregond[m]: What kind of config files? serde works in no_std
<Beregond[m]>
human-written, I could raw parse .ini but looking for something more robust
<Beregond[m]>
serde-* looks promissing - will check that, thanks all 🚀
ryan-summers[m] has joined #rust-embedded
<ryan-summers[m]>
As mentioned above, serde-json-core is a no-std serde deserializer/serializer that handles structured JSON. You can't do dynamic maps or structures, but it covers a vast majority of embedded use cases
<diondokter[m]>
Why do you need a config *file* btw?
<diondokter[m]>
Are you taking it in from a SD card or something?
<Beregond[m]>
diondokter[m]: > <@diondokter:matrix.org> Why do you need a config *file* btw?
<Beregond[m]>
> Are you taking it in from a SD card or something?
<Beregond[m]>
yes, that is the intention - to load it from sd card
<diondokter[m]>
Alright! Seems reasonable.
<diondokter[m]>
If it was something else I'd recommend not using files for it to simplify your life
<Beregond[m]>
Yeah, I suspect it wont be easy ride 😅 but no other simpler method if you want to be able to configure anything
<mameluc[m]>
This was why I went with postcard on the device side as it is pretty light weight. Ofc you need to have something to write your configuration to the postcard format AND you need to keep track of you config structs but for me it was more important to keep the binary and the payloads small
<mameluc[m]>
diondokter[m]: > I don't think you'll want to parse toml on low resource systems where you can't have a decent heap as the deserialize function for my config type takes up a lot of flash.
starblue1 has quit [Ping timeout: 252 seconds]
starblue1 has joined #rust-embedded
<Beregond[m]>
<diondokter[m]> "A long long time ago I tried..." <- Oh, nice! too bad it wasn't accepted :/
<jonored[m]>
<Beregond[m]> "Yeah, I suspect it wont be..." <- Arguably there's the "binary settings in a flash region accessed with something like sequential_storage" approach, which is probably simpler on the device side.
<i509vcb[m]>
Unfortunately even 1600p isn't enough to see more than like 10 characters of each
<i509vcb[m]>
TI does have an interesting way of getting around the M0's 32 interrupt limit by just grouping interrupts together and providing a register to determine which specific thing was interrupted
<jonored[m]>
you can write async without any particular support from the hal, but it's more work rather than less and only worth it in rarer cases.
<Henk[m]>
Most sync HALs will block the executor while reading from peripherals, which is most of the time not what you want. But yeah. it's possible
bartmassey[m] has joined #rust-embedded
<bartmassey[m]>
(How does one message the whole channel here again?)
<bartmassey[m]>
We'll be starting the Weekly Meeting in a few minutes. Agenda is https://github.com/rust-embedded/wg/discussions/794 . I'm having some technical Matrix difficulties at the moment: if someone else could run the meeting it would be appreciated
<Henk[m]>
@channel
<bartmassey[m]>
@channel Weekly Meeting shortly — see above
<i509vcb[m]>
iirc it was room?
<bartmassey[m]>
@room Weekly Meeting shortly see above
<alex[m]123>
Alright, I'll give embassy-stm32 a whirl, thank you!
sourcebox[m] has joined #rust-embedded
<sourcebox[m]>
Matrix is lagging here also.
<Henk[m]>
I guess only admins can do @room?
<bartmassey[m]>
Ah. Oh well.
rmsyn[m] has joined #rust-embedded
<rmsyn[m]>
:wave:
<bartmassey[m]>
Alright, let's go. Who has announcements? (Please add to the meeting notes as well…)
<Henk[m]>
🙌 Next book sprint for the Discovery MB2 book will be on October 26, 17:00-21:00 CEST, 15:00-19:00 UTC, 8:00AM-12:00PM PST. Join if you're interested in helping to write the new discovery book!
<Henk[m]>
(I might have messed up the time zones, which I always do, but the CEST time is correct ;))
<jannic[m]>
I wonder, if the macros are able to check that you are using the right macro for the type of interrupt / exception: Why do you need multiple macros in the first place? Wouldn't it be easier to have a single one that automatically does the right thing?
<romancardenas[m]>
Mm not sure what you mean
<bartmassey[m]>
Rather than core_interrupt, external_interrupt etc, just use interrupt, I think
<romancardenas[m]>
Oh right!
<jannic[m]>
I don't know what the macros do internally, so there's probably a good reason I just don't know. But as a user I'd think it would be easier to just write #[interrupt(CoreInterrupt::MachineTimer)] and not care if it's an internal or external one.
<romancardenas[m]>
The main motivation is that core interrupts may need a start trap routine when working in vectored mode
<romancardenas[m]>
Whereas external interrupts don't need that
<bartmassey[m]>
But you have the CoreInterrupt etc enums anyhow…
<bartmassey[m]>
I guess the matching for the macro gets harder?
<jannic[m]>
But if the macro can tell if the specified interrupt is internal or external, it could do that distinction automatically.
<romancardenas[m]>
Maybe we can assume that enums are called CoreInterrupt and ExternalInterrupt and do the filtering
andar1an[m]1 has joined #rust-embedded
<andar1an[m]1>
Could struct type states be of use here?
<jannic[m]>
But of course, if it's a natural distinction for a RISC-V developer, it may indeed be better to not do too much magic and have separate macros.
<romancardenas[m]>
The current version does not assume any naming convention. You can even import all the variants and just use the interrupt source name
<romancardenas[m]>
The macro just checks that the provided path implements the corresponding trait
<bartmassey[m]>
So the macro really doesn't care, only the generated code?
<jannic[m]>
For RP235x, it would be nice to have a common syntax for ARM and RISC-V, so one could write code that's portable to both core types. But that's probably a niche use case.
<andar1an[m]1>
Could potentially use typestate pattern for internal and external interrupts, but dk. Just saw at rust conf and it was cool pattern i have never used
<andar1an[m]1>
May allow for accounting for a vector chip or io interrupt easily while adhering to interupt variants that exist
<andar1an[m]1>
Sorry if silly, haven't played with that yet. Just seeing this made me think of that
<andar1an[m]1>
It seemed much less cumbersome than enum data modelling in some cases
<romancardenas[m]>
Yes, kind of. The macro checks that the signature is OK and generates dummy code that fails to compile if the provided path is not a valid core/external interrupt variant
<romancardenas[m]>
andar1an[m]1: Not sure about this, can you provide a toy example in the discussion?
<romancardenas[m]>
Currently in a RISC-V target I would expect an enum of Exceptions, an enum of CoreInterrupts and an enum of ExternalInterrupts
<andar1an[m]1>
I can provide a pattern link, 1 sec
<romancardenas[m]>
While the standard riscv crate provides a standard set of exceptions and core interrupts, targets can provide their own in case they don't comply with the standard
<romancardenas[m]>
Given that circumstance, I thought it would be better not to assume any naming convention for enums, but I can change it if you think it would make the code more readable/friendlier
<bartmassey[m]>
romancardenas: Shall we continue here, or are you good for now? We could continue with a discussion issue on Github and pick it up next week if you liked? I think exploring type system ideas might produce something a little cleaner, but it seems like maybe out-of-scope for here
<jannic[m]>
romancardenas[m]: Don't let your PR get derailed by my drive-by comment. It was just an idea, not a criticism.
<bartmassey[m]>
I think the tl;dr, by the way, is that this all looks really cool.
<bartmassey[m]>
Thanks much for proposing it!
<romancardenas[m]>
Np! We can continue next week after I explore your proposals
<rmsyn[m]>
imho, if one has to assume naming convention of the macro invocation or enum type, the macro invocation might be slightly more clear / offer more user freedom on types
<rmsyn[m]>
plus provides a gentle introduction to the different interrupt types via docs
<bartmassey[m]>
romancardenas: Maybe create a new discussion on `rust-embedded/wg` and link it from the meeting notes? I'd like to post some thoughts myself maybe…
<andar1an[m]1>
P.S. thanks to this PR I learned about interrupt types. Im just peanut gallery soaking in awesome haha
<bartmassey[m]>
Or should be just use the PR comments?
<jannic[m]>
Not sure who's here right now to look at the tickets. If nobody can contribute anything, that's fine as well, I'll bring them up again next week.
<jannic[m]>
I just try to bring a few tickets each week where I think that there could be some progress.
<romancardenas[m]>
I would like to take whatever we end up doing in riscv_rt to cortex_m_rt to keep a common interface, so we can discuss it as a wg issue if you like
<bartmassey[m]>
I think the first one is pretty easy. Does rmsyn or someone else have any comments? Sounds like there's pretty much consensus
korken89[m] has joined #rust-embedded
<korken89[m]>
If anyone else use KiCAD in CI a lot, but if you do and want to insert info in CI before generating production info I made this small script to inserts QR codes, maybe someone finds it useful https://github.com/korken89/kicad-qr-inserter :)
<bartmassey[m]>
korken89: Cool
<bartmassey[m]>
I'
<bartmassey[m]>
I'm going to hit merge on adding rmsyn to RISC-V team. Thanks much for your help!
<bartmassey[m]>
The third one seems easy also. I will close the issue unless there's any objection…
<bartmassey[m]>
I think we've pretty much agreed on an approach for alignment in linker scripts
<rmsyn[m]>
thanks for welcoming me to the team <3
<bartmassey[m]>
Alright. jannic I think we leave the second issue until next week when more folks are back who can comment. I'm not on top of the `cortex-m-rt` changes at all. Sounds good?
<jannic[m]>
Definitely. It also looks too involved to be decided about within the time of a meeting. Probably needs some review / testing.
<bartmassey[m]>
Cool. Anyone have anything else before we call it?
<bartmassey[m]>
(jannic BTW, I think the link for the second one is wrong? It seems to point to 0.7.4…)
<bartmassey[m]>
Rust memory model too hard 😀. Let's continue/capture this discussion as comments on the issue?
<bartmassey[m]>
It's a fun one though!
<jannic[m]>
<bartmassey[m]> "Cool. Anyone have anything..." <- Thanks for running the meeting!
<rmsyn[m]>
ah, thanks @dirbaio. maybe hiding the UnsafeCell in a similar way in critical-section also helps address the issue?
<bartmassey[m]>
No prob!
<dirbaio[m]>
rmsyn[m]: so people casually browsing the code don't see it therefore don't open issues asking why it's there? :D
<rmsyn[m]>
pretty much, and so that it conforms with most other Mutex types in the Rust ecosystem
<dirbaio[m]>
ah, you mean make the c-s Mutex give you &mut to the contents, like the other mutexes?
<rmsyn[m]>
yes, that was confusing to me as well when first using the c-s Mutex
<dirbaio[m]>
it's a super commonly asked question yeah
<bartmassey[m]>
Oh right, me too.
<bartmassey[m]>
Is there any technical reason why we couldn't or shouldn't give out `&mut` there?
<dirbaio[m]>
the reason it's like this is: a "normal" mutex like std::Mutex does two things:
<dirbaio[m]>
- convert Send to Sync: the mutex is always Sync even if the contents is only Send.
<dirbaio[m]>
- Interior mutability: let you mutate the contents even if you have only a `&Mutex`.
<dirbaio[m]>
critical_section::Mutex does only the "convert Send to Sync"
<Henk[m]>
I'm out, see you next week! 👋
<dirbaio[m]>
why? to allow you to "bring your own" interior mutability
<dirbaio[m]>
why would you want to do that? because then you can choose between RefCell or Cell
<dirbaio[m]>
RefCell gives you `&mut` to the contents, but uses extra memory for an "is locked" flag
<dirbaio[m]>
Cell only allows you to get/set, but in exchange uses no extra memory
<dirbaio[m]>
`critical_section::Mutex<Cell<u32>>` is 4 bytes
<dirbaio[m]>
`critical_section::Mutex<RefCell<u32>>` is 8 bytes
<dirbaio[m]>
`std::sync::Mutex<u32>` is sort of equivalent to `critical_section::Mutex<RefCell<u32>>`
<rmsyn[m]>
maybe provide a top-level alias with a UnsafeCell or similar wrapper for interior mutability, and another API for the bring-your-own interior mutability?
<bartmassey[m]>
But critical section get typically requires Option, so unless niche optimization is possible back to 8 bytes
<dirbaio[m]>
bartmassey[m]: only if you're working with owned things
<dirbaio[m]>
if you're storing "just data" you can go by fine without Option
<dirbaio[m]>
and it's not that universal: if you're using Embassy or RTIC you pretty much never do that crazy "put owned thing in CS mutex to later get it from interrupt" dance
<bartmassey[m]>
dirbaio: Not following I think. If I want to change the contents of the cell, I kind of have to take it, no? Unless the cell just contains a mut ref I guess?
<bartmassey[m]>
BTW, @room I think the meeting is officially over. Don't let that stop the discussion
<dirbaio[m]>
if you have `Mutex<Cell<u32>>` you can `MUTEX.borrow(cs).set(42)`, you never get a `&mut` to the content
<bartmassey[m]>
Ah. I see.
<bartmassey[m]>
So for copy types at least you're ok.
<dirbaio[m]>
Cell doesn't give you `&mut` to the content, it just allows you to get/set it wholesale.
<dirbaio[m]>
yeah it only works for Copy types
<dirbaio[m]>
but most "just plain data" is Copy
<dirbaio[m]>
when you're e.g. syncing the state of something between tasks
<rmsyn[m]>
right, but for types that require methods like `fn some(&mut self, args...)`, the ability to get a mutable reference is needed
<dirbaio[m]>
yep
<dirbaio[m]>
then you need RefCell, which has memory overhead
<dirbaio[m]>
(OR you can copy the whole thing out and call the `&mut self` method on the copy)
<bartmassey[m]>
Or UnsafeCell, which has footguns
<dirbaio[m]>
so ikd
<dirbaio[m]>
* so idk
<dirbaio[m]>
I agree it's confusing. everyone seems confused by it
<dirbaio[m]>
but fixing it would require critical-section 2.0 which would be quite disruptive
<bartmassey[m]>
I've got to go right now, but I'd love to get some docs + a crate together to provide sort of "standard idioms" for all of this…
<rmsyn[m]>
^ +1, I'd be willing to help on that
<dirbaio[m]>
because it also contains the trait that all arch crates / hals impl and everyone needing a CS consumes
<jannic[m]>
dirbaio[m]: Why would it? Is it visible from the outside?
<dirbaio[m]>
if we release 2.0 it'll split the ecosystem, a hal using the 1.0 trait wouldn't work with a crate wanting 2.0
JamesMunns[m] has joined #rust-embedded
<JamesMunns[m]>
oh, btw, reading back, but I have a critical section mutex crate that works like a normal mutex
<JamesMunns[m]>
docs.rs/mutex
<dirbaio[m]>
(unless we do more hax like trying to name the 2.0 and 1.0 symbols the same way...)
<dirbaio[m]>
I think putting Mutex in the critical-section crate was a mistake
<rmsyn[m]>
if it's that disruptive, maybe being able to point to a crate like @jamemunns is enough to clarify confusion?
<bartmassey[m]>
My thing is stale and maybe wrong. But it works and makes my students' lives easier 😀
<dirbaio[m]>
critical-section should've had only the trait, then make the mutex in a separate crate like mutex or embedded-mutex or critical-section-mutex so we can iterate on it without breaking the world :|
<dirbaio[m]>
we can always deprecate critical_section::Mutex and tell the users to use $CRATE
<bartmassey[m]>
Like I say, have to go, but yeah, let's figure out a better state for all this. Thanks all!
<dirbaio[m]>
* hmmmm it feels like this godbolt is a proof removing the UnsafeCell is unsound?
<thalesfragoso[m]>
The libstd's Mutex itself had problems with niche optimization, then they made a special case for UnsafeCell to remove it.
<dirbaio[m]>
it's "recycling" the pointer read from the 1st mutex lock in the 2nd one
<dirbaio[m]>
that shouldn't be allowed
<dirbaio[m]>
another thread might have locked the mutex in the middle and changed the pointer
<jannic[m]>
Why should it make it unsound? There's nothing allowing interior mutability, the Mutex itself doesn't provide that. So the compiler is actually allowed to assume that the pointer doesn't change. It's a valid optimization.
<jannic[m]>
Even with a locked Mutex, the pointer itself can't be changed.
dav1d has quit [Ping timeout: 276 seconds]
<dirbaio[m]>
ah sorry you have `Mutex1<Register>`, not `Mutex1<Cell<Register>>`
<dirbaio[m]>
yesyes
<dirbaio[m]>
hmmm
<dirbaio[m]>
okay so the optimization seems correct then
<thalesfragoso[m]>
Niche optimization would still be a problem, because rustc could emit a read to the mutex data without locking it, if the mutex is inside an enum.
<jannic[m]>
Exactly. Once you have a `Cell` you get interior mutability. And there really shouldn't be a difference between `Cell<Register>` and `UnsafeCell<Cell<Register>>`.
<jannic[m]>
But in embedded code, it's not unreasonable to actually have a `Mutex<Register>` without a cell, and the Mutex only makes sure nobody accesses the _contents_ of the register concurrently.
<thalesfragoso[m]>
And you shouldn't be able to read for !Sync data without a lock.
<dirbaio[m]>
interesting
<thalesfragoso[m]>
I think MaybeUninit also has this niche optimization disabled ? So maybe we could use that instead of UnsafeCell
<dirbaio[m]>
so `Option<Mutex<Cell<NonZeroU32>>>`?
<thalesfragoso[m]>
But I have to double check.
<dirbaio[m]>
doesn't the UnsafeCell inside Cell prevent the issue?
dav1d has joined #rust-embedded
<dirbaio[m]>
and the issue can't happen with `Option<Mutex<NonZeroU32>>` afaict?
<dirbaio[m]>
because no way to modify the mutex contents
<thalesfragoso[m]>
dirbaio[m]: It should, but are we relying on all Send + !Sync types to contain an UnsafeCell inside ?
<dirbaio[m]>
the niche issue can only occur with interior mutability, and interior mutability requires UnsafeCell which disables niches
<dirbaio[m]>
I think...?
<thalesfragoso[m]>
Sync means that you can't even have a &T, it doesn't talk about interior mutability
<thalesfragoso[m]>
I mean, I also don't think that it could be a problem without UnsafeCell, but if there's some weird type out there now, or someone creates it on the future .-.
<thalesfragoso[m]>
They even had to suppress niches in coroutines recently, so I thinking not suppressing niches in a Mutex is a bug waiting to happen.
<thalesfragoso[m]>
But I think we could just use MaybeUninit instead.
<thalesfragoso[m]>
If we want that.
<thalesfragoso[m]>
s/thinking/think/
<jannic[m]>
I don't understand the issue yet. Is it that `T` might contain something like a `NonZeroU32`, and one thread might assume that it's safe to store a 0 in there temporarily as nobody else can see it anyway, because `T` is `!Sync`, and the other thread, having an `Option<Mutex<T>>` might use the niche optimization and confuse a `Some` with a `None`?
<thejpster[m]1>
hey - quick pop quiz.
<thejpster[m]1>
What's the difference between building the rust toolchain from source, and using a nightly rust, for a tier 3 target.
<thalesfragoso[m]>
jannic[m]: The problem is that the compiler can store the Option variant inside the data contained in the Mutex, and then rustc emit reads to that data without locking it, e.g. on match
<thejpster[m]1>
The issue is that one produces a binary that crashes weirdly, and the other does not
<JamesMunns[m]>
thejpster[m]1: uhh, do you have the same llvm version?
<thalesfragoso[m]>
* The problem is that the compiler can store the Option variant inside the data contained in the Mutex, and then rustc will emit reads to that data without locking it, e.g. on match
<thejpster[m]1>
yup - first thing I checked
<JamesMunns[m]>
I forget how hermetic llvm settings are when you build locally, or what things get skipped using existing assets
<thejpster[m]1>
I believe I have the answer, and I believe it's not an LLVM issue.
<JamesMunns[m]>
oh neat, I'm about to be horrified, aren't I?
<thejpster[m]1>
something more sinister, involving tier 3 specifically. And a simulated target where all the code is in RAM.
<JamesMunns[m]>
oh no is it something like fs paths?
<thejpster[m]1>
nope
<thejpster[m]1>
what's special about tier 3?
<JamesMunns[m]>
I'm way too tired for a quiz, i'll let someone else guess, I suppose
<thejpster[m]1>
tier 3 has no stdlibs in rustup
<thejpster[m]1>
so you just build-std=core
<thejpster[m]1>
whilst if you compile from source, the libcore is pre-built
<thejpster[m]1>
these libcores are very different
<thejpster[m]1>
because if I understand this right, one is built with --release and one is not (unless you build your whole project with --release)
<thejpster[m]1>
so I think the issue is plain old stack overflow which only happens with a non-optimised libcore and not with an optimised libcore
<jannic[m]>
<thalesfragoso[m]> "The problem is that the compiler..." <- In what case is such a read problematic? We know that it's a type that doesn't have interior mutability, so there can't be a data race. And we could get a `&` via locking the Mutex, so it can't be some strange thing that must not be read at all.
<thejpster[m]1>
or, "this target only works if I compile the toolchain from source"
<JamesMunns[m]>
oh, at least for stock libcores, they use opt-level=2
<JamesMunns[m]>
but build-std would use your opt level profile
<JamesMunns[m]>
which might be s/3/z
<thejpster[m]1>
they're building without --release
<thejpster[m]1>
anyway, I'm sorry, this bug has occupied my brain for hours and I had to let it out
<thejpster[m]1>
because on the face of it, it MAKES NO SENSE
<JamesMunns[m]>
thejpster[m]1: this seems very likely
<dirbaio[m]>
<thejpster[m]1> "whilst if you compile from..." <- I think when you build rustc yourself you can not prebuild the std and still use -Zbuild-std
<dirbaio[m]>
* you can still not prebuild
<dirbaio[m]>
you need to copy/symlink the sources in the right place, dunno if there's some magic bootstrap flag that doe sit
<dirbaio[m]>
s/doe/does/, s/sit/it/
<thalesfragoso[m]>
<jannic[m]> "In what case is such a read..." <- That's the thing, we don't have these kind of guarantees. !Sync only says that you can't even get an immutable reference, it doesn't specify more than that.
<thejpster[m]1>
actually that's a good point - if the unstable flag is set for build-std=core, does it do it even if it has a prebuilt libcore available?
<thejpster[m]1>
because if both compilers are building libcore from source, there goes my theory
<dirbaio[m]>
yes, build-std always takes priority
<thejpster[m]1>
does a locally built rustc obey the [unstable] section in the cargo config?
<thejpster[m]1>
or does it believe itself to be a stable compiler? I guess it might think it is a nightly.
<thejpster[m]1>
s/might/must/
<dirbaio[m]>
i'm not sure
<dirbaio[m]>
it doesn't build cargo from source
<thejpster[m]1>
I can't actually test any of this because the RTEMS tools are for Linux and I don't wanna rebuild their gcc from source for macOS
<dirbaio[m]>
so maybe it's possible it's using your stable cargo from rustup, yes
<thejpster[m]1>
oh no, of course
<thalesfragoso[m]>
thalesfragoso[m]: Like, take the coroutine example, there isn't interior mutability there, but it was causing issues with no-alias.
<thalesfragoso[m]>
So it's something I would rather err on the cautions side.
<dirbaio[m]>
toolchains installed with rustup link will use whatever cargo you have installed, giving prefence to nightly
<dirbaio[m]>
you'll get stable if you don't have nightly installed
<dirbaio[m]>
maybe that's why it's ignoring build-std for you
<thalesfragoso[m]>
<jannic[m]> "Do you have a link to that..." <- I don't think it applies to our Mutex case, that only happened in rustc because they're breaking the no-alias rule by making exceptions to some built-in types.
aliceryhl[m] has quit [Quit: Idle timeout reached: 172800s]
<thalesfragoso[m]>
But let me get the link for you
<jannic[m]>
The main difference between the coroutine example and ours is: Given a T without interior mutability, once you hand out a `&Mutex<T>`, you can't modify the `T` at all, as long as the reference exists. And you can't get a `&mut` to something inside T either. So the situation is very different.
<jannic[m]>
(Though, of course, your point that it may be wise to just be cautious still stands.)
<jannic[m]>
Also, as I understand it, even in the coroutine example, there was no actual miscompilation. They "only" had miri complain about a `&mut` and a `&` existing at the same time. Which, of course, is bad enough.
AlexandrosLiarok has joined #rust-embedded
<AlexandrosLiarok>
Does anyone have a strong opinion on what a good dma ringbuffer interface looks like? Especially when it comes to underrun reporting and recovery.
<thalesfragoso[m]>
jannic[m]: > <@jannic:matrix.org> The main difference between the coroutine example and ours is: Given a T without interior mutability, once you hand out a `&Mutex<T>`, you can't modify the `T` at all, as long as the reference exists. And you can't get a `&mut` to something inside T either. So the situation is very different.
<thalesfragoso[m]>
> (Though, of course, your point that it may be wise to just be cautious still stands.)
<thalesfragoso[m]>
Indeed, my point is that Sync itself doesn't mention modification at all. Only having &T across "threads" is enough to be unsound.
<thalesfragoso[m]>
Can I provide an example where this is a problem in practice? Nope...
<JamesMunns[m]>
thalesfragoso[m]: there's a bunch of OS things like "you can't touch this gui instance outside of the main thread/thread you created it in"
<thalesfragoso[m]>
Right, but that might already be broken with our Mutex...
<JamesMunns[m]>
very fair!
<jannic[m]>
Such a thing wouldn't be Send, but Mutex is about Sync. And Mutex requires T: Send.
<thalesfragoso[m]>
> The precise definition is: a type T is Sync if and only if &T is Send. In other words, if there is no possibility of undefined behavior (including data races) when passing &T references between threads.
<jannic[m]>
"In other words, if there is no possibility of undefined behavior (including data races) when passing &T references between threads." is similar. But it's the opposite statement. If passing `&T` to another thread would cause UB, it's obvious that T can't be `Sync`.
<JamesMunns[m]>
<jannic[m]> "Such a thing wouldn't be Send,..." <- oops yes, you're right!
<jannic[m]>
But we are not creating concurrent instances of `&T`. We only have a `&Mutex<T>` concurrently with a `&T`. Which, as we noticed, may cause a concurrent read access because of niche optimization. But that's not the same as having a `&T`.
<jannic[m]>
It would be unsound if the other side wasn't just another `&T` but a `&mut T` (like in the coroutine example). But that's not the case here.
<thalesfragoso[m]>
With niche optimization, having &Mutex<T> is the same as having &T.
<jannic[m]>
Is it? Can I call a function on `T` if I have a `&Mutex<T>` without locking it? Due to niche optimization?
<thalesfragoso[m]>
jannic[m]: If we go with the premise that our Mutex only makes Send types Sync if going through the locking mechanism, then having &T without locking is a problem.
<thalesfragoso[m]>
jannic[m]: rustc generates it when matching, and that follows all LLVM rules, that's why the coroutine example was a problem.
<thalesfragoso[m]>
You would see the &T anywhere in the code, but that still was enough to trigger alias problems.
<jannic[m]>
Rustc generates a function call on T? AFAIK it only reads the value. Which is fine. After all it's an initialized, immutable value.
<thalesfragoso[m]>
wouldn't*
<thalesfragoso[m]>
jannic[m]: Reading the value is enough, because rustc generating a read due to a match is no different than some code doing it, at least through LLVM's point of view.
<thalesfragoso[m]>
If someone creates a magical type that can only be accessed as Sync once the mutex is locked, then that's a problem.
<thalesfragoso[m]>
I don't know of any such magical type though, but the rules of rust don't rule it out.
<thalesfragoso[m]>
Again, I'm just being "too cautions" and recommending `MaybeUninit<T>` instead of a bare `T`.
<thalesfragoso[m]>
I don't think losing that niche optimization will hurt anyone.
<thalesfragoso[m]>
Yes, but that's the whole LLVM poison discussion, i.e. LLVM knows how to handle that. But having rustc generation control flow based on a poison value is another story.
<thalesfragoso[m]>
Your points are correct and I can't provide a practical example where it would cause problems.
<thalesfragoso[m]>
But in theory only by having &T, reading its value and performing control flow based on it, without locking the Mutex could be seen as breaking the Sync invariant of T.
<thalesfragoso[m]>
And only the fact of that being possible already can cause problems because it's quite surprising.
<thalesfragoso[m]>
I'm for sure was surprised when they discovered this problem in std::sync::Mutex. (UnsafeCell didn't prevent niches before people found about that).