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

kmc-solid: Use the filesystem thread-safety wrapper #93847

Merged
merged 1 commit into from
Feb 19, 2022

Conversation

kawadakk
Copy link
Contributor

@kawadakk kawadakk commented Feb 10, 2022

Fixes the thread unsafety of the std::fs implementation used by the *-kmc-solid_* Tier 3 targets.

Neither the SOLID filesystem API nor built-in filesystem drivers guarantee thread safety by default. Although this may suffice in general embedded-system use cases, and in fact the API can be used from multiple threads without any problems in many cases, this has been a source of unsoundness in std::sys::solid::fs.

This commit updates the implementation to leverage the filesystem thread-safety wrapper (which uses a pluggable synchronization mechanism) to enforce thread safety. This is done by prefixing all paths passed to the filesystem API with \TS. (Note that relative paths aren't supported in this platform.)

Neither the SOLID filesystem API nor built-in filesystems guarantee
thread safety by default. Although this may suffice in general embedded-
system use cases, and in fact the API can be used from multiple threads
without any problems in many cases, this has been a source of
unsoundness in `std::sys::solid::fs`.

This commit updates the `std` code to leverage the filesystem thread-
safety wrapper to enforce thread safety. This is done by prefixing all
paths passed to the filesystem API with `\TS`. (Note that relative paths
aren't supported in this platform.)
@rust-highfive
Copy link
Collaborator

r? @yaahc

(rust-highfive has picked a reviewer for you, use r? to override)

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Feb 10, 2022
@yaahc
Copy link
Member

yaahc commented Feb 16, 2022

This commit updates the implementation to leverage the filesystem thread-safety wrapper (which uses a pluggable synchronization mechanism) to enforce thread safety. This is done by prefixing all paths passed to the filesystem API with \TS. (Note that relative paths aren't supported in this platform.)

For my own edification, could you point me towards the API docs that outline how the \TS threadsafety prefix works on SOLID? I'm unfamiliar with the SOLID platform and having trouble finding the API docs.

@kawadakk
Copy link
Contributor Author

kawadakk commented Feb 17, 2022

For my own edification, could you point me towards the API docs that outline how the \TS threadsafety prefix works on SOLID? I'm unfamiliar with the SOLID platform and having trouble finding the API docs.

I'm afraid this part is one of the areas where the documentation is lagging behind.

To give a brief explanation, the thread-safety wrapper is an optional mechanism of SOLID Core Services Filesystem that adds thread safety to otherwise-unsafe filesystem drivers. The basic idea is that all accesses made through this prefix are somehow made safe (at a runtime cost and complication with RTOS interaction), as long as other parts of the application don't do the otherwise.

Since std is dependent on an operating system anyway, I think it's a fair assumption that all prospective SOLID applications including Rust code don't have much issue taking the trade-off regarding the use of the wrapper and using it exclusively.

@yaahc
Copy link
Member

yaahc commented Feb 17, 2022

Sounds good, thank you for the explanation ^_^

@bors r+

@bors
Copy link
Contributor

bors commented Feb 17, 2022

📌 Commit 64406c5 has been approved by yaahc

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Feb 17, 2022
bors added a commit to rust-lang-ci/rust that referenced this pull request Feb 19, 2022
…askrgr

Rollup of 10 pull requests

Successful merges:

 - rust-lang#89892 (Suggest `impl Trait` return type when incorrectly using a generic return type)
 - rust-lang#91675 (Add MemTagSanitizer Support)
 - rust-lang#92806 (Add more information to `impl Trait` error)
 - rust-lang#93497 (Pass `--test` flag through rustdoc to rustc so `#[test]` functions can be scraped)
 - rust-lang#93814 (mips64-openwrt-linux-musl: correct soft-foat)
 - rust-lang#93847 (kmc-solid: Use the filesystem thread-safety wrapper)
 - rust-lang#93877 (asm: Allow the use of r8-r14 as clobbers on Thumb1)
 - rust-lang#93892 (Only mark projection as ambiguous if GAT substs are constrained)
 - rust-lang#93915 (Implement --check-cfg option (RFC 3013), take 2)
 - rust-lang#93953 (Add the `known-bug` test directive, use it, and do some cleanup)

Failed merges:

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit 724cca6 into rust-lang:master Feb 19, 2022
@rustbot rustbot added this to the 1.60.0 milestone Feb 19, 2022
@kawadakk kawadakk deleted the fix-kmc-solid-fs-ts branch February 21, 2022 01:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants