unnecessary Send+Sync requirement on framebuffer of AutoCommandBufferBuilder::begin_render_pass

[SamP20]

This an issue currently blocking Vulkano support in rust-SDL2 Rust-SDL2/rust-sdl2#785.

The current implementation of vulkano::command_buffer::AutoCommandBufferBuilder::begin_render_pass (and possibly others) requires framebuffer to be Send+Sync. This in turn requires vulkano::swapchain::Surface<W> to also be Send+Sync. Unfortunately rust-SDL2 has the requirement that it's windows cannot be sent between threads, and thus cannot be stored in a Surface<W>.

Is there any way of removing the Send+Sync requirement of W in Surface<W>?

[Rua]

If the requirement can't be removed, then W should be required to also implement Send + Sync to communicate this. It would avoid unexpected surprises like this one.

[Rua]

Just out of curiosity I downloaded the code, and removed Send + Sync as a requirement from AutoCommandBufferBuilder::begin_render_pass, and then continued to remove it from various pieces of code until it compiled successfully again. I don't know if this proves that these traits aren't really necessary, but it's a good indication.

[Rua]

Is there any news on this? The people at SDL are wondering if SDL is safe to use with Vulkan. Vulkano is forcing unsafe usage.

[rukai]

I wish I could help here but I have no idea about this kind of thing and my attention is currently on other issues ¯\(ツ)/¯

[hammypants]

Getting bit by this a few years later. :)

[KeyboardDanni]

This is currently blocking me from using vulkano in my engine.

[Eliah-Lakhin]

@cosmicchipsocket, @hammypants I sent a message to @AustinJ235 (current maintainer) by email. Hope we can resolve the issue soon.

[KeyboardDanni]

I am using the workaround mentioned in Rust-SDL2/rust-sdl2#785 but it involves unsafe code.

[Eliah-Lakhin]

@cosmicchipsocket Can you please show an example of your code or a snippet of how you currently use it(in unsafe way)?

[KeyboardDanni]

@Eliah-Lakhin The code is from here: https://github.com/SamP20/HelloTriangle

Basically the window context is wrapped in a stub implementation of Send + Sync so using it on the wrong thread causes a panic.

Most interface libraries only support UI-related code on the main thread so I don't understand why vulkano makes Send + Sync a requirement here.

[Eliah-Lakhin]

@cosmicchipsocket If we remove a requirement of W to be Send+Sync then the Surface itself will not be Send+Sync too(if I not mistaken), and as such a lot of stuff becoming single-threaded too which is contradicting Vulkan design itself which is distinct from the main-threaded OpenGL for example. I agree that the most of the windowing libraries are still single-threaded, but we shouldn't limit Vulkano because of that which is a general-purpose Vulkan wrapper, I think.

Well, this is just my personal point of view. But I agree that this feature makes SDL hard to use in a pure safe way. Probably it would be a good idea to implement an independent crate that will wrap the approach providing in that example, but managing the Surface destruction in a main thread only somehow(i.e. through the Weak references).

[AustinJ235]

I did do some looking into at one point. I believe a couple years ago vulkano didn't have this requirement. Not sure though. ⁣Get BlueMail for Android ​

…

On Sep 28, 2020, 5:35 PM, at 5:35 PM, Ilya Lakhin ***@***.***> wrote: @cosmicchipsocket If we remove a requirement of `W` to be Send+Sync then the Surface itself will not be Send+Sync too(if I not mistaken), and as such a lot of stuff becoming single-threaded too which is contradicting Vulkan design itself which is distinct from a main-threaded OpenGL for example. I agree that the most of the windowing libraries are still single-threaded, but we shouldn't limit Vulkano because of that which is a general-purpose Vulkan wrapper, I think. Well, this is just my personal point of view. But I agree that this feature makes SDL hard to use in a pure safe way. Probably it would be a good idea to implement an independent crate that will wrap the approach providing in that example, but managing the Surface destruction in a main thread only somehow(i.e. through the Weak references). -- You are receiving this because you were mentioned. Reply to this email directly or view it on GitHub: #994 (comment)

[Rua]

The requirement is that W isn't dropped before Surface. That can be achieved, as is currently, by making the latter a member of the former. But there's other ways to achieve it as well.

A possibility I thought of is to have a non-thread-safe owning wrapper around W that contains an atomic variable. A reference to the variable gets passed to Surface and incremented when it is constructed and decremented when it's dropped. When the wrapper is dropped, it checks the atomic and if it's not zero, it blocks the thread waiting for it. Only after that is W dropped. True, it will indefinitely block your main thread if you destroy them in the wrong order, but it removes the unsafety.