Skip to content

Revise the VEXos synchronization model - #162257

Draft
lewisfm wants to merge 3 commits into
rust-lang:mainfrom
vexide:lewisfm/sync-model
Draft

lewisfm wants to merge 3 commits into
rust-lang:mainfrom
vexide:lewisfm/sync-model

Conversation

@lewisfm

@lewisfm lewisfm commented Sep 3, 2026

Copy link
Copy Markdown
Contributor

This PR fixes some compilation errors in std::sync under VEXos by switching to a simple spinlock-based model that properly handles the "multithreading" provided by this platform through user interrupt handlers and documenting exactly how and how not std::sync may be used on this target. Spinlocks are used because they are very simple and the OS on this target is not aware of threading by default and has no documented builtin futex or mutex API we can use. I don't expect this platform to have more than 1 thread blocking on access for a mutex at once (since there is only 1 thread that's allowed to block per the updated target docs).

Previously Rust programs running on VEXos were only allowed a single execution context and user ISRs were not allowed to interact with std code whatsoever. After these changes, ISRs are allowed to partially interact with std and std::sync can be used to synchronize shared resources between the main thread and interrupts.

Using stack-switching schedulers on this platform remains unsound; this is now written explicitly in the target documentation.

More context on these changes in this Zulip thread: https://rust-lang.zulipchat.com/#narrow/channel/219381-t-libs/topic/Meaning.20of.20.22singlethread.22.20in.20target.20spec/with/614265573

LLM disclosure: I did not use LLMs to generate any part of this PR. I did use an LLM to help plan and review my changes before submitting them publicly. All code and documentation is written from scratch in my own style.

cc @tropicaaal @Gavin-Niederman

@rustbot

rustbot commented Sep 3, 2026

Copy link
Copy Markdown
Collaborator

Some changes occurred in src/doc/rustc/src/platform-support

cc @Noratrieb

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-libs Relevant to the library team, which will review and decide on the PR/issue. labels Sep 3, 2026
@rustbot

rustbot commented Sep 3, 2026

Copy link
Copy Markdown
Collaborator

r? @nnethercote

rustbot has assigned @nnethercote.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

Why was this reviewer chosen?

The reviewer was selected based on:

  • Owners of files modified in this PR: compiler
  • compiler expanded to 75 candidates
  • Random selection from 21 candidates

@lewisfm

lewisfm commented Sep 3, 2026

Copy link
Copy Markdown
Contributor Author

This fixes the compile errors in libstd but I haven't had a chance to test it on hardware, I'll report back in a few days once I get the chance. Not sure if it's worth marking this as a draft in the meantime?

@nnethercote

Copy link
Copy Markdown
Contributor

@lewisfm: Marking as a draft would be good, thanks.

I'm not an appropriate reviewer. I think someone more appropriate would be:

r? @joboet

Why was this reviewer chosen?

The reviewer was selected based on:
* Owners of files modified in this PR: compiler
* compiler expanded to 75 candidates
* Random selection from 21 candidates

@Kobzol: Is this a bug in reviewer allocation? This PR touches multiple files under library/std/src/sys/ and src/doc/rustc/src/platform-support/thumbv7a-vex-v5.md, but nothing under compiler/. (The "T-libs" label was correctly applied.)

@rustbot

rustbot commented Sep 3, 2026

Copy link
Copy Markdown
Collaborator

joboet is currently at their maximum review capacity.
They may take a while to respond.

@lewisfm
lewisfm marked this pull request as draft September 3, 2026 22:11
@rustbot rustbot added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Sep 3, 2026
@Kobzol

Kobzol commented Sep 4, 2026

Copy link
Copy Markdown
Member

@lewisfm: Marking as a draft would be good, thanks.

I'm not an appropriate reviewer. I think someone more appropriate would be:

r? @joboet

Why was this reviewer chosen?
The reviewer was selected based on:

  • Owners of files modified in this PR: compiler
  • compiler expanded to 75 candidates
  • Random selection from 21 candidates

@Kobzol: Is this a bug in reviewer allocation? This PR touches multiple files under library/std/src/sys/ and src/doc/rustc/src/platform-support/thumbv7a-vex-v5.md, but nothing under compiler/. (The "T-libs" label was correctly applied.)

Not sure if a bug, but it seems suboptimal for sure. What happened is that library/std is mapped to libs, but src/doc/rustc is mapped to compiler. Before counting file count and line change contributions, triagebot will select only the owners with the maximum "length", which is the number of path segments in the owners table. So it sees that we match both library/std and src/doc/rustc, and the latter has 3 segments, so it completely ignores library/std, even if it matched a thousand files in this PR. And thus only src/doc/rustc remains, and that gets mapped to compiler.

I'll send a PR to triagebot to fix it.

Scratch the above, I did more investigation and the cause is different. It is simply caused by the fact that the src/doc/rustc/... file has more changed lines than the sum of the changes lines in the library directory.

@tropicaaal tropicaaal left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This seems good, it's kind of precarious that ISRs can't access thread_locals since this makes panicking inside of interrupt context effectively UB (and that's hard to avoid entirely), but this is a necessary stepping stone to adding support for interrupt-aware thread_locals anyways. I'll test this on hardware in a bit.

Also, for whoever ends up reviewing this: it should be noted that the vast majority of users will never have to touch interrupts on this platform, and these changes were done so that we can be singlethread = false and therefore get real atomics (see the zulip discussion linked above). #[no-std] applications on this target have use-cases for having real atomics, despite this platform not having OS threads. Previously, we used the no_threads implementation of std::sync/thread_local while being singlethread = false, but libstd was changed to throw a compile error under those conditions causing this target to no longer build.

Anyways, tl;dr, this change was done because libstd equates supporting atomics with supporting threads, and the best way to work around that was to make std::sync interrupt-aware.

View changes since this review

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

Labels

S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. T-libs Relevant to the library team, which will review and decide on the PR/issue.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants