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

Rwlock try upgrade #138560

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from
Draft

Conversation

ShrigmaDev
Copy link

Tracking issue: #138559

This PR adds a try_upgrade method on RwLockReadGuard that transforms it into a RwLockWriteGuard if it is immediately possible (ie if there are no read guards except for this one at this time)

@rustbot
Copy link
Collaborator

rustbot commented Mar 16, 2025

r? @thomcc

rustbot has assigned @thomcc.
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

@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 Mar 16, 2025
@rust-log-analyzer

This comment has been minimized.

@rust-log-analyzer

This comment has been minimized.

@rust-log-analyzer

This comment has been minimized.

@ShrigmaDev ShrigmaDev marked this pull request as draft March 16, 2025 23:22
@ShrigmaDev
Copy link
Author

ShrigmaDev commented Mar 18, 2025

Not sure how to proceed in rwlock.rs, since the RwLock variables must be sized, like the compiler error suggests. Basically I'm unsure how to convert a RwLockReadGuard into a RwLockWriteGuard. Do you have any pointers @thomcc ?

@rust-log-analyzer
Copy link
Collaborator

The job x86_64-gnu-llvm-18 failed! Check out the build log: (web) (plain)

Click to see the possible cause of the failure (guessed by this bot)
#19 exporting to docker image format
#19 sending tarball 19.7s done
#19 DONE 25.1s
##[endgroup]
Setting extra environment values for docker:  --env ENABLE_GCC_CODEGEN=1 --env GCC_EXEC_PREFIX=/usr/lib/gcc/
[CI_JOB_NAME=x86_64-gnu-llvm-18]
[CI_JOB_NAME=x86_64-gnu-llvm-18]
debug: `DISABLE_CI_RUSTC_IF_INCOMPATIBLE` configured.
---
sccache: Listening on address 127.0.0.1:4226
##[group]Configure the build
configure: processing command line
configure: 
configure: build.configure-args := ['--build=x86_64-unknown-linux-gnu', '--llvm-root=/usr/lib/llvm-18', '--enable-llvm-link-shared', '--set', 'rust.randomize-layout=true', '--set', 'rust.thin-lto-import-instr-limit=10', '--set', 'build.print-step-timings', '--enable-verbose-tests', '--set', 'build.metrics', '--enable-verbose-configure', '--enable-sccache', '--disable-manage-submodules', '--enable-locked-deps', '--enable-cargo-native-static', '--set', 'rust.codegen-units-std=1', '--set', 'dist.compression-profile=balanced', '--dist-compression-formats=xz', '--set', 'rust.lld=false', '--disable-dist-src', '--release-channel=nightly', '--enable-debug-assertions', '--enable-overflow-checks', '--enable-llvm-assertions', '--set', 'rust.verify-llvm-ir', '--set', 'rust.codegen-backends=llvm,cranelift,gcc', '--set', 'llvm.static-libstdcpp', '--enable-new-symbol-mangling']
configure: build.build          := x86_64-unknown-linux-gnu
configure: target.x86_64-unknown-linux-gnu.llvm-config := /usr/lib/llvm-18/bin/llvm-config
configure: llvm.link-shared     := True
configure: rust.randomize-layout := True
configure: rust.thin-lto-import-instr-limit := 10
---
     |
858  | impl<'a, T: ?Sized> RwLockReadGuard<'a, T> {
     |          - this type parameter needs to be `Sized`
...
925  |             data: UnsafeCell::new(orig.data.as_ptr().read()),
     |                                                      ^^^^ doesn't have a size known at compile-time
     |
note: required by a bound in `core::ptr::mut_ptr::<impl *mut T>::read`
    --> /checkout/library/core/src/ptr/mut_ptr.rs:1353:12
     |
---
858  + impl<'a, T> RwLockReadGuard<'a, T> {
     |
help: consider removing this method call, as the receiver has type `NonNull<T>` and `NonNull<T>: core::marker::Sized` trivially holds
     |
925  -             data: UnsafeCell::new(orig.data.as_ptr().read()),
925  +             data: UnsafeCell::new(orig.data.read()),
     |

error[E0277]: the size for values of type `T` cannot be known at compilation time
    --> library/std/src/sync/poison/rwlock.rs:925:35
     |
858  | impl<'a, T: ?Sized> RwLockReadGuard<'a, T> {
     |          - this type parameter needs to be `Sized`
...
925  |             data: UnsafeCell::new(orig.data.as_ptr().read()),
     |                   --------------- ^^^^^^^^^^^^^^^^^^^^^^^^^ doesn't have a size known at compile-time
     |                   |
     |                   required by a bound introduced by this call
     |
note: required by a bound in `UnsafeCell::<T>::new`
---
     |
858  - impl<'a, T: ?Sized> RwLockReadGuard<'a, T> {
858  + impl<'a, T> RwLockReadGuard<'a, T> {
     |
help: consider removing this method call, as the receiver has type `*mut T` and `*mut T: core::marker::Sized` trivially holds
     |
925  -             data: UnsafeCell::new(orig.data.as_ptr().read()),
925  +             data: UnsafeCell::new(orig.data.as_ptr()),
     |

error[E0277]: the size for values of type `T` cannot be known at compilation time
   --> library/std/src/sync/poison/rwlock.rs:924:20
    |
858 |   impl<'a, T: ?Sized> RwLockReadGuard<'a, T> {
    |            - this type parameter needs to be `Sized`
...
924 |           let rwl = &RwLock {
    |  ____________________^
925 | |             data: UnsafeCell::new(orig.data.as_ptr().read()),
926 | |             poison: poison::Flag::new(),
927 | |             inner: *orig.inner_lock,
928 | |         };
    | |_________^ doesn't have a size known at compile-time
    |
note: required because it appears within the type `poison::rwlock::RwLock<T>`
   --> library/std/src/sync/poison/rwlock.rs:80:12

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
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.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants