From: Gabriel Ivăncescu gabrielopcode@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@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; }
From: Gabriel Ivăncescu gabrielopcode@gmail.com
Signed-off-by: Gabriel Ivăncescu gabrielopcode@gmail.com --- dlls/mshtml/navigate.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/dlls/mshtml/navigate.c b/dlls/mshtml/navigate.c index 373cfa03585..f4b45555b31 100644 --- a/dlls/mshtml/navigate.c +++ b/dlls/mshtml/navigate.c @@ -1422,8 +1422,10 @@ static HRESULT async_stop_request(nsChannelBSC *This) }
task = malloc(sizeof(*task)); - if(!task) + if(!task) { + IHTMLWindow2_Release(&window->base.IHTMLWindow2_iface); return E_OUTOFMEMORY; + }
IBindStatusCallback_AddRef(&This->bsc.IBindStatusCallback_iface); task->bsc = This;
From: Gabriel Ivăncescu gabrielopcode@gmail.com
Signed-off-by: Gabriel Ivăncescu gabrielopcode@gmail.com --- dlls/jscript/enumerator.c | 11 +++++++++++ 1 file changed, 11 insertions(+)
diff --git a/dlls/jscript/enumerator.c b/dlls/jscript/enumerator.c index d724d0685c9..bddb177f4a3 100644 --- a/dlls/jscript/enumerator.c +++ b/dlls/jscript/enumerator.c @@ -84,12 +84,23 @@ static void Enumerator_destructor(jsdisp_t *dispex)
TRACE("\n");
+ if(This->enumvar) + IEnumVARIANT_Release(This->enumvar); jsval_release(This->item); free(dispex); }
static HRESULT Enumerator_gc_traverse(struct gc_ctx *gc_ctx, enum gc_traverse_op op, jsdisp_t *dispex) { + EnumeratorInstance *This = enumerator_from_jsdisp(dispex); + + if(op == GC_TRAVERSE_UNLINK) { + IEnumVARIANT *enumvar = This->enumvar; + if(enumvar) { + This->enumvar = NULL; + IEnumVARIANT_Release(enumvar); + } + } return gc_process_linked_val(gc_ctx, op, dispex, &enumerator_from_jsdisp(dispex)->item); }
From: Gabriel Ivăncescu gabrielopcode@gmail.com
Signed-off-by: Gabriel Ivăncescu gabrielopcode@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)) {
From: Gabriel Ivăncescu gabrielopcode@gmail.com
Signed-off-by: Gabriel Ivăncescu gabrielopcode@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;
From: Gabriel Ivăncescu gabrielopcode@gmail.com
It already increases refcount.
Signed-off-by: Gabriel Ivăncescu gabrielopcode@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;
From: Gabriel Ivăncescu gabrielopcode@gmail.com
Signed-off-by: Gabriel Ivăncescu gabrielopcode@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; }
From: Gabriel Ivăncescu gabrielopcode@gmail.com
Signed-off-by: Gabriel Ivăncescu gabrielopcode@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); } }
From: Gabriel Ivăncescu gabrielopcode@gmail.com
Signed-off-by: Gabriel Ivăncescu gabrielopcode@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);
From: Gabriel Ivăncescu gabrielopcode@gmail.com
Signed-off-by: Gabriel Ivăncescu gabrielopcode@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; }
Jacek Caban (@jacek) commented about dlls/mshtml/navigate.c:
} task = malloc(sizeof(*task));
- if(!task)
- if(!task) {
IHTMLWindow2_Release(&window->base.IHTMLWindow2_iface);
You may just move the allocation before acquiring window reference.
Jacek Caban (@jacek) commented about dlls/jscript/enumerator.c:
IEnumVARIANT_Release(This->enumvar);}
static HRESULT Enumerator_gc_traverse(struct gc_ctx *gc_ctx, enum gc_traverse_op op, jsdisp_t *dispex) {
- EnumeratorInstance *This = enumerator_from_jsdisp(dispex);
- if(op == GC_TRAVERSE_UNLINK) {
IEnumVARIANT *enumvar = This->enumvar;if(enumvar) {This->enumvar = NULL;IEnumVARIANT_Release(enumvar);}- }
This is an external reference that does not participate in GC traversal, so there is no point unlinking it here. Releasing it in the destructor should be enough.
On Fri Nov 3 17:41:02 2023 +0000, Jacek Caban wrote:
This is an external reference that does not participate in GC traversal, so there is no point unlinking it here. Releasing it in the destructor should be enough.
Ah sorry, leftover from my jscript CC integration (where it will be useful), I'll move it to there.
-
Gabriel Ivăncescu -
Gabriel Ivăncescu (＠insn)
-
Jacek Caban (＠jacek)