Segmentation fault if destroy_shaders is true when calling create_program()

[const-volatile]

If the last parameter (destroy_shaders) to bgfx::create_program(&vsh, &fsh, false) is set to true, I'm running into a segmentation fault; When set to false it is working correctly, but as far as I'm aware the shader objects are not needed anynore after the program is created.

On Windows: "(exit code: 0xc0000005, STATUS_ACCESS_VIOLATION)"
On Linux: "Process finished with exit code 139 (interrupted by signal 11: SIGSEGV)"

The debugger seems to indicate the problem happens in GL_CHECK(glUniform1iv(loc, num, data) ); on line 4427 in renderer_gl.cpp which is called from bgfx_rs::static_lib::frame().

Sample repository: https://github.com/const-volatile/bgfx-imgui-glfw-rs-example
Toolchain: rustc 1.62.0-nightly (52ca603da 2022-04-12)
If changing line 60 here: https://github.com/const-volatile/bgfx-imgui-glfw-rs-example/blob/main/src/imgui_bgfx_renderer/mod.rs the segmentation fault can be observed.

Could this be an issue with bgfx-rs?

[emoon]

Thanks for the report. I will take a look

[emoon]

Ok, I found the issue.

So the way Shader works in bgfx-rs is that when it goes out of scope (in your case here https://github.com/const-volatile/bgfx-imgui-glfw-rs-example/blob/main/src/imgui_bgfx_renderer/mod.rs#L61 ) it will be dropped and the implementation of Drop looks like this

impl Drop for Shader {
    fn drop(&mut self) {
        unsafe {
            bgfx_sys::bgfx_destroy_shader(self.handle);
        }
    }
}

So in bgfx-rs the shader will always be dropped if it's not kept alive. So in this case setting destroy_shaders to false in create_program will be fine as the shaders will get destroyed anyway. Maybe in the Rust wrapper it would make sense to just remove that parameter as Rust scoping of variables can decide if something is alive or not instead.

[const-volatile]

I think the Shader does not go out of scope here https://github.com/const-volatile/bgfx-imgui-glfw-rs-example/blob/main/src/imgui_bgfx_renderer/mod.rs#L61 and hence should not be dropped. Because the return value of bgfx::create_program is returned in the expression and assigned to the variable shader_program.

shader_program: {
                let vsh = bgfx::create_shader(&Memory::copy(get_shader_code!(VS_OCORNUT_IMGUI)));
                let fsh = bgfx::create_shader(&Memory::copy(get_shader_code!(FS_OCORNUT_IMGUI)));
                bgfx::create_program(&vsh, &fsh, false)
            },

(no semicolon on the last line, hence the ownership of the value is transferred to the caller)

The lifetime of shader_program should be for as long as the Renderer object has an owner (which is for the complete execution time).
Or I'm missing something here?

[emoon]

it's vsh and fsh that goes out of scope

[const-volatile]

Oh, I see - That makes sense!
Because they leave scope, they're destroyed anyway.
I agree its not an real issue with bgfx-rs then and can be closed!

[emoon]

Yeah, exactly. So I'm thinking that perhaps removing the last parameter for create_program(...) might make sense for bgfx-rs

[const-volatile]

Right, that might make sense - Thanks a lot!