BufferMappedRange trait object can not be sent between threads

[pema99]

Description
With wgpu 0.14, code like the following is valid:

let (tx, rx) = futures::channel::oneshot::channel();
wgpu::util::DownloadBuffer::read_buffer(
    &self.fw.device,
    &self.fw.queue,
    &self.buffer.slice(..download_size as u64),
    move |result| {
        tx.send(result).unwrap_or_else(|_| panic!("Failed to download buffer."));
    });
self.fw.device.poll(wgpu::Maintain::Wait);
let download = rx.block_on().unwrap().unwrap();
...

With wgpu 0.15 and forward, it won't compile, because of this change 052bd17#diff-00d306ac48be4eb4a03df7b093e35fabfb521d981f82315a3a725a9dc15e1ae1R77-R80

The error I get is:

error[E0277]: `(dyn wgpu::context::BufferMappedRange + 'static)` cannot be sent between threads safely
   --> src\gpgpu2.rs:332:13
    |
328 |           wgpu::util::DownloadBuffer::read_buffer(
    |           --------------------------------------- required by a bound introduced by this call
...
332 | /             move |result| {
333 | |                 tx.send(result)
334 | |                     .unwrap_or_else(|_| panic!("Failed to download buffer."));
335 | |             },
    | |_____________^ `(dyn wgpu::context::BufferMappedRange + 'static)` cannot be sent between threads safely
    |
    = help: the trait `std::marker::Send` is not implemented for `(dyn wgpu::context::BufferMappedRange + 'static)`
    = note: required for `Unique<(dyn wgpu::context::BufferMappedRange + 'static)>` to implement `std::marker::Send`
    = note: required because it appears within the type `Box<dyn BufferMappedRange>`
    = note: required because it appears within the type `DownloadBuffer`
    = note: required because it appears within the type `Result<DownloadBuffer, BufferAsyncError>`
    = note: required because it appears within the type `Option<Result<DownloadBuffer, BufferAsyncError>>`
    = note: required for `channel::lock::Lock<Option<Result<DownloadBuffer, BufferAsyncError>>>` to implement `Sync`
    = note: required because it appears within the type `Inner<Result<DownloadBuffer, BufferAsyncError>>`
    = note: required for `Arc<oneshot::Inner<Result<DownloadBuffer, BufferAsyncError>>>` to implement `std::marker::Send`
    = note: required because it appears within the type `Sender<Result<DownloadBuffer, BufferAsyncError>>`
note: required because it's used within this closure
   --> src\gpgpu2.rs:332:13
    |
332 |             move |result| {
    |             ^^^^^^^^^^^^^
note: required by a bound in `DownloadBuffer::read_buffer`
   --> C:\Users\Pema Malling\.cargo\registry\src\github.com-1ecc6299db9ec823\wgpu-0.15.1\src\util\mod.rs:98:72
    |
98  |         callback: impl FnOnce(Result<Self, super::BufferAsyncError>) + Send + 'static,
    |                                                                        ^^^^ required by this bound in `DownloadBuffer::read_buffer`

Repro steps

• Clone https://github.com/pema99/rust-path-tracer/tree/wgpu-fail (branch wgpu-fail).
• cargo build
• Build fails

Expected vs observed behavior
Expected: Build succeeds
Actual: Build fails

Extra materials
The reason I am using this older method of downloading buffers, and not directly using map_async, is that I found for my use case that adding MAP_READ to the buffer I want to read back causes a massive drop in performance. I can still use this older method, but because the trait object isn't Send, I'm forced to do an extra copy (ie. copy to vector, send the vector between threads instead). This causes a drop in performance.

I've "fixed" the issue myself on my own branch, but I'm not sure if the change is ok and/or breaking: pema99@e44f8ae

[teoxoy]

BufferMappedRange implements Send and Sync on the direct backend

wgpu/wgpu/src/backend/direct.rs

Lines 3050 to 3051 in 9c8666a

	unsafe impl Send for BufferMappedRange {}
	unsafe impl Sync for BufferMappedRange {}

but not on the web

wgpu/wgpu/src/backend/web.rs

Line 3232 in 9c8666a

actual_mapping: js_sys::Uint8Array,

due to js_sys::Uint8Array.

BufferView and BufferViewMut have the same issue.

[teoxoy]

It would be good to coordinate with #3691 cc @daxpedda for a fix (making the BufferMappedRange trait require Send + Sync on native but not on web).

[daxpedda]

Already covered in that PR.

wgpu/wgpu/src/backend/direct.rs

Lines 3051 to 3054 in 826b36e

	#[cfg(not(target_arch = "wasm32"))]
	unsafe impl Send for BufferMappedRange {}
	#[cfg(not(target_arch = "wasm32"))]
	unsafe impl Sync for BufferMappedRange {}

[teoxoy]

I meant the trait specifically.

[Wumpf]

Fixed in

• Fix BufferMapRange not being Send/Sync (on native) #4818