On Tue Jun 10 12:36:47 2025 +0000, Panayiotis Talianos wrote:
I had another look:
- You are right about `param->shared_data`; it should not be `free()`d
while the pool is still in use. It can be accessed next time `d3dx_pool_sync_shared_parameter()` reaches the line `if (!pool->shared_data[i].count)` which will cause a crash. 2. `param->shared_data->data` also should not be `free()`d here. Currently, only the line `pool->shared_data[i].data = param->param.data;` is setting its value. So, `free()`ing that bit of memory should be left to `free_parameter()`. This is called right after `d3dx_pool_release_shared_parameter()` returns. They are both called by `free_top_level_parameter()`, which is also the only function that calls `d3dx_pool_release_shared_parameter()`. So `param->shared_data->data` can be set to `NULL` here. 3. I've noticed, however, that even `free_parameter()` will not set `param->param.data` to `NULL`. The only place I see `d3dx_parameter::data` being set to `NULL` is `param_zero_data_func()` which is only called under certain conditions. That being said, I guess this is a story for another ~~day~~ MR :smile: Looking forward to @Mystral 's input. Good night.
I think Yuxuan is correct. It's surely convoluted, but yeah, over a number of effect creations and releases, table entries could become unused and eventually be reused. The conditions for that to happen are pretty specific (basically, all effects with the same `pool` that have some `shared` parameter are released, then an effect is created on the same `pool` with a new `shared` parameter) but not impossible, as those WineHQ bugs demonstrate...
We do certainly need to set the parameter to `NULL` though.