search

gfx-rs / gfx_macros

deprecated
Apache License 2.0
4 stars 7 forks source link

Prevent shader_param from drop()ing uninitialized memory #7

Closed AerialX closed 9 years ago

AerialX commented 9 years ago

See #649 and #642 in gfx-rs for more details. The underlying issue is that fill_params is assigning to uninitialized memory on a type that impls Drop (so any shader that uses textures or uniform blocks, since they use Arc to reference a handle).

This fix is a stopgap that assumes that fill_params is only ever meant to be called on an uninitialized/empty ParamStorage. I believe the library works in this fashion - but if it doesn't, it will leak resources. A safer implementation should be considered for the long term.

AerialX commented 9 years ago

And double-checked, that assumption was wrong. Closing, just fix it the right way :)

kvark commented 9 years ago

Thanks for the try! I guess we'll just use the safe path as we did earlier.

AerialX commented 9 years ago

@kvark Yeah, if you have an old safe path, just use it. The approach in this PR is actually fine, you just have to make sure to run out.blocks.clear(); out.textures.clear() at the beginning of fill_params(), or guarantee that they will be empty beforehand. So a stopgap, if bringing a safe approach will take some time to develop.

kvark commented 9 years ago

I see. We do call clear before fill_params, but removing the unsafe code would be better anyway. See #8