[PATCH v2 0/10] MR4285: jscript/mshtml: More leak fixes.
-- v2: mshtml: Fix BindInfo leak in htmldoc test. jscript/tests: Fix Date object leak in test_default_value. jscript/tests: Fix variant leak after conversion when testing jscript: Fix value arg leak in transform_json_object. jscript: Don't addref before calling jsdisp_propput_name. jscript: Fix array leak in Array.filter. jscript: Fix value leak in Array.shift. jscript: Fix EnumVARIANT leak in enumerators. mshtml: Fix window leak on error in async_stop_request. mshtml: Release the returned lists from Gecko's QuerySelectorAll even https://gitlab.winehq.org/wine/wine/-/merge_requests/4285
From: Gabriel Ivăncescu <gabrielopcode(a)gmail.com> Gecko may return a pseudo-element-list even on error (such as with an invalid selector, for example), which holds refs to elements and leaks. Signed-off-by: Gabriel Ivăncescu <gabrielopcode(a)gmail.com> --- dlls/mshtml/htmldoc.c | 18 ++++++++++++++---- dlls/mshtml/htmlelem.c | 4 +++- dlls/mshtml/script.c | 4 +++- 3 files changed, 20 insertions(+), 6 deletions(-) diff --git a/dlls/mshtml/htmldoc.c b/dlls/mshtml/htmldoc.c index 4daf5ccc58d..540bbbd1626 100644 --- a/dlls/mshtml/htmldoc.c +++ b/dlls/mshtml/htmldoc.c @@ -2685,7 +2685,7 @@ static HRESULT WINAPI HTMLDocument3_getElementsByName(IHTMLDocument3 *iface, BST IHTMLElementCollection **ppelColl) { HTMLDocumentNode *This = impl_from_IHTMLDocument3(iface); - nsIDOMNodeList *node_list; + nsIDOMNodeList *node_list = NULL; nsAString selector_str; WCHAR *selector; nsresult nsres; @@ -2715,6 +2715,8 @@ static HRESULT WINAPI HTMLDocument3_getElementsByName(IHTMLDocument3 *iface, BST free(selector); if(NS_FAILED(nsres)) { ERR("QuerySelectorAll failed: %08lx\n", nsres); + if(node_list) + nsIDOMNodeList_Release(node_list); return E_FAIL; } @@ -2781,12 +2783,15 @@ static HRESULT WINAPI HTMLDocument3_getElementsByTagName(IHTMLDocument3 *iface, return E_UNEXPECTED; } + nslist = NULL; nsAString_InitDepend(&nsstr, v); nsres = nsIDOMDocumentFragment_QuerySelectorAll(docfrag, &nsstr, &nslist); nsAString_Finish(&nsstr); nsIDOMDocumentFragment_Release(docfrag); if(NS_FAILED(nsres)) { ERR("QuerySelectorAll failed: %08lx\n", nsres); + if(nslist) + nsIDOMNodeList_Release(nslist); return E_FAIL; } } @@ -4751,7 +4756,7 @@ static HRESULT WINAPI DocumentSelector_querySelector(IDocumentSelector *iface, B static HRESULT WINAPI DocumentSelector_querySelectorAll(IDocumentSelector *iface, BSTR v, IHTMLDOMChildrenCollection **pel) { HTMLDocumentNode *This = impl_from_IDocumentSelector(iface); - nsIDOMNodeList *node_list; + nsIDOMNodeList *node_list = NULL; nsAString nsstr; nsresult nsres; HRESULT hres; @@ -4773,6 +4778,8 @@ static HRESULT WINAPI DocumentSelector_querySelectorAll(IDocumentSelector *iface if(NS_FAILED(nsres)) { WARN("QuerySelectorAll failed: %08lx\n", nsres); + if(node_list) + nsIDOMNodeList_Release(node_list); return map_nsresult(nsres); } @@ -5946,7 +5953,7 @@ static HRESULT HTMLDocumentNode_next_dispid(DispatchEx *dispex, DISPID id, DISPI { DWORD idx = (id == DISPID_STARTENUM) ? 0 : id - MSHTML_DISPID_CUSTOM_MIN + 1; HTMLDocumentNode *This = impl_from_DispatchEx(dispex); - nsIDOMNodeList *node_list; + nsIDOMNodeList *node_list = NULL; const PRUnichar *name; nsIDOMElement *nselem; nsIDOMNode *nsnode; @@ -5973,8 +5980,11 @@ static HRESULT HTMLDocumentNode_next_dispid(DispatchEx *dispex, DISPID id, DISPI nsAString_InitDepend(&nsstr, L":-moz-any(applet,embed,form,iframe,img,object)[name]"); nsres = nsIDOMHTMLDocument_QuerySelectorAll(This->html_document, &nsstr, &node_list); nsAString_Finish(&nsstr); - if(NS_FAILED(nsres)) + if(NS_FAILED(nsres)) { + if(node_list) + nsIDOMNodeList_Release(node_list); return map_nsresult(nsres); + } for(i = 0, hres = S_OK; SUCCEEDED(hres); i++) { nsres = nsIDOMNodeList_Item(node_list, i, &nsnode); diff --git a/dlls/mshtml/htmlelem.c b/dlls/mshtml/htmlelem.c index cfc84523436..687e62f1784 100644 --- a/dlls/mshtml/htmlelem.c +++ b/dlls/mshtml/htmlelem.c @@ -6476,7 +6476,7 @@ static HRESULT WINAPI ElementSelector_querySelector(IElementSelector *iface, BST static HRESULT WINAPI ElementSelector_querySelectorAll(IElementSelector *iface, BSTR v, IHTMLDOMChildrenCollection **pel) { HTMLElement *This = impl_from_IElementSelector(iface); - nsIDOMNodeList *node_list; + nsIDOMNodeList *node_list = NULL; nsAString nsstr; nsresult nsres; HRESULT hres; @@ -6493,6 +6493,8 @@ static HRESULT WINAPI ElementSelector_querySelectorAll(IElementSelector *iface, nsAString_Finish(&nsstr); if(NS_FAILED(nsres)) { WARN("QuerySelectorAll failed: %08lx\n", nsres); + if(node_list) + nsIDOMNodeList_Release(node_list); return map_nsresult(nsres); } diff --git a/dlls/mshtml/script.c b/dlls/mshtml/script.c index ad44a1ace41..149abcf70e2 100644 --- a/dlls/mshtml/script.c +++ b/dlls/mshtml/script.c @@ -1605,9 +1605,9 @@ void bind_event_scripts(HTMLDocumentNode *doc) { HTMLPluginContainer *plugin_container; nsIDOMHTMLScriptElement *nsscript; + nsIDOMNodeList *node_list = NULL; HTMLScriptElement *script_elem; EventTarget *event_target; - nsIDOMNodeList *node_list; nsIDOMNode *script_node; nsAString selector_str; IDispatch *event_disp; @@ -1626,6 +1626,8 @@ void bind_event_scripts(HTMLDocumentNode *doc) nsAString_Finish(&selector_str); if(NS_FAILED(nsres)) { ERR("QuerySelectorAll failed: %08lx\n", nsres); + if(node_list) + nsIDOMNodeList_Release(node_list); return; } -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/4285
From: Gabriel Ivăncescu <gabrielopcode(a)gmail.com> Signed-off-by: Gabriel Ivăncescu <gabrielopcode(a)gmail.com> --- dlls/mshtml/navigate.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/dlls/mshtml/navigate.c b/dlls/mshtml/navigate.c index 373cfa03585..90ca22ea8b1 100644 --- a/dlls/mshtml/navigate.c +++ b/dlls/mshtml/navigate.c @@ -1415,16 +1415,16 @@ static HRESULT async_stop_request(nsChannelBSC *This) stop_request_task_t *task; HRESULT hres; + task = malloc(sizeof(*task)); + if(!task) + return E_OUTOFMEMORY; + IHTMLWindow2_AddRef(&window->base.IHTMLWindow2_iface); if(!This->bsc.read) { TRACE("No data read, calling OnStartRequest\n"); on_start_nsrequest(This); } - task = malloc(sizeof(*task)); - if(!task) - return E_OUTOFMEMORY; - IBindStatusCallback_AddRef(&This->bsc.IBindStatusCallback_iface); task->bsc = This; -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/4285
From: Gabriel Ivăncescu <gabrielopcode(a)gmail.com> Signed-off-by: Gabriel Ivăncescu <gabrielopcode(a)gmail.com> --- dlls/jscript/enumerator.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/dlls/jscript/enumerator.c b/dlls/jscript/enumerator.c index d724d0685c9..d962c65d229 100644 --- a/dlls/jscript/enumerator.c +++ b/dlls/jscript/enumerator.c @@ -84,6 +84,8 @@ static void Enumerator_destructor(jsdisp_t *dispex) TRACE("\n"); + if(This->enumvar) + IEnumVARIANT_Release(This->enumvar); jsval_release(This->item); free(dispex); } -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/4285
From: Gabriel Ivăncescu <gabrielopcode(a)gmail.com> Signed-off-by: Gabriel Ivăncescu <gabrielopcode(a)gmail.com> --- dlls/jscript/array.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/dlls/jscript/array.c b/dlls/jscript/array.c index 36f45120055..7e5bcaeaefe 100644 --- a/dlls/jscript/array.c +++ b/dlls/jscript/array.c @@ -527,8 +527,10 @@ static HRESULT Array_shift(script_ctx_t *ctx, jsval_t vthis, WORD flags, unsigne hres = jsdisp_get_idx(jsthis, i, &v); if(hres == DISP_E_UNKNOWNNAME) hres = jsdisp_delete_idx(jsthis, i-1); - else if(SUCCEEDED(hres)) + else if(SUCCEEDED(hres)) { hres = jsdisp_propput_idx(jsthis, i-1, v); + jsval_release(v); + } } if(SUCCEEDED(hres)) { -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/4285
From: Gabriel Ivăncescu <gabrielopcode(a)gmail.com> Signed-off-by: Gabriel Ivăncescu <gabrielopcode(a)gmail.com> --- dlls/jscript/array.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/dlls/jscript/array.c b/dlls/jscript/array.c index 7e5bcaeaefe..078e2ce7d4c 100644 --- a/dlls/jscript/array.c +++ b/dlls/jscript/array.c @@ -1152,6 +1152,8 @@ static HRESULT Array_filter(script_ctx_t *ctx, jsval_t vthis, WORD flags, unsign if(r) *r = jsval_obj(arr); + else + jsdisp_release(arr); done: jsdisp_release(jsthis); return hres; -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/4285
From: Gabriel Ivăncescu <gabrielopcode(a)gmail.com> It already increases refcount. Signed-off-by: Gabriel Ivăncescu <gabrielopcode(a)gmail.com> --- dlls/jscript/engine.c | 2 +- dlls/jscript/jsregexp.c | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/dlls/jscript/engine.c b/dlls/jscript/engine.c index 1bf5bb77858..76e6a271bdf 100644 --- a/dlls/jscript/engine.c +++ b/dlls/jscript/engine.c @@ -758,7 +758,7 @@ static HRESULT detach_scope(script_ctx_t *ctx, call_frame_t *frame, scope_chain_ if (scope == frame->base_scope && func->name && func->local_ref == INVALID_LOCAL_REF && ctx->version >= SCRIPTLANGUAGEVERSION_ES5) - jsdisp_propput_name(jsobj, func->name, jsval_obj(jsdisp_addref(frame->function_instance))); + jsdisp_propput_name(jsobj, func->name, jsval_obj(frame->function_instance)); index = scope->scope_index; for(i = 0; i < frame->function->local_scopes[index].locals_cnt; i++) diff --git a/dlls/jscript/jsregexp.c b/dlls/jscript/jsregexp.c index 378dee9bc53..70673ec190e 100644 --- a/dlls/jscript/jsregexp.c +++ b/dlls/jscript/jsregexp.c @@ -390,7 +390,7 @@ static HRESULT create_match_array(script_ctx_t *ctx, jsstr_t *input_str, if(FAILED(hres)) break; - hres = jsdisp_propput_name(array, L"input", jsval_string(jsstr_addref(input_str))); + hres = jsdisp_propput_name(array, L"input", jsval_string(input_str)); if(FAILED(hres)) break; -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/4285
From: Gabriel Ivăncescu <gabrielopcode(a)gmail.com> Signed-off-by: Gabriel Ivăncescu <gabrielopcode(a)gmail.com> --- dlls/jscript/json.c | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/dlls/jscript/json.c b/dlls/jscript/json.c index e07ad863348..277a5e01144 100644 --- a/dlls/jscript/json.c +++ b/dlls/jscript/json.c @@ -295,7 +295,7 @@ static jsval_t transform_json_object(struct transform_json_object_ctx *proc_ctx, if(!obj) { FIXME("non-JS obj in JSON object: %p\n", get_object(args[1])); proc_ctx->hres = E_NOTIMPL; - return jsval_undefined(); + goto ret; }else if(is_class(obj, JSCLASS_ARRAY)) { unsigned i, length = array_get_length(obj); WCHAR buf[14], *buf_end; @@ -306,13 +306,13 @@ static jsval_t transform_json_object(struct transform_json_object_ctx *proc_ctx, str = idx_to_str(i, buf_end); if(!(jsstr = jsstr_alloc(str))) { proc_ctx->hres = E_OUTOFMEMORY; - return jsval_undefined(); + goto ret; } res = transform_json_object(proc_ctx, obj, jsstr); jsstr_release(jsstr); if(is_undefined(res)) { if(FAILED(proc_ctx->hres)) - return jsval_undefined(); + goto ret; if(FAILED(jsdisp_get_id(obj, str, 0, &id))) continue; proc_ctx->hres = disp_delete((IDispatch*)&obj->IDispatchEx_iface, id, &b); @@ -321,7 +321,7 @@ static jsval_t transform_json_object(struct transform_json_object_ctx *proc_ctx, jsval_release(res); } if(FAILED(proc_ctx->hres)) - return jsval_undefined(); + goto ret; } }else { id = DISPID_STARTENUM; @@ -330,7 +330,7 @@ static jsval_t transform_json_object(struct transform_json_object_ctx *proc_ctx, if(proc_ctx->hres == S_FALSE) break; if(FAILED(proc_ctx->hres) || FAILED(proc_ctx->hres = jsdisp_get_prop_name(obj, id, &jsstr))) - return jsval_undefined(); + goto ret; res = transform_json_object(proc_ctx, obj, jsstr); if(is_undefined(res)) { if(SUCCEEDED(proc_ctx->hres)) @@ -344,7 +344,7 @@ static jsval_t transform_json_object(struct transform_json_object_ctx *proc_ctx, } jsstr_release(jsstr); if(FAILED(proc_ctx->hres)) - return jsval_undefined(); + goto ret; } } } @@ -352,6 +352,8 @@ static jsval_t transform_json_object(struct transform_json_object_ctx *proc_ctx, args[0] = jsval_string(name); proc_ctx->hres = disp_call_value(proc_ctx->ctx, proc_ctx->reviver, jsval_obj(holder), DISPATCH_METHOD, ARRAY_SIZE(args), args, &res); +ret: + jsval_release(args[1]); return FAILED(proc_ctx->hres) ? jsval_undefined() : res; } -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/4285
From: Gabriel Ivăncescu <gabrielopcode(a)gmail.com> Signed-off-by: Gabriel Ivăncescu <gabrielopcode(a)gmail.com> --- dlls/jscript/tests/caller.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/dlls/jscript/tests/caller.c b/dlls/jscript/tests/caller.c index 8a1ebb83a33..99b6a21e7fb 100644 --- a/dlls/jscript/tests/caller.c +++ b/dlls/jscript/tests/caller.c @@ -173,6 +173,7 @@ static void test_change_type(IVariantChangeType *change_type, VARIANT *src, cons else { call_change_type(change_type, &v, src, VT_UNKNOWN); ok(V_UNKNOWN(&v) == V_UNKNOWN(src), "V_UNKNOWN(v) != V_UNKNOWN(src)\n"); + VariantClear(&v); } if(V_VT(src) != VT_DISPATCH) @@ -180,6 +181,7 @@ static void test_change_type(IVariantChangeType *change_type, VARIANT *src, cons else { call_change_type(change_type, &v, src, VT_DISPATCH); ok(V_DISPATCH(&v) == V_DISPATCH(src), "V_DISPATCH(v) != V_DISPATCH(src)\n"); + VariantClear(&v); } } -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/4285
From: Gabriel Ivăncescu <gabrielopcode(a)gmail.com> Signed-off-by: Gabriel Ivăncescu <gabrielopcode(a)gmail.com> --- dlls/jscript/tests/run.c | 1 + 1 file changed, 1 insertion(+) diff --git a/dlls/jscript/tests/run.c b/dlls/jscript/tests/run.c index c5d9c47e63c..509f01e3211 100644 --- a/dlls/jscript/tests/run.c +++ b/dlls/jscript/tests/run.c @@ -3069,6 +3069,7 @@ static void test_default_value(void) V_VT(&v) = VT_EMPTY; hres = IDispatch_Invoke(disp, DISPID_VALUE, &IID_NULL, 0, DISPATCH_PROPERTYGET, &dp, &v, NULL, NULL); ok(hres == E_UNEXPECTED, "Invoke failed: %08lx\n", hres); + IDispatch_Release(disp); hres = parse_script_expr(L"new Date()", &v, &script); ok(hres == S_OK, "parse_script_expr failed: %08lx\n", hres); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/4285
From: Gabriel Ivăncescu <gabrielopcode(a)gmail.com> Signed-off-by: Gabriel Ivăncescu <gabrielopcode(a)gmail.com> --- dlls/mshtml/tests/htmldoc.c | 1 + 1 file changed, 1 insertion(+) diff --git a/dlls/mshtml/tests/htmldoc.c b/dlls/mshtml/tests/htmldoc.c index 4db4777dca4..f2ad881df39 100644 --- a/dlls/mshtml/tests/htmldoc.c +++ b/dlls/mshtml/tests/htmldoc.c @@ -836,6 +836,7 @@ static HRESULT WINAPI Protocol_Start(IInternetProtocol *iface, LPCWSTR szUrl, hres = IInternetProtocolSink_ReportResult(pOIProtSink, S_OK, 0, NULL); ok(hres == S_OK, "ReportResult failed: %08lx\n", hres); + ReleaseBindInfo(&bindinfo); return S_OK; } -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/4285
participants (1)
-
Gabriel Ivăncescu