On Wed Jul 26 11:39:59 2023 +0000, Jacek Caban wrote:
I think that the series could be split better. One way I'd suggest is to introduce a helper like: ``` void destroy_dispatch(DispatchEx *dispex) { dispex->info->desc->vtbl->unlink(disex); dispex->info->desc->vtbl->destroy(dispex); // maybe if (vtbl->destroy) vtbl->destroy() else free() ?? } ``` Then you could convert all objects to call `destroy_dispatch` in their `Release` implementation in a series of self-contained patches. Once all objects are converted, changing it further will require much less invasive module-wide patches. Since we will need a lot of unlinking, maybe a helper like: ``` void unlink(void *ptr) { IUnknown **unk_ptr = ptr, unk = *unk_ptr; *unk_ptr = NULL; if (unk) IUnknown_Release(unk); } ``` would make worth it. Ok, I'll try and see how to split it up with temporary helpers. I'll probably convert this MR just for destruction first, with follow-up MRs doing the unlinking and traversal. I also won't add the cyclic refs (temporary leaks) until later when we actually implement the CC.
In the end, the vtbl won't be optional, which actually simplifies the code *and* makes debugging leaks easier because we can't just "forget" to implement traversal, as it will crash, but that will come later. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/3408#note_40292