[PATCH 0/1] MR10096: mshtml: Fix memory leak in `set_script_elem_readystate()`.
From: Yaroslav Osipov <mcm@etersoft.ru> --- dlls/mshtml/script.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/dlls/mshtml/script.c b/dlls/mshtml/script.c index 786c49990d3..0660110623b 100644 --- a/dlls/mshtml/script.c +++ b/dlls/mshtml/script.c @@ -924,6 +924,9 @@ static void set_script_elem_readystate(HTMLScriptElement *script_elem, READYSTAT hres = push_event_task(&task->header, script_elem->element.node.doc->window, fire_readystatechange_proc, fire_readystatechange_task_destr, script_elem->element.node.doc->window->task_magic); + + free(task); + if(SUCCEEDED(hres)) script_elem->pending_readystatechange_event = TRUE; }else { -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/10096
Jacek Caban (@jacek) commented about dlls/mshtml/script.c:
hres = push_event_task(&task->header, script_elem->element.node.doc->window, fire_readystatechange_proc, fire_readystatechange_task_destr, script_elem->element.node.doc->window->task_magic); + + free(task);
`push_event_task` takes ownership of `task`. It's queued and later used for an asynchronous execution, we can't free it here. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/10096#note_129610
On Fri Feb 13 11:50:04 2026 +0000, Jacek Caban wrote:
`push_event_task` takes ownership of `task`. It's queued and later used for an asynchronous execution, we can't free it here. I see that ownership is taken over `task->header`, not over the `task` itself. The pointer to `task` never leaves this function.
Maybe I miss something. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/10096#note_129629
On Fri Feb 13 15:40:02 2026 +0000, Yaroslav Osipov wrote:
I see that ownership is taken over `task->header`, not over the `task` itself. The pointer to `task` never leaves this function. Maybe I miss something. No, there's no other pointer, it takes the address of header which is basically the address of the entire struct, since the function is generic to all event tasks (the correct one is retrieved in the specified procs).
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/10096#note_129634
On Mon Feb 16 08:28:24 2026 +0000, Gabriel Ivăncescu wrote:
No, there's no other pointer, it takes the address of header which is basically the address of the entire struct, since the function is generic to all event tasks (the correct one is retrieved in the specified procs). Aha, I see it now: `&task` == `&task->header`, and `winegcc` should not reorder fields in a struct by default.
OK, it seems like I should be more careful. Thank you Jacek and Gabriel for the review! -- https://gitlab.winehq.org/wine/wine/-/merge_requests/10096#note_129723
This merge request was closed by Yaroslav Osipov. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/10096
participants (4)
-
Gabriel Ivăncescu (＠insn) -
Jacek Caban (＠jacek)
-
Yaroslav Osipov
-
Yaroslav Osipov (＠mcm)