[PATCH v2 0/8] MR10045: jscript/mshtml: Implement mshtml cycle collection for jscript objects.
This implements mshtml's cycle collection for JS objects so that they can be part of gecko's CC graph and have circular refs dealt with. It only does this during a Full CC, not incremental phases as we can't be part of gecko's "purple buffer" without replacing the entire refcounting logic to gecko's (which doesn't seem desirable as it will be too slow, nor possible, since we still need the current one for normal jscript without gecko). Full CC is forced when doing jscript garbage collection, and otherwise we are ignoring reporting jscript objects during incremental collection. The way it does it now is to fakingly report the current wine-specific-refcount to gecko only when traversing. This is fine when we are doing Full CC without being part of a purple buffer, to integrate existing jscript objects' refcounting into the gecko CC. -- v2: mshtml/tests: Test that the WeakMap can handle external/host objects. mshtml: Unlink the mshtml node from the gecko node during unlink. jscript/mshtml: Traverse external objects that have gecko CC participants jscript: Use get_host_disp in the vtbl when retrieving the host outer jscript: Replace the addref/release methods for host objects with jscript: Get rid of the obj parameter in gc_process_linked_val. jscript: Get rid of the obj parameter in gc_process_linked_obj. jscript: Use no_gc_traverse for host functions and constructors. https://gitlab.winehq.org/wine/wine/-/merge_requests/10045
From: Gabriel Ivăncescu <gabrielopcode@gmail.com> Signed-off-by: Gabriel Ivăncescu <gabrielopcode@gmail.com> --- dlls/jscript/function.c | 14 ++------------ 1 file changed, 2 insertions(+), 12 deletions(-) diff --git a/dlls/jscript/function.c b/dlls/jscript/function.c index f4f9dd57d3b..5ff28e6e9ee 100644 --- a/dlls/jscript/function.c +++ b/dlls/jscript/function.c @@ -1067,17 +1067,12 @@ static void HostFunction_destructor(FunctionInstance *func) { } -static HRESULT HostFunction_gc_traverse(struct gc_ctx *gc_ctx, enum gc_traverse_op op, FunctionInstance *func) -{ - return S_OK; -} - static const function_vtbl_t HostFunctionVtbl = { HostFunction_call, HostFunction_toString, HostFunction_get_code, HostFunction_destructor, - HostFunction_gc_traverse + no_gc_traverse, }; HRESULT create_host_function(script_ctx_t *ctx, const struct property_info *desc, DWORD flags, jsdisp_t **ret) @@ -1202,17 +1197,12 @@ static void HostConstructor_destructor(FunctionInstance *func) { } -static HRESULT HostConstructor_gc_traverse(struct gc_ctx *gc_ctx, enum gc_traverse_op op, FunctionInstance *func) -{ - return S_OK; -} - static const function_vtbl_t HostConstructorVtbl = { HostConstructor_call, HostConstructor_toString, HostConstructor_get_code, HostConstructor_destructor, - HostConstructor_gc_traverse + no_gc_traverse, }; HRESULT init_host_constructor(script_ctx_t *ctx, IWineJSDispatchHost *host_constr, const WCHAR *method_name, IWineJSDispatch **ret) -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/10045
From: Gabriel Ivăncescu <gabrielopcode@gmail.com> Signed-off-by: Gabriel Ivăncescu <gabrielopcode@gmail.com> --- dlls/jscript/arraybuf.c | 4 ++-- dlls/jscript/dispex.c | 2 +- dlls/jscript/engine.c | 4 ++-- dlls/jscript/function.c | 7 +++---- dlls/jscript/jscript.h | 2 +- 5 files changed, 9 insertions(+), 10 deletions(-) diff --git a/dlls/jscript/arraybuf.c b/dlls/jscript/arraybuf.c index d7f004c207e..68af4e97190 100644 --- a/dlls/jscript/arraybuf.c +++ b/dlls/jscript/arraybuf.c @@ -644,7 +644,7 @@ static void DataView_destructor(jsdisp_t *dispex) static HRESULT DataView_gc_traverse(struct gc_ctx *gc_ctx, enum gc_traverse_op op, jsdisp_t *dispex) { DataViewInstance *view = dataview_from_jsdisp(dispex); - return gc_process_linked_obj(gc_ctx, op, dispex, &view->buffer->dispex, (void**)&view->buffer); + return gc_process_linked_obj(gc_ctx, op, &view->buffer->dispex, (void**)&view->buffer); } static const builtin_info_t DataView_info = { @@ -1034,7 +1034,7 @@ static HRESULT TypedArray_fill_props(jsdisp_t *dispex) static HRESULT TypedArray_gc_traverse(struct gc_ctx *gc_ctx, enum gc_traverse_op op, jsdisp_t *dispex) { TypedArrayInstance *typedarr = typedarr_from_jsdisp(dispex); - return gc_process_linked_obj(gc_ctx, op, dispex, &typedarr->buffer->dispex, (void**)&typedarr->buffer); + return gc_process_linked_obj(gc_ctx, op, &typedarr->buffer->dispex, (void**)&typedarr->buffer); } static const builtin_prop_t TypedArrayInst_props[] = { diff --git a/dlls/jscript/dispex.c b/dlls/jscript/dispex.c index 14eb30b03c5..0685eca01fd 100644 --- a/dlls/jscript/dispex.c +++ b/dlls/jscript/dispex.c @@ -1100,7 +1100,7 @@ HRESULT gc_run(script_ctx_t *ctx) return S_OK; } -HRESULT gc_process_linked_obj(struct gc_ctx *gc_ctx, enum gc_traverse_op op, jsdisp_t *obj, jsdisp_t *link, void **unlink_ref) +HRESULT gc_process_linked_obj(struct gc_ctx *gc_ctx, enum gc_traverse_op op, jsdisp_t *link, void **unlink_ref) { if(op == GC_TRAVERSE_UNLINK) { *unlink_ref = NULL; diff --git a/dlls/jscript/engine.c b/dlls/jscript/engine.c index 243b922c8f1..4006d54a058 100644 --- a/dlls/jscript/engine.c +++ b/dlls/jscript/engine.c @@ -538,7 +538,7 @@ static HRESULT scope_gc_traverse(struct gc_ctx *gc_ctx, enum gc_traverse_op op, } if(scope->next) { - hres = gc_process_linked_obj(gc_ctx, op, dispex, &scope->next->dispex, (void**)&scope->next); + hres = gc_process_linked_obj(gc_ctx, op, &scope->next->dispex, (void**)&scope->next); if(FAILED(hres)) return hres; } @@ -552,7 +552,7 @@ static HRESULT scope_gc_traverse(struct gc_ctx *gc_ctx, enum gc_traverse_op op, return S_OK; } - return scope->obj && (jsobj = to_jsdisp(scope->obj)) ? gc_process_linked_obj(gc_ctx, op, dispex, jsobj, (void**)&scope->obj) : S_OK; + return scope->obj && (jsobj = to_jsdisp(scope->obj)) ? gc_process_linked_obj(gc_ctx, op, jsobj, (void**)&scope->obj) : S_OK; } static const builtin_info_t scope_info = { diff --git a/dlls/jscript/function.c b/dlls/jscript/function.c index 5ff28e6e9ee..b9663495e93 100644 --- a/dlls/jscript/function.c +++ b/dlls/jscript/function.c @@ -194,7 +194,7 @@ static HRESULT Arguments_gc_traverse(struct gc_ctx *gc_ctx, enum gc_traverse_op } if(arguments->scope) { - hres = gc_process_linked_obj(gc_ctx, op, jsdisp, &arguments->scope->dispex, (void**)&arguments->scope); + hres = gc_process_linked_obj(gc_ctx, op, &arguments->scope->dispex, (void**)&arguments->scope); if(FAILED(hres)) return hres; } @@ -942,8 +942,7 @@ static HRESULT InterpretedFunction_gc_traverse(struct gc_ctx *gc_ctx, enum gc_tr if(!function->scope_chain) return S_OK; - return gc_process_linked_obj(gc_ctx, op, &function->function.dispex, &function->scope_chain->dispex, - (void**)&function->scope_chain); + return gc_process_linked_obj(gc_ctx, op, &function->scope_chain->dispex, (void**)&function->scope_chain); } static const function_vtbl_t InterpretedFunctionVtbl = { @@ -1311,7 +1310,7 @@ static HRESULT BindFunction_gc_traverse(struct gc_ctx *gc_ctx, enum gc_traverse_ return hres; } - hres = gc_process_linked_obj(gc_ctx, op, &function->function.dispex, &function->target->dispex, (void**)&function->target); + hres = gc_process_linked_obj(gc_ctx, op, &function->target->dispex, (void**)&function->target); if(FAILED(hres)) return hres; diff --git a/dlls/jscript/jscript.h b/dlls/jscript/jscript.h index f484b059bff..2c430b0c9ac 100644 --- a/dlls/jscript/jscript.h +++ b/dlls/jscript/jscript.h @@ -164,7 +164,7 @@ HRESULT create_named_item_script_obj(script_ctx_t*,named_item_t*); named_item_t *lookup_named_item(script_ctx_t*,const WCHAR*,unsigned); void release_named_item(named_item_t*); HRESULT gc_run(script_ctx_t*); -HRESULT gc_process_linked_obj(struct gc_ctx*,enum gc_traverse_op,jsdisp_t*,jsdisp_t*,void**); +HRESULT gc_process_linked_obj(struct gc_ctx*,enum gc_traverse_op,jsdisp_t*,void**); HRESULT gc_process_linked_val(struct gc_ctx*,enum gc_traverse_op,jsdisp_t*,jsval_t*); typedef struct { -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/10045
From: Gabriel Ivăncescu <gabrielopcode@gmail.com> Signed-off-by: Gabriel Ivăncescu <gabrielopcode@gmail.com> --- dlls/jscript/dispex.c | 2 +- dlls/jscript/engine.c | 2 +- dlls/jscript/enumerator.c | 2 +- dlls/jscript/function.c | 6 +++--- dlls/jscript/jscript.h | 2 +- dlls/jscript/jsregexp.c | 2 +- dlls/jscript/set.c | 6 +++--- 7 files changed, 11 insertions(+), 11 deletions(-) diff --git a/dlls/jscript/dispex.c b/dlls/jscript/dispex.c index 0685eca01fd..3538e985e18 100644 --- a/dlls/jscript/dispex.c +++ b/dlls/jscript/dispex.c @@ -1115,7 +1115,7 @@ HRESULT gc_process_linked_obj(struct gc_ctx *gc_ctx, enum gc_traverse_op op, jsd return S_OK; } -HRESULT gc_process_linked_val(struct gc_ctx *gc_ctx, enum gc_traverse_op op, jsdisp_t *obj, jsval_t *link) +HRESULT gc_process_linked_val(struct gc_ctx *gc_ctx, enum gc_traverse_op op, jsval_t *link) { jsdisp_t *jsdisp; diff --git a/dlls/jscript/engine.c b/dlls/jscript/engine.c index 4006d54a058..de5a357c657 100644 --- a/dlls/jscript/engine.c +++ b/dlls/jscript/engine.c @@ -531,7 +531,7 @@ static HRESULT scope_gc_traverse(struct gc_ctx *gc_ctx, enum gc_traverse_op op, unsigned i, cnt = vars->argc; for(i = 0; i < cnt; i++) { - hres = gc_process_linked_val(gc_ctx, op, dispex, &vars->var[i]); + hres = gc_process_linked_val(gc_ctx, op, &vars->var[i]); if(FAILED(hres)) return hres; } diff --git a/dlls/jscript/enumerator.c b/dlls/jscript/enumerator.c index 1f57f6b5173..73835f4418f 100644 --- a/dlls/jscript/enumerator.c +++ b/dlls/jscript/enumerator.c @@ -91,7 +91,7 @@ static void Enumerator_destructor(jsdisp_t *dispex) static HRESULT Enumerator_gc_traverse(struct gc_ctx *gc_ctx, enum gc_traverse_op op, jsdisp_t *dispex) { - return gc_process_linked_val(gc_ctx, op, dispex, &enumerator_from_jsdisp(dispex)->item); + return gc_process_linked_val(gc_ctx, op, &enumerator_from_jsdisp(dispex)->item); } static HRESULT Enumerator_atEnd(script_ctx_t *ctx, jsval_t vthis, WORD flags, unsigned argc, jsval_t *argv, diff --git a/dlls/jscript/function.c b/dlls/jscript/function.c index b9663495e93..5d499ecabec 100644 --- a/dlls/jscript/function.c +++ b/dlls/jscript/function.c @@ -187,7 +187,7 @@ static HRESULT Arguments_gc_traverse(struct gc_ctx *gc_ctx, enum gc_traverse_op if(arguments->buf) { for(i = 0; i < arguments->argc; i++) { - hres = gc_process_linked_val(gc_ctx, op, jsdisp, &arguments->buf[i]); + hres = gc_process_linked_val(gc_ctx, op, &arguments->buf[i]); if(FAILED(hres)) return hres; } @@ -1305,7 +1305,7 @@ static HRESULT BindFunction_gc_traverse(struct gc_ctx *gc_ctx, enum gc_traverse_ unsigned i; for(i = 0; i < function->argc; i++) { - hres = gc_process_linked_val(gc_ctx, op, &function->function.dispex, &function->args[i]); + hres = gc_process_linked_val(gc_ctx, op, &function->args[i]); if(FAILED(hres)) return hres; } @@ -1314,7 +1314,7 @@ static HRESULT BindFunction_gc_traverse(struct gc_ctx *gc_ctx, enum gc_traverse_ if(FAILED(hres)) return hres; - return gc_process_linked_val(gc_ctx, op, &function->function.dispex, &function->this); + return gc_process_linked_val(gc_ctx, op, &function->this); } static const function_vtbl_t BindFunctionVtbl = { diff --git a/dlls/jscript/jscript.h b/dlls/jscript/jscript.h index 2c430b0c9ac..fa460b23b20 100644 --- a/dlls/jscript/jscript.h +++ b/dlls/jscript/jscript.h @@ -165,7 +165,7 @@ named_item_t *lookup_named_item(script_ctx_t*,const WCHAR*,unsigned); void release_named_item(named_item_t*); HRESULT gc_run(script_ctx_t*); HRESULT gc_process_linked_obj(struct gc_ctx*,enum gc_traverse_op,jsdisp_t*,void**); -HRESULT gc_process_linked_val(struct gc_ctx*,enum gc_traverse_op,jsdisp_t*,jsval_t*); +HRESULT gc_process_linked_val(struct gc_ctx*,enum gc_traverse_op,jsval_t*); typedef struct { const WCHAR *name; diff --git a/dlls/jscript/jsregexp.c b/dlls/jscript/jsregexp.c index 3fe781f01a7..01cbdd2aeb9 100644 --- a/dlls/jscript/jsregexp.c +++ b/dlls/jscript/jsregexp.c @@ -559,7 +559,7 @@ static void RegExp_destructor(jsdisp_t *dispex) static HRESULT RegExp_gc_traverse(struct gc_ctx *gc_ctx, enum gc_traverse_op op, jsdisp_t *dispex) { - return gc_process_linked_val(gc_ctx, op, dispex, ®exp_from_jsdisp(dispex)->last_index_val); + return gc_process_linked_val(gc_ctx, op, ®exp_from_jsdisp(dispex)->last_index_val); } static const builtin_prop_t RegExp_props[] = { diff --git a/dlls/jscript/set.c b/dlls/jscript/set.c index c114e3fd6e2..5ab7abe1285 100644 --- a/dlls/jscript/set.c +++ b/dlls/jscript/set.c @@ -375,10 +375,10 @@ static HRESULT Map_gc_traverse(struct gc_ctx *gc_ctx, enum gc_traverse_op op, js } LIST_FOR_EACH_ENTRY(entry, &map->entries, struct jsval_map_entry, list_entry) { - hres = gc_process_linked_val(gc_ctx, op, dispex, &entry->key); + hres = gc_process_linked_val(gc_ctx, op, &entry->key); if(FAILED(hres)) return hres; - hres = gc_process_linked_val(gc_ctx, op, dispex, &entry->value); + hres = gc_process_linked_val(gc_ctx, op, &entry->value); if(FAILED(hres)) return hres; } @@ -825,7 +825,7 @@ static HRESULT WeakMap_gc_traverse(struct gc_ctx *gc_ctx, enum gc_traverse_op op if(op == GC_TRAVERSE && entry->key->gc_marked) continue; - hres = gc_process_linked_val(gc_ctx, op, dispex, &entry->value); + hres = gc_process_linked_val(gc_ctx, op, &entry->value); if(FAILED(hres)) return hres; } -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/10045
From: Gabriel Ivăncescu <gabrielopcode@gmail.com> To remove redundancy since we'll need the host dispatch anyway. Signed-off-by: Gabriel Ivăncescu <gabrielopcode@gmail.com> --- dlls/jscript/dispex.c | 43 +++++++++++++++++------------------------ dlls/jscript/function.c | 27 ++++++++++---------------- dlls/jscript/jscript.h | 3 +-- 3 files changed, 29 insertions(+), 44 deletions(-) diff --git a/dlls/jscript/dispex.c b/dlls/jscript/dispex.c index 3538e985e18..38b21e6a334 100644 --- a/dlls/jscript/dispex.c +++ b/dlls/jscript/dispex.c @@ -947,7 +947,7 @@ HRESULT gc_run(script_ctx_t *ctx) } LIST_FOR_EACH_ENTRY(obj, &thread_data->objects, jsdisp_t, entry) { /* Skip objects with external reference counter */ - if(obj->builtin_info->addref) { + if(obj->builtin_info->get_host_disp) { obj->gc_marked = FALSE; continue; } @@ -1880,8 +1880,8 @@ static void jsdisp_free(jsdisp_t *obj) jsdisp_t *jsdisp_addref(jsdisp_t *obj) { - if(obj->builtin_info->addref) - obj->builtin_info->addref(obj); + if(obj->builtin_info->get_host_disp) + IWineJSDispatchHost_AddRef(obj->builtin_info->get_host_disp(obj)); else ++obj->ref; return obj; @@ -1891,8 +1891,8 @@ ULONG jsdisp_release(jsdisp_t *obj) { ULONG ref; - if(obj->builtin_info->release) - return obj->builtin_info->release(obj); + if(obj->builtin_info->get_host_disp) + return IWineJSDispatchHost_Release(obj->builtin_info->get_host_disp(obj)); ref = --obj->ref; if(!ref) @@ -1934,8 +1934,8 @@ static HRESULT WINAPI DispatchEx_QueryInterface(IWineJSDispatch *iface, REFIID r static ULONG WINAPI DispatchEx_AddRef(IWineJSDispatch *iface) { jsdisp_t *This = impl_from_IWineJSDispatch(iface); - if(This->builtin_info->addref) - return This->builtin_info->addref(This); + if(This->builtin_info->get_host_disp) + return IWineJSDispatchHost_AddRef(This->builtin_info->get_host_disp(This)); jsdisp_addref(This); return This->ref; } @@ -3528,16 +3528,10 @@ static inline HostObject *HostObject_from_jsdisp(jsdisp_t *jsdisp) return CONTAINING_RECORD(jsdisp, HostObject, jsdisp); } -static ULONG HostObject_addref(jsdisp_t *jsdisp) +static IWineJSDispatchHost *HostObject_get_host_disp(jsdisp_t *jsdisp) { HostObject *This = HostObject_from_jsdisp(jsdisp); - return IWineJSDispatchHost_AddRef(This->host_iface); -} - -static ULONG HostObject_release(jsdisp_t *jsdisp) -{ - HostObject *This = HostObject_from_jsdisp(jsdisp); - return IWineJSDispatchHost_Release(This->host_iface); + return This->host_iface; } static HRESULT HostObject_lookup_prop(jsdisp_t *jsdisp, const WCHAR *name, unsigned flags, struct property_info *desc) @@ -3623,16 +3617,15 @@ static HRESULT HostObject_to_string(jsdisp_t *jsdisp, jsstr_t **ret) } static const builtin_info_t HostObject_info = { - .class = JSCLASS_HOST, - .addref = HostObject_addref, - .release = HostObject_release, - .lookup_prop = HostObject_lookup_prop, - .prop_get = HostObject_prop_get, - .prop_put = HostObject_prop_put, - .prop_delete = HostObject_prop_delete, - .prop_config = HostObject_prop_config, - .fill_props = HostObject_fill_props, - .to_string = HostObject_to_string, + .class = JSCLASS_HOST, + .get_host_disp = HostObject_get_host_disp, + .lookup_prop = HostObject_lookup_prop, + .prop_get = HostObject_prop_get, + .prop_put = HostObject_prop_put, + .prop_delete = HostObject_prop_delete, + .prop_config = HostObject_prop_config, + .fill_props = HostObject_fill_props, + .to_string = HostObject_to_string, }; HRESULT init_host_object(script_ctx_t *ctx, IWineJSDispatchHost *host_iface, IWineJSDispatch *prototype_iface, diff --git a/dlls/jscript/function.c b/dlls/jscript/function.c index 5d499ecabec..c6aa0b2f99a 100644 --- a/dlls/jscript/function.c +++ b/dlls/jscript/function.c @@ -1095,16 +1095,10 @@ HRESULT create_host_function(script_ctx_t *ctx, const struct property_info *desc return S_OK; } -static ULONG HostConstructor_addref(jsdisp_t *jsdisp) +static IWineJSDispatchHost *HostConstructor_get_host_disp(jsdisp_t *jsdisp) { HostConstructor *constr = (HostConstructor*)jsdisp; - return IWineJSDispatchHost_AddRef(constr->host_iface); -} - -static ULONG HostConstructor_release(jsdisp_t *jsdisp) -{ - HostConstructor *constr = (HostConstructor*)jsdisp; - return IWineJSDispatchHost_Release(constr->host_iface); + return constr->host_iface; } static HRESULT HostConstructor_lookup_prop(jsdisp_t *jsdisp, const WCHAR *name, unsigned flags, struct property_info *desc) @@ -1116,15 +1110,14 @@ static HRESULT HostConstructor_lookup_prop(jsdisp_t *jsdisp, const WCHAR *name, } static const builtin_info_t HostConstructor_info = { - .class = JSCLASS_FUNCTION, - .addref = HostConstructor_addref, - .release = HostConstructor_release, - .call = Function_value, - .destructor = Function_destructor, - .props_cnt = ARRAY_SIZE(HostFunction_props), - .props = HostFunction_props, - .gc_traverse = Function_gc_traverse, - .lookup_prop = HostConstructor_lookup_prop, + .class = JSCLASS_FUNCTION, + .call = Function_value, + .destructor = Function_destructor, + .props_cnt = ARRAY_SIZE(HostFunction_props), + .props = HostFunction_props, + .get_host_disp = HostConstructor_get_host_disp, + .gc_traverse = Function_gc_traverse, + .lookup_prop = HostConstructor_lookup_prop, }; static HRESULT HostConstructor_call(script_ctx_t *ctx, FunctionInstance *func, jsval_t vthis, unsigned flags, diff --git a/dlls/jscript/jscript.h b/dlls/jscript/jscript.h index fa460b23b20..4e464e9d458 100644 --- a/dlls/jscript/jscript.h +++ b/dlls/jscript/jscript.h @@ -181,8 +181,7 @@ typedef struct { DWORD props_cnt; const builtin_prop_t *props; void (*destructor)(jsdisp_t*); - ULONG (*addref)(jsdisp_t*); - ULONG (*release)(jsdisp_t*); + IWineJSDispatchHost *(*get_host_disp)(jsdisp_t*); void (*on_put)(jsdisp_t*,const WCHAR*); HRESULT (*lookup_prop)(jsdisp_t*,const WCHAR*,unsigned,struct property_info*); HRESULT (*prop_get)(jsdisp_t*,unsigned,jsval_t*); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/10045
From: Gabriel Ivăncescu <gabrielopcode@gmail.com> Signed-off-by: Gabriel Ivăncescu <gabrielopcode@gmail.com> --- dlls/jscript/dispex.c | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/dlls/jscript/dispex.c b/dlls/jscript/dispex.c index 38b21e6a334..75bbb2378d9 100644 --- a/dlls/jscript/dispex.c +++ b/dlls/jscript/dispex.c @@ -3660,16 +3660,12 @@ HRESULT init_host_object(script_ctx_t *ctx, IWineJSDispatchHost *host_iface, IWi IWineJSDispatchHost *get_host_dispatch(IDispatch *disp) { IWineJSDispatchHost *ret; - HostObject *host_obj; jsdisp_t *jsdisp; - if(!(jsdisp = to_jsdisp(disp))) - return NULL; - if(jsdisp->builtin_info != &HostObject_info) + if(!(jsdisp = to_jsdisp(disp)) || !jsdisp->builtin_info->get_host_disp) return NULL; - host_obj = HostObject_from_jsdisp(jsdisp); - IWineJSDispatchHost_GetOuterDispatch(host_obj->host_iface, &ret); + IWineJSDispatchHost_GetOuterDispatch(jsdisp->builtin_info->get_host_disp(jsdisp), &ret); return ret; } -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/10045
From: Gabriel Ivăncescu <gabrielopcode@gmail.com> This makes the jscript GC aware of external object edges by using the CC participant callback APIs, and then figure out cycles with the jscript objects. Note that our GC will never unlink external objects, this is purely for knowing which jscript objects are garbage and need to be cycle collected, and for that it needs an accurate representation of all the edges in the graph (including those from external objects). To do this, we are using the CC participant API of Gecko just like its own Cycle Collector does. We assign a map of all the pure/raw external objects we find and count their speculative refcounts there; same logic as used in the normal GC including the speculative traversal. This is to know which of the objects need to be unmarked from garbage collection (if their refcount is not zero after doing the entire speculative traversal) by flooding all the paths reachable from them. For jscript "host" objects (those that have both a host/external object and a jsdisp), we do traverse the host object's edges on top of the jsdisp edges as normal, but use their own refcount there (from the jsdisp) and do NOT assign them to the map. This is done in the "canonicalize" API so we get the jsdisp itself. For this to work the gc stack chunk is now able to store either a jsdisp pushed on it, or an external object; for the latter, we actually store a pointer to the entry in the map, since it needs both the raw object pointer and the participant (and it wouldn't fit on the stack's chunk). When popping, we determine whether it's a jsdisp or an external object and traverse it appropriately, but other than that, it's mostly the same GC logic as before, just with the ability to traverse external objects. Unlinking in principle is not changed because we only unlink jscript objects, but we do extend the unlink itself so it actually unlinks the whole jscript object (including the external host object). Signed-off-by: Gabriel Ivăncescu <gabrielopcode@gmail.com> --- dlls/jscript/dispex.c | 355 +++++++++++++++++++++++++++++------ dlls/jscript/function.c | 9 +- dlls/jscript/jscript.c | 7 + dlls/jscript/jscript.h | 2 + dlls/jscript/jscript_main.c | 11 ++ dlls/jscript/jsdisp.idl | 29 +++ dlls/mshtml/dispex.c | 141 ++++++++++++++ dlls/mshtml/htmlwindow.c | 31 +++ dlls/mshtml/mshtml_private.h | 8 + dlls/mshtml/nsiface.idl | 42 +++++ dlls/mshtml/script.c | 36 ++-- 11 files changed, 599 insertions(+), 72 deletions(-) diff --git a/dlls/jscript/dispex.c b/dlls/jscript/dispex.c index 75bbb2378d9..67bde2f78fe 100644 --- a/dlls/jscript/dispex.c +++ b/dlls/jscript/dispex.c @@ -21,6 +21,7 @@ #include "jscript.h" #include "engine.h" +#include "wine/rbtree.h" #include "wine/debug.h" WINE_DEFAULT_DEBUG_CHANNEL(jscript); @@ -861,17 +862,37 @@ static void unlink_jsdisp(jsdisp_t *jsdisp) * */ struct gc_stack_chunk { - jsdisp_t *objects[1020]; + void *objects[1020]; struct gc_stack_chunk *prev; }; +struct cc_map_entry { + struct rb_entry entry; + ULONG ref; + struct cc_native_obj obj; +}; + struct gc_ctx { + const struct cc_participant_api *cc_participant_api; struct gc_stack_chunk *chunk; struct gc_stack_chunk *next; unsigned idx; + struct cc_traverse_callback cc_cb; + struct cc_map_entry *cc_map_entry; + struct rb_tree cc_map; }; -static HRESULT gc_stack_push(struct gc_ctx *gc_ctx, jsdisp_t *obj) +static inline struct cc_map_entry *cc_map_entry_from_stack_obj(void *obj) +{ + return CONTAINING_RECORD(obj, struct cc_map_entry, obj.obj); +} + +static inline void *cc_map_entry_to_stack_obj(struct cc_map_entry *entry) +{ + return &entry->obj.obj; +} + +static HRESULT gc_stack_push_impl(struct gc_ctx *gc_ctx, void *obj) { if(!gc_ctx->idx) { if(gc_ctx->next) @@ -891,9 +912,24 @@ static HRESULT gc_stack_push(struct gc_ctx *gc_ctx, jsdisp_t *obj) return S_OK; } -static jsdisp_t *gc_stack_pop(struct gc_ctx *gc_ctx) +static inline HRESULT gc_stack_init(struct gc_ctx *gc_ctx) { - jsdisp_t *obj = gc_ctx->chunk->objects[gc_ctx->idx]; + return gc_stack_push_impl(gc_ctx, NULL); +} + +static inline HRESULT gc_stack_push(struct gc_ctx *gc_ctx, jsdisp_t *obj) +{ + return gc_stack_push_impl(gc_ctx, &obj->IWineJSDispatch_iface); +} + +static inline HRESULT gc_stack_push_cc_obj(struct gc_ctx *gc_ctx, struct cc_map_entry *entry) +{ + return gc_stack_push_impl(gc_ctx, cc_map_entry_to_stack_obj(entry)); +} + +static void *gc_stack_pop(struct gc_ctx *gc_ctx) +{ + void *obj = gc_ctx->chunk->objects[gc_ctx->idx]; if(++gc_ctx->idx == ARRAY_SIZE(gc_ctx->chunk->objects)) { free(gc_ctx->next); @@ -904,6 +940,138 @@ static jsdisp_t *gc_stack_pop(struct gc_ctx *gc_ctx) return obj; } +static inline struct gc_ctx *impl_from_cc_traverse_callback(struct cc_traverse_callback *cb) +{ + return CONTAINING_RECORD(cb, struct gc_ctx, cc_cb); +} + +static HRESULT WINAPI cc_speculative_cb_AdviseRefCount(struct cc_traverse_callback *This, ULONG refcount) +{ + struct gc_ctx *gc_ctx = impl_from_cc_traverse_callback(This); + + /* Add to the existing accumulated (negative) refcount of edges we've already processed */ + if(gc_ctx->cc_map_entry) { + gc_ctx->cc_map_entry->ref += refcount; + gc_ctx->cc_map_entry = NULL; + } + return S_OK; +} + +static HRESULT WINAPI cc_speculative_cb_NoteRawEdge(struct cc_traverse_callback *This, struct cc_native_obj obj) +{ + struct gc_ctx *gc_ctx = impl_from_cc_traverse_callback(This); + struct rb_entry *rb_entry = rb_get(&gc_ctx->cc_map, obj.obj); + struct cc_map_entry *entry; + + if(rb_entry) { + /* Skip traversal if we already visited or will visit it, but do process the edge's ref */ + WINE_RB_ENTRY_VALUE(rb_entry, struct cc_map_entry, entry)->ref--; + return S_OK; + } + + if(!(entry = malloc(sizeof(*entry)))) + return E_OUTOFMEMORY; + entry->obj = obj; + entry->ref = -1; /* account for the edge's ref */ + rb_put(&gc_ctx->cc_map, obj.obj, &entry->entry); + return gc_stack_push_cc_obj(gc_ctx, entry); +} + +static HRESULT WINAPI cc_speculative_cb_NoteEdge(struct cc_traverse_callback *This, IUnknown *obj) +{ + struct gc_ctx *gc_ctx = impl_from_cc_traverse_callback(This); + struct cc_native_obj cc_obj = gc_ctx->cc_participant_api->canonicalize(obj); + jsdisp_t *jsdisp = to_jsdisp(cc_obj.obj); + + /* If it's one of our own jscript objects, process the ref and don't traverse it further */ + if(jsdisp) { + jsdisp->ref--; + return S_OK; + } + + return cc_obj.participant ? cc_speculative_cb_NoteRawEdge(&gc_ctx->cc_cb, cc_obj) : S_OK; +} + +static const struct cc_traverse_callback_vtbl cc_speculative_cb_vtbl = { + cc_speculative_cb_AdviseRefCount, + cc_speculative_cb_NoteEdge, + cc_speculative_cb_NoteRawEdge, +}; + +static HRESULT WINAPI cc_unmark_cb_AdviseRefCount(struct cc_traverse_callback *This, ULONG refcount) +{ + return S_OK; +} + +static HRESULT WINAPI cc_unmark_cb_NoteRawEdge(struct cc_traverse_callback *This, struct cc_native_obj obj) +{ + struct gc_ctx *gc_ctx = impl_from_cc_traverse_callback(This); + struct rb_entry *rb_entry = rb_get(&gc_ctx->cc_map, obj.obj); + struct cc_map_entry *entry; + + assert(rb_entry != NULL); + entry = WINE_RB_ENTRY_VALUE(rb_entry, struct cc_map_entry, entry); + + /* Check participant in the map entry itself, as we use it to mark those we've already visited. */ + return entry->obj.participant ? gc_stack_push_cc_obj(gc_ctx, WINE_RB_ENTRY_VALUE(rb_entry, struct cc_map_entry, entry)) : S_OK; +} + +static HRESULT WINAPI cc_unmark_cb_NoteEdge(struct cc_traverse_callback *This, IUnknown *obj) +{ + struct gc_ctx *gc_ctx = impl_from_cc_traverse_callback(This); + struct cc_native_obj cc_obj = gc_ctx->cc_participant_api->canonicalize(obj); + jsdisp_t *jsdisp = to_jsdisp(cc_obj.obj); + + if(jsdisp) { + /* The temporary refcount itself doesn't matter by this point, just make sure it's not 0, + * as this will unmark it and all accessible objects from it later, since they're alive. */ + jsdisp->ref = 1; + return S_OK; + } + + return cc_obj.participant ? cc_unmark_cb_NoteRawEdge(&gc_ctx->cc_cb, cc_obj) : S_OK; +} + +static const struct cc_traverse_callback_vtbl cc_unmark_cb_vtbl = { + cc_unmark_cb_AdviseRefCount, + cc_unmark_cb_NoteEdge, + cc_unmark_cb_NoteRawEdge, +}; + +static HRESULT WINAPI cc_unmark2_cb_NoteEdge(struct cc_traverse_callback *This, IUnknown *obj) +{ + struct gc_ctx *gc_ctx = impl_from_cc_traverse_callback(This); + struct cc_native_obj cc_obj = gc_ctx->cc_participant_api->canonicalize(obj); + jsdisp_t *jsdisp = to_jsdisp(cc_obj.obj); + + if(jsdisp) + return jsdisp->gc_marked ? gc_stack_push(gc_ctx, jsdisp) : S_OK; + + return cc_obj.participant ? cc_unmark_cb_NoteRawEdge(&gc_ctx->cc_cb, cc_obj) : S_OK; +} + +static const struct cc_traverse_callback_vtbl cc_unmark2_cb_vtbl = { + cc_unmark_cb_AdviseRefCount, + cc_unmark2_cb_NoteEdge, + cc_unmark_cb_NoteRawEdge, +}; + +static inline HRESULT process_edge_speculatively(struct gc_ctx *gc_ctx, IDispatch *edge) +{ + return cc_speculative_cb_NoteEdge(&gc_ctx->cc_cb, (IUnknown*)edge); +} + +static inline HRESULT unmark_edge(struct gc_ctx *gc_ctx, IDispatch *edge) +{ + return cc_unmark2_cb_NoteEdge(&gc_ctx->cc_cb, (IUnknown*)edge); +} + +static int cc_map_compare(const void *k, const struct rb_entry *e) +{ + ULONG_PTR a = (ULONG_PTR)k, b = (ULONG_PTR)RB_ENTRY_VALUE(e, struct cc_map_entry, entry)->obj.obj; + return (a > b) - (a < b); +} + HRESULT gc_run(script_ctx_t *ctx) { /* Save original refcounts in a linked list of chunks */ @@ -913,16 +1081,25 @@ HRESULT gc_run(script_ctx_t *ctx) LONG ref[1020]; } *head, *chunk; struct thread_data *thread_data = ctx->thread_data; - jsdisp_t *obj, *obj2, *link, *link2; + struct cc_map_entry *cc_map_iter, *cc_map_iter2; dispex_prop_t *prop, *props_end; - struct gc_ctx gc_ctx = { 0 }; unsigned chunk_idx = 0; + jsdisp_t *obj, *obj2; + struct gc_ctx gc_ctx; HRESULT hres = S_OK; struct list *iter; + void *stack_obj; /* Prevent recursive calls from side-effects during unlinking (e.g. CollectGarbage from host object's Release) */ if(thread_data->gc_is_unlinking) return S_OK; + gc_ctx.next = NULL; + gc_ctx.idx = 0; + + /* We also scan for non-jscript (host) objects reachable from any jscript object, and store a map of them with their refs */ + gc_ctx.cc_participant_api = thread_data->cc_participant_api; + gc_ctx.cc_cb.vtbl = &cc_speculative_cb_vtbl; + rb_init(&gc_ctx.cc_map, cc_map_compare); if(!(head = malloc(sizeof(*head)))) return E_OUTOFMEMORY; @@ -944,18 +1121,24 @@ HRESULT gc_run(script_ctx_t *ctx) chunk->next = NULL; } chunk->ref[chunk_idx++] = obj->ref; + + /* Obtain actual refcount from host objects with external reference counter */ + if(obj->builtin_info->get_host_disp) + obj->ref = IWineJSDispatchHost_GetRefCount(obj->builtin_info->get_host_disp(obj)); } LIST_FOR_EACH_ENTRY(obj, &thread_data->objects, jsdisp_t, entry) { - /* Skip objects with external reference counter */ - if(obj->builtin_info->get_host_disp) { - obj->gc_marked = FALSE; - continue; - } + hres = gc_stack_init(&gc_ctx); + if(FAILED(hres)) + goto fail_init; + for(prop = obj->props, props_end = prop + obj->prop_cnt; prop < props_end; prop++) { switch(prop->type) { case PROP_JSVAL: - if(is_object_instance(prop->u.val) && (link = to_jsdisp(get_object(prop->u.val)))) - link->ref--; + if(!is_object_instance(prop->u.val)) + break; + hres = process_edge_speculatively(&gc_ctx, get_object(prop->u.val)); + if(FAILED(hres)) + goto fail; break; case PROP_ACCESSOR: if(prop->u.accessor.getter) @@ -972,21 +1155,72 @@ HRESULT gc_run(script_ctx_t *ctx) obj->prototype->ref--; if(obj->builtin_info->gc_traverse) obj->builtin_info->gc_traverse(&gc_ctx, GC_TRAVERSE_SPECULATIVELY, obj); + + for(;;) { + stack_obj = gc_stack_pop(&gc_ctx); + if(!stack_obj) + break; + gc_ctx.cc_map_entry = cc_map_entry_from_stack_obj(stack_obj); + hres = gc_ctx.cc_participant_api->traverse(gc_ctx.cc_map_entry->obj, &gc_ctx.cc_cb); + if(FAILED(hres)) + goto fail; + } + obj->gc_marked = TRUE; } /* 2. Clear mark on objects with non-zero "external refcount" and all objects accessible from them */ + gc_ctx.cc_cb.vtbl = &cc_unmark_cb_vtbl; + RB_FOR_EACH_ENTRY(cc_map_iter, &gc_ctx.cc_map, struct cc_map_entry, entry) { + if(!cc_map_iter->ref || !cc_map_iter->obj.participant) + continue; + + hres = gc_stack_init(&gc_ctx); + if(FAILED(hres)) + goto fail_init; + + /* Traverse and flood all accessible objects from the native objects in the cc map, + * marking already-visited native objects in the map using a NULL participant. */ + stack_obj = cc_map_entry_to_stack_obj(cc_map_iter); + do { + struct cc_native_obj cc_obj = cc_map_entry_from_stack_obj(stack_obj)->obj; + + if(cc_obj.participant) { + cc_map_entry_from_stack_obj(stack_obj)->obj.participant = NULL; + hres = gc_ctx.cc_participant_api->traverse(cc_obj, &gc_ctx.cc_cb); + if(FAILED(hres)) + goto fail; + } + stack_obj = gc_stack_pop(&gc_ctx); + } while(stack_obj); + } + + gc_ctx.cc_cb.vtbl = &cc_unmark2_cb_vtbl; LIST_FOR_EACH_ENTRY(obj, &thread_data->objects, jsdisp_t, entry) { if(!obj->ref || !obj->gc_marked) continue; - hres = gc_stack_push(&gc_ctx, NULL); + hres = gc_stack_init(&gc_ctx); if(FAILED(hres)) - break; + goto fail_init; - obj2 = obj; + stack_obj = &obj->IWineJSDispatch_iface; do { + if(!(obj2 = to_jsdisp(stack_obj))) { + struct cc_native_obj cc_obj = cc_map_entry_from_stack_obj(stack_obj)->obj; + + if(cc_obj.participant) { + cc_map_entry_from_stack_obj(stack_obj)->obj.participant = NULL; + hres = gc_ctx.cc_participant_api->traverse(cc_obj, &gc_ctx.cc_cb); + if(FAILED(hres)) + goto fail; + } + continue; + } + + if(!obj2->gc_marked) + continue; obj2->gc_marked = FALSE; for(prop = obj2->props, props_end = prop + obj2->prop_cnt; prop < props_end; prop++) { @@ -994,41 +1228,37 @@ HRESULT gc_run(script_ctx_t *ctx) case PROP_JSVAL: if(!is_object_instance(prop->u.val)) continue; - link = to_jsdisp(get_object(prop->u.val)); - link2 = NULL; + hres = unmark_edge(&gc_ctx, get_object(prop->u.val)); + if(FAILED(hres)) + goto fail; break; case PROP_ACCESSOR: - link = prop->u.accessor.getter; - link2 = prop->u.accessor.setter; + if(prop->u.accessor.getter && prop->u.accessor.getter->gc_marked) { + hres = gc_stack_push(&gc_ctx, prop->u.accessor.getter); + if(FAILED(hres)) + goto fail; + } + if(prop->u.accessor.setter && prop->u.accessor.setter->gc_marked) { + hres = gc_stack_push(&gc_ctx, prop->u.accessor.setter); + if(FAILED(hres)) + goto fail; + } break; default: continue; } - if(link && link->gc_marked) { - hres = gc_stack_push(&gc_ctx, link); - if(FAILED(hres)) - break; - } - if(link2 && link2->gc_marked) { - hres = gc_stack_push(&gc_ctx, link2); - if(FAILED(hres)) - break; - } } - if(FAILED(hres)) - break; - if(obj2->prototype && obj2->prototype->gc_marked) { hres = gc_stack_push(&gc_ctx, obj2->prototype); if(FAILED(hres)) - break; + goto fail; } if(obj2->builtin_info->gc_traverse) { hres = obj2->builtin_info->gc_traverse(&gc_ctx, GC_TRAVERSE, obj2); if(FAILED(hres)) - break; + goto fail; } /* For weak refs, traverse paths accessible from it via the WeakMaps, if the WeakMaps are alive at this point. @@ -1038,27 +1268,25 @@ HRESULT gc_run(script_ctx_t *ctx) struct weakmap_entry *entry; LIST_FOR_EACH_ENTRY(entry, list, struct weakmap_entry, weak_refs_entry) { - if(!entry->weakmap->gc_marked && is_object_instance(entry->value) && (link = to_jsdisp(get_object(entry->value)))) { - hres = gc_stack_push(&gc_ctx, link); + if(!entry->weakmap->gc_marked && is_object_instance(entry->value)) { + hres = unmark_edge(&gc_ctx, get_object(entry->value)); if(FAILED(hres)) - break; + goto fail; } } - - if(FAILED(hres)) - break; } - - do obj2 = gc_stack_pop(&gc_ctx); while(obj2 && !obj2->gc_marked); - } while(obj2); - - if(FAILED(hres)) { - do obj2 = gc_stack_pop(&gc_ctx); while(obj2); - break; - } + } while((stack_obj = gc_stack_pop(&gc_ctx))); } + +fail: + if(FAILED(hres)) + do stack_obj = gc_stack_pop(&gc_ctx); while(stack_obj); free(gc_ctx.next); +fail_init: + RB_FOR_EACH_ENTRY_DESTRUCTOR(cc_map_iter, cc_map_iter2, &gc_ctx.cc_map, struct cc_map_entry, entry) + free(cc_map_iter); + /* Restore */ chunk = head; chunk_idx = 0; LIST_FOR_EACH_ENTRY(obj, &thread_data->objects, jsdisp_t, entry) { @@ -1117,8 +1345,6 @@ HRESULT gc_process_linked_obj(struct gc_ctx *gc_ctx, enum gc_traverse_op op, jsd HRESULT gc_process_linked_val(struct gc_ctx *gc_ctx, enum gc_traverse_op op, jsval_t *link) { - jsdisp_t *jsdisp; - if(op == GC_TRAVERSE_UNLINK) { jsval_t val = *link; *link = jsval_undefined(); @@ -1126,13 +1352,22 @@ HRESULT gc_process_linked_val(struct gc_ctx *gc_ctx, enum gc_traverse_op op, jsv return S_OK; } - if(!is_object_instance(*link) || !(jsdisp = to_jsdisp(get_object(*link)))) + if(!is_object_instance(*link)) return S_OK; if(op == GC_TRAVERSE_SPECULATIVELY) - jsdisp->ref--; - else if(jsdisp->gc_marked) - return gc_stack_push(gc_ctx, jsdisp); - return S_OK; + return process_edge_speculatively(gc_ctx, get_object(*link)); + return unmark_edge(gc_ctx, get_object(*link)); +} + +HRESULT host_dispatch_gc_traverse(struct gc_ctx *gc_ctx, enum gc_traverse_op op, IWineJSDispatchHost *host_disp) +{ + if(op == GC_TRAVERSE_UNLINK) { + IWineJSDispatchHost_Unlink(host_disp); + return S_OK; + } + + gc_ctx->cc_map_entry = NULL; /* traverse edges but not the refcount on itself as we didn't arrive here from an edge during speculative traversal */ + return IWineJSDispatchHost_Traverse(host_disp, &gc_ctx->cc_cb); } @@ -3616,6 +3851,13 @@ static HRESULT HostObject_to_string(jsdisp_t *jsdisp, jsstr_t **ret) return *ret ? S_OK : E_OUTOFMEMORY; } +static HRESULT HostObject_gc_traverse(struct gc_ctx *gc_ctx, enum gc_traverse_op op, jsdisp_t *jsdisp) +{ + HostObject *This = HostObject_from_jsdisp(jsdisp); + + return host_dispatch_gc_traverse(gc_ctx, op, This->host_iface); +} + static const builtin_info_t HostObject_info = { .class = JSCLASS_HOST, .get_host_disp = HostObject_get_host_disp, @@ -3626,6 +3868,7 @@ static const builtin_info_t HostObject_info = { .prop_config = HostObject_prop_config, .fill_props = HostObject_fill_props, .to_string = HostObject_to_string, + .gc_traverse = HostObject_gc_traverse }; HRESULT init_host_object(script_ctx_t *ctx, IWineJSDispatchHost *host_iface, IWineJSDispatch *prototype_iface, diff --git a/dlls/jscript/function.c b/dlls/jscript/function.c index c6aa0b2f99a..7363670d6b8 100644 --- a/dlls/jscript/function.c +++ b/dlls/jscript/function.c @@ -1189,12 +1189,19 @@ static void HostConstructor_destructor(FunctionInstance *func) { } +static HRESULT HostConstructor_gc_traverse(struct gc_ctx *gc_ctx, enum gc_traverse_op op, FunctionInstance *func) +{ + HostConstructor *constr = (HostConstructor*)func; + + return host_dispatch_gc_traverse(gc_ctx, op, constr->host_iface); +} + static const function_vtbl_t HostConstructorVtbl = { HostConstructor_call, HostConstructor_toString, HostConstructor_get_code, HostConstructor_destructor, - no_gc_traverse, + HostConstructor_gc_traverse, }; HRESULT init_host_constructor(script_ctx_t *ctx, IWineJSDispatchHost *host_constr, const WCHAR *method_name, IWineJSDispatch **ret) diff --git a/dlls/jscript/jscript.c b/dlls/jscript/jscript.c index 02b157749ce..ee881e015e1 100644 --- a/dlls/jscript/jscript.c +++ b/dlls/jscript/jscript.c @@ -1489,6 +1489,12 @@ static HRESULT WINAPI WineJScript_FillGlobals(IWineJScript *iface, IWineJSDispat return fill_globals(This->ctx, script_global); } +static void WINAPI WineJScript_InitCCParticipantAPI(IWineJScript *iface, const struct cc_participant_api *api) +{ + JScript *This = impl_from_IWineJScript(iface); + This->ctx->thread_data->cc_participant_api = api; +} + static const IWineJScriptVtbl WineJScriptVtbl = { WineJScript_QueryInterface, WineJScript_AddRef, @@ -1498,6 +1504,7 @@ static const IWineJScriptVtbl WineJScriptVtbl = { WineJScript_CreateObject, WineJScript_CreateArrayBuffer, WineJScript_FillGlobals, + WineJScript_InitCCParticipantAPI, }; HRESULT create_jscript_object(BOOL is_encode, REFIID riid, void **ppv) diff --git a/dlls/jscript/jscript.h b/dlls/jscript/jscript.h index 4e464e9d458..4a61c33be9f 100644 --- a/dlls/jscript/jscript.h +++ b/dlls/jscript/jscript.h @@ -134,6 +134,7 @@ struct thread_data { BOOL gc_is_unlinking; DWORD gc_last_tick; + const struct cc_participant_api *cc_participant_api; struct list objects; struct rb_tree weak_refs; @@ -166,6 +167,7 @@ void release_named_item(named_item_t*); HRESULT gc_run(script_ctx_t*); HRESULT gc_process_linked_obj(struct gc_ctx*,enum gc_traverse_op,jsdisp_t*,void**); HRESULT gc_process_linked_val(struct gc_ctx*,enum gc_traverse_op,jsval_t*); +HRESULT host_dispatch_gc_traverse(struct gc_ctx*, enum gc_traverse_op, IWineJSDispatchHost*); typedef struct { const WCHAR *name; diff --git a/dlls/jscript/jscript_main.c b/dlls/jscript/jscript_main.c index 6ccab362051..ea6a1579687 100644 --- a/dlls/jscript/jscript_main.c +++ b/dlls/jscript/jscript_main.c @@ -41,6 +41,16 @@ HINSTANCE jscript_hinstance; static DWORD jscript_tls; static ITypeInfo *dispatch_typeinfo; +static struct cc_native_obj WINAPI cc_participant_api_stub_canonicalize(IUnknown *obj) +{ + struct cc_native_obj cc_obj = { .obj = obj, .participant = NULL }; + return cc_obj; +} + +static const struct cc_participant_api cc_participant_api_stub = { + .canonicalize = cc_participant_api_stub_canonicalize, +}; + static int weak_refs_compare(const void *key, const struct rb_entry *entry) { const struct weak_refs_entry *weak_refs_entry = RB_ENTRY_VALUE(entry, const struct weak_refs_entry, entry); @@ -57,6 +67,7 @@ struct thread_data *get_thread_data(void) if(!thread_data) return NULL; thread_data->thread_id = GetCurrentThreadId(); + thread_data->cc_participant_api = &cc_participant_api_stub; list_init(&thread_data->objects); rb_init(&thread_data->weak_refs, weak_refs_compare); TlsSetValue(jscript_tls, thread_data); diff --git a/dlls/jscript/jsdisp.idl b/dlls/jscript/jsdisp.idl index 4783edbfef5..9384be762d4 100644 --- a/dlls/jscript/jsdisp.idl +++ b/dlls/jscript/jsdisp.idl @@ -43,6 +43,31 @@ const unsigned int HOSTOBJ_CONSTRUCTOR = 0x0001; const unsigned int HOSTOBJ_VOLATILE_FILL = 0x0002; const unsigned int HOSTOBJ_VOLATILE_PROPS = 0x0004; +struct cc_native_obj +{ + void *obj; + void *participant; +}; + +struct cc_traverse_callback; +struct cc_traverse_callback_vtbl +{ + HRESULT (__stdcall *AdviseRefCount)(struct cc_traverse_callback *This, ULONG refcount); + HRESULT (__stdcall *NoteEdge)(struct cc_traverse_callback *This, IUnknown *obj); + HRESULT (__stdcall *NoteRawEdge)(struct cc_traverse_callback *This, struct cc_native_obj obj); +}; + +struct cc_traverse_callback +{ + const struct cc_traverse_callback_vtbl *vtbl; +}; + +struct cc_participant_api +{ + struct cc_native_obj (__stdcall *canonicalize)(IUnknown *obj); + HRESULT (__stdcall *traverse)(struct cc_native_obj obj, struct cc_traverse_callback *callback); +}; + interface IWineJSDispatchHost; [ @@ -67,6 +92,7 @@ interface IWineJSDispatch : IDispatchEx ] interface IWineJSDispatchHost : IDispatchEx { + ULONG GetRefCount(); HRESULT GetJSDispatch(IWineJSDispatch **ret); HRESULT LookupProperty(const WCHAR *name, DWORD flags, struct property_info *desc); HRESULT GetProperty(DISPID id, LCID lcid, VARIANT *r, EXCEPINFO *ei, IServiceProvider *caller); @@ -78,6 +104,8 @@ interface IWineJSDispatchHost : IDispatchEx HRESULT FillProperties(); HRESULT GetOuterDispatch(IWineJSDispatchHost **ret); HRESULT ToString(BSTR *str); + HRESULT Traverse(struct cc_traverse_callback *cb); + void Unlink(); } const unsigned int SCRIPTLANGUAGEVERSION_HTML = 0x400; @@ -97,4 +125,5 @@ interface IWineJScript : IUnknown HRESULT CreateObject(IWineJSDispatch **ret); HRESULT CreateArrayBuffer(DWORD size, IWineJSDispatch **ret, void **data); HRESULT FillGlobals(IWineJSDispatchHost *script_global); + void InitCCParticipantAPI(const struct cc_participant_api *api); } diff --git a/dlls/mshtml/dispex.c b/dlls/mshtml/dispex.c index a6b8afb0543..c58aac4b122 100644 --- a/dlls/mshtml/dispex.c +++ b/dlls/mshtml/dispex.c @@ -138,6 +138,8 @@ PRIVATE_TID_LIST #undef XDIID }; +static nsresult NSAPI dispex_unlink(void*); + static HRESULT load_typelib(void) { WCHAR module_path[MAX_PATH + 3]; @@ -2656,6 +2658,13 @@ static HRESULT WINAPI DispatchEx_GetNameSpaceParent(IWineJSDispatchHost *iface, return E_NOTIMPL; } +static ULONG WINAPI JSDispatchHost_GetRefCount(IWineJSDispatchHost *iface) +{ + DispatchEx *This = impl_from_IWineJSDispatchHost(iface); + + return NS_REFCOUNT_VALUE(This->ccref); +} + static HRESULT WINAPI JSDispatchHost_GetJSDispatch(IWineJSDispatchHost *iface, IWineJSDispatch **ret) { DispatchEx *This = impl_from_IWineJSDispatchHost(iface); @@ -2873,6 +2882,21 @@ static HRESULT WINAPI JSDispatchHost_ToString(IWineJSDispatchHost *iface, BSTR * return dispex_to_string(This, str); } +static HRESULT WINAPI JSDispatchHost_Traverse(IWineJSDispatchHost *iface, struct cc_traverse_callback *cb) +{ + DispatchEx *This = impl_from_IWineJSDispatchHost(iface); + struct cc_native_obj obj = { .obj = &This->IWineJSDispatchHost_iface, .participant = &dispex_ccp }; + + return cc_participant_api.traverse(obj, cb); +} + +static void WINAPI JSDispatchHost_Unlink(IWineJSDispatchHost *iface) +{ + DispatchEx *This = impl_from_IWineJSDispatchHost(iface); + + dispex_unlink(&This->IWineJSDispatchHost_iface); +} + static IWineJSDispatchHostVtbl JSDispatchHostVtbl = { DispatchEx_QueryInterface, DispatchEx_AddRef, @@ -2889,6 +2913,7 @@ static IWineJSDispatchHostVtbl JSDispatchHostVtbl = { DispatchEx_GetMemberName, DispatchEx_GetNextDispID, DispatchEx_GetNameSpaceParent, + JSDispatchHost_GetRefCount, JSDispatchHost_GetJSDispatch, JSDispatchHost_LookupProperty, JSDispatchHost_GetProperty, @@ -2900,6 +2925,122 @@ static IWineJSDispatchHostVtbl JSDispatchHostVtbl = { JSDispatchHost_FillProperties, JSDispatchHost_GetOuterDispatch, JSDispatchHost_ToString, + JSDispatchHost_Traverse, + JSDispatchHost_Unlink +}; + +static DispatchEx *unsafe_impl_from_IWineJSDispatchHost(IWineJSDispatchHost *iface) +{ + return iface->lpVtbl == &JSDispatchHostVtbl ? impl_from_IWineJSDispatchHost(iface) : NULL; +} + +static struct cc_native_obj WINAPI cc_participant_api_canonicalize(IUnknown *obj) +{ + struct cc_native_obj cc_obj = { .obj = obj, .participant = NULL }; + DispatchEx *dispex = NULL; + IUnknown *unk; + + /* These QI do not AddRef, so they are scan-safe */ + if(IUnknown_QueryInterface(obj, &IID_nsCycleCollectionISupports, (void**)&unk) != S_OK) + return cc_obj; + + /* We can't QI here because we can't touch the refcount and that would add a ref, so inspect vtbl directly */ + if(!(dispex = unsafe_impl_from_IWineJSDispatchHost((IWineJSDispatchHost*)unk))) { + HTMLOuterWindow *outer_window = unsafe_HTMLOuterWindow_from_IHTMLWindow2((IHTMLWindow2*)unk); + + if(outer_window && outer_window->base.inner_window) + dispex = &outer_window->base.inner_window->event_target.dispex; + } + + if(dispex && dispex->jsdisp) { + cc_obj.obj = dispex->jsdisp; + return cc_obj; + } + + cc_obj.obj = unk; + IUnknown_QueryInterface(unk, &IID_nsXPCOMCycleCollectionParticipant, (void**)&cc_obj.participant); + return cc_obj; +} + +struct cc_callback_ctx { + struct cc_traverse_callback *callback; + nsCycleCollectionTraversalCallback cb; + HRESULT hres; +}; + +static inline struct cc_callback_ctx *impl_from_nsCycleCollectionTraversalCallback(nsCycleCollectionTraversalCallback *cb) +{ + return CONTAINING_RECORD(cb, struct cc_callback_ctx, cb); +} + +static void NSAPI cc_callback_DescribeRefCountedNode(nsCycleCollectionTraversalCallback *This, nsrefcnt refcount, const char *objName) +{ + struct cc_callback_ctx *ctx = impl_from_nsCycleCollectionTraversalCallback(This); + if(SUCCEEDED(ctx->hres)) + ctx->hres = ctx->callback->vtbl->AdviseRefCount(ctx->callback, refcount); +} + +static void NSAPI cc_callback_DescribeGCedNode(nsCycleCollectionTraversalCallback *This, unsigned char isMarked, const char *objName, UINT64 compartmentAddress) +{ +} + +static void NSAPI cc_callback_NoteXPCOMChild(nsCycleCollectionTraversalCallback *This, nsISupports *child) +{ + struct cc_callback_ctx *ctx = impl_from_nsCycleCollectionTraversalCallback(This); + if(ctx->hres == S_OK && child) + ctx->hres = ctx->callback->vtbl->NoteEdge(ctx->callback, (IUnknown*)child); +} + +static void NSAPI cc_callback_NoteJSObject(nsCycleCollectionTraversalCallback *This, void *child) +{ + /* FIXME: somehow obtain the participant and traverse JS objects */ +} + +static void NSAPI cc_callback_NoteJSScript(nsCycleCollectionTraversalCallback *This, void *child) +{ +} + +static void NSAPI cc_callback_NoteNativeChild(nsCycleCollectionTraversalCallback *This, void *child, nsCycleCollectionParticipant *participant) +{ + struct cc_callback_ctx *ctx = impl_from_nsCycleCollectionTraversalCallback(This); + if(ctx->hres == S_OK && child) { + struct cc_native_obj cc_obj = { .obj = child, .participant = participant }; + ctx->hres = ctx->callback->vtbl->NoteRawEdge(ctx->callback, cc_obj); + } +} + +static void NSAPI cc_callback_NoteNextEdgeName(nsCycleCollectionTraversalCallback *This, const char *name) +{ +} + +static const nsCycleCollectionTraversalCallbackVtbl cc_callback_vtbl = { + cc_callback_DescribeRefCountedNode, + cc_callback_DescribeGCedNode, + cc_callback_NoteXPCOMChild, + cc_callback_NoteJSObject, + cc_callback_NoteJSScript, + cc_callback_NoteNativeChild, + cc_callback_NoteNextEdgeName +}; + +static HRESULT WINAPI cc_participant_api_traverse(struct cc_native_obj obj, struct cc_traverse_callback *callback) +{ + nsCycleCollectionParticipant *participant = obj.participant; + struct cc_callback_ctx ctx; + nsresult nsres; + + ctx.cb.lpVtbl = &cc_callback_vtbl; + ctx.cb.flags = nsCycleCollectionTraversalCallback_WANT_ALL_TRACES; + ctx.callback = callback; + ctx.hres = S_FALSE; + + nsres = participant->lpVtbl->Traverse(participant, obj.obj, &ctx.cb); + return NS_FAILED(nsres) ? map_nsresult(nsres) : ctx.hres; +} + +const struct cc_participant_api cc_participant_api = { + cc_participant_api_canonicalize, + cc_participant_api_traverse, }; struct EnumVARIANT { diff --git a/dlls/mshtml/htmlwindow.c b/dlls/mshtml/htmlwindow.c index 1977c6137ba..c84834b2847 100644 --- a/dlls/mshtml/htmlwindow.c +++ b/dlls/mshtml/htmlwindow.c @@ -47,6 +47,7 @@ WINE_DEFAULT_DEBUG_CHANNEL(mshtml); static ExternalCycleCollectionParticipant outer_window_ccp; +static nsresult NSAPI outer_window_unlink(void*); static int window_map_compare(const void *key, const struct wine_rb_entry *entry) { @@ -1585,6 +1586,11 @@ static const IHTMLWindow2Vtbl outer_window_HTMLWindow2Vtbl = { HTMLWindow2_get_external }; +HTMLOuterWindow *unsafe_HTMLOuterWindow_from_IHTMLWindow2(IHTMLWindow2 *iface) +{ + return iface->lpVtbl == &outer_window_HTMLWindow2Vtbl ? HTMLOuterWindow_from_IHTMLWindow2(iface) : NULL; +} + static inline HTMLWindow *impl_from_IHTMLWindow3(IHTMLWindow3 *iface) { return CONTAINING_RECORD(iface, HTMLWindow, IHTMLWindow3_iface); @@ -3462,6 +3468,13 @@ static HRESULT WINAPI WindowDispEx_GetNameSpaceParent(IWineJSDispatchHost *iface return S_OK; } +static ULONG WINAPI WindowDispEx_GetRefCount(IWineJSDispatchHost *iface) +{ + HTMLOuterWindow *This = impl_from_IWineJSDispatchHost(iface); + + return NS_REFCOUNT_VALUE(This->ccref); +} + static HRESULT WINAPI WindowDispEx_GetJSDispatch(IWineJSDispatchHost *iface, IWineJSDispatch **ret) { HTMLOuterWindow *This = impl_from_IWineJSDispatchHost(iface); @@ -3551,6 +3564,21 @@ static HRESULT WINAPI WindowDispEx_ToString(IWineJSDispatchHost *iface, BSTR *st return IWineJSDispatchHost_ToString(&This->base.inner_window->event_target.dispex.IWineJSDispatchHost_iface, str); } +static HRESULT WINAPI WindowDispEx_Traverse(IWineJSDispatchHost *iface, struct cc_traverse_callback *cb) +{ + HTMLOuterWindow *This = impl_from_IWineJSDispatchHost(iface); + struct cc_native_obj obj = { .obj = &This->base.IHTMLWindow2_iface, .participant = &outer_window_ccp }; + + return cc_participant_api.traverse(obj, cb); +} + +static void WINAPI WindowDispEx_Unlink(IWineJSDispatchHost *iface) +{ + HTMLOuterWindow *This = impl_from_IWineJSDispatchHost(iface); + + outer_window_unlink(&This->base.IHTMLWindow2_iface); +} + static const IWineJSDispatchHostVtbl WindowDispExVtbl = { WindowDispEx_QueryInterface, WindowDispEx_AddRef, @@ -3567,6 +3595,7 @@ static const IWineJSDispatchHostVtbl WindowDispExVtbl = { WindowDispEx_GetMemberName, WindowDispEx_GetNextDispID, WindowDispEx_GetNameSpaceParent, + WindowDispEx_GetRefCount, WindowDispEx_GetJSDispatch, WindowDispEx_LookupProperty, WindowDispEx_GetProperty, @@ -3578,6 +3607,8 @@ static const IWineJSDispatchHostVtbl WindowDispExVtbl = { WindowDispEx_FillProperties, WindowDispEx_GetOuterDispatch, WindowDispEx_ToString, + WindowDispEx_Traverse, + WindowDispEx_Unlink }; static inline HTMLOuterWindow *impl_from_IEventTarget(IEventTarget *iface) diff --git a/dlls/mshtml/mshtml_private.h b/dlls/mshtml/mshtml_private.h index e5236ed5f03..0bac63e90aa 100644 --- a/dlls/mshtml/mshtml_private.h +++ b/dlls/mshtml/mshtml_private.h @@ -377,6 +377,11 @@ typedef struct { UINT_PTR x; } nsCycleCollectingAutoRefCnt; +static inline UINT_PTR NS_REFCOUNT_VALUE(nsCycleCollectingAutoRefCnt ref) +{ + return ref.x >> 2; +} + /* dispex is our base IDispatchEx implementation for all mshtml objects, and the vtbl allows customizing the behavior depending on the object. Objects have basically 3 types of props: @@ -646,6 +651,7 @@ extern void (__cdecl *ccref_init)(nsCycleCollectingAutoRefCnt*,nsrefcnt); extern void (__cdecl *ccp_init)(ExternalCycleCollectionParticipant*,const CCObjCallback*); extern void (__cdecl *describe_cc_node)(nsCycleCollectingAutoRefCnt*,const char*,nsCycleCollectionTraversalCallback*); extern void (__cdecl *note_cc_edge)(nsISupports*,const char*,nsCycleCollectionTraversalCallback*); +extern const struct cc_participant_api cc_participant_api; void init_dispatch(DispatchEx*,dispex_static_data_t*,HTMLInnerWindow*,compat_mode_t); void init_dispatch_from_desc(DispatchEx*,dispex_data_t*,HTMLInnerWindow*,DispatchEx*); @@ -795,6 +801,8 @@ struct HTMLOuterWindow { struct wine_rb_entry entry; }; +HTMLOuterWindow *unsafe_HTMLOuterWindow_from_IHTMLWindow2(IHTMLWindow2*); + struct HTMLInnerWindow { HTMLWindow base; EventTarget event_target; diff --git a/dlls/mshtml/nsiface.idl b/dlls/mshtml/nsiface.idl index 8bba7d9f83b..586e4919ae6 100644 --- a/dlls/mshtml/nsiface.idl +++ b/dlls/mshtml/nsiface.idl @@ -208,6 +208,48 @@ typedef void *JSContext; typedef void *JSObject; typedef uint64_t jsval; +typedef struct nsCycleCollectionTraversalCallback nsCycleCollectionTraversalCallback; +typedef struct nsCycleCollectionParticipant nsCycleCollectionParticipant; + +typedef struct nsCycleCollectionParticipantVtbl +{ + nsresult (__stdcall *Traverse)(nsCycleCollectionParticipant *This, void *p, nsCycleCollectionTraversalCallback *cb); + void (__stdcall *Root)(nsCycleCollectionParticipant *This, void *p); + void (__stdcall *Unlink)(nsCycleCollectionParticipant *This, void *p); + void (__stdcall *Unroot)(nsCycleCollectionParticipant *This, void *p); + void (__stdcall *Trace)(nsCycleCollectionParticipant *This, void *p, const void /*TraceCallbacks*/ *cb, void *closure); + void (__stdcall *DeleteCycleCollectable)(nsCycleCollectionParticipant *This, void *p); + bool (__stdcall *CanSkipReal)(nsCycleCollectionParticipant *This, bool removingAllowed); + bool (__stdcall *CanSkipInCCReal)(nsCycleCollectionParticipant *This, void *p); + bool (__stdcall *CanSkipThisReal)(nsCycleCollectionParticipant *This, void *p); +} nsCycleCollectionParticipantVtbl; + +struct nsCycleCollectionParticipant +{ + const nsCycleCollectionParticipantVtbl *lpVtbl; + bool mightSkip; +}; + +typedef struct nsCycleCollectionTraversalCallbackVtbl +{ + void (__stdcall *DescribeRefCountedNode)(nsCycleCollectionTraversalCallback *This, nsrefcnt refcount, const char *objName); + void (__stdcall *DescribeGCedNode)(nsCycleCollectionTraversalCallback *This, bool isMarked, const char *objName, uint64_t compartmentAddress); + void (__stdcall *NoteXPCOMChild)(nsCycleCollectionTraversalCallback *This, nsISupports *child); + void (__stdcall *NoteJSObject)(nsCycleCollectionTraversalCallback *This, void /*JSObject*/ *child); + void (__stdcall *NoteJSScript)(nsCycleCollectionTraversalCallback *This, void /*JSScript*/ *child); + void (__stdcall *NoteNativeChild)(nsCycleCollectionTraversalCallback *This, void *child, nsCycleCollectionParticipant *participant); + void (__stdcall *NoteNextEdgeName)(nsCycleCollectionTraversalCallback *This, const char *name); +} nsCycleCollectionTraversalCallbackVtbl; + +struct nsCycleCollectionTraversalCallback +{ + const nsCycleCollectionTraversalCallbackVtbl *lpVtbl; + uint32_t flags; +}; + +const unsigned int nsCycleCollectionTraversalCallback_WANT_DEBUG_INFO = 1; +const unsigned int nsCycleCollectionTraversalCallback_WANT_ALL_TRACES = 2; + [ object, uuid(8bb35ed9-e332-462d-9155-4a002ab5c958), diff --git a/dlls/mshtml/script.c b/dlls/mshtml/script.c index 786c49990d3..85dd6b7ca49 100644 --- a/dlls/mshtml/script.c +++ b/dlls/mshtml/script.c @@ -204,27 +204,33 @@ static BOOL init_script_engine(ScriptHost *script_host, IActiveScript *script) return FALSE; } - if(compat_mode >= COMPAT_MODE_IE9 && IsEqualGUID(&CLSID_JScript, &script_host->guid)) { + if(IsEqualGUID(&CLSID_JScript, &script_host->guid)) { IWineJScript *jscript; hres = IActiveScript_QueryInterface(script, &IID_IWineJScript, (void **)&jscript); if(SUCCEEDED(hres)) { - DispatchEx *prototype; + IWineJScript_InitCCParticipantAPI(jscript, &cc_participant_api); - assert(!script_host->window->jscript); - assert(!script_host->window->event_target.dispex.jsdisp); - script_host->window->jscript = jscript; + if(compat_mode < COMPAT_MODE_IE9) + IWineJScript_Release(jscript); + else { + DispatchEx *prototype; - hres = get_prototype(script_host->window, OBJID_Window, &prototype); - if(SUCCEEDED(hres)) - hres = IWineJScript_InitHostObject(jscript, - &script_host->window->event_target.dispex.IWineJSDispatchHost_iface, - prototype->jsdisp, object_descriptors[OBJID_Window]->js_flags, - &script_host->window->event_target.dispex.jsdisp); - if(FAILED(hres)) - ERR("Could not initialize script global: %08lx\n", hres); + assert(!script_host->window->jscript); + assert(!script_host->window->event_target.dispex.jsdisp); + script_host->window->jscript = jscript; - /* make sure that script global is fully initialized */ - dispex_compat_mode(&script_host->window->event_target.dispex); + hres = get_prototype(script_host->window, OBJID_Window, &prototype); + if(SUCCEEDED(hres)) + hres = IWineJScript_InitHostObject(jscript, + &script_host->window->event_target.dispex.IWineJSDispatchHost_iface, + prototype->jsdisp, object_descriptors[OBJID_Window]->js_flags, + &script_host->window->event_target.dispex.jsdisp); + if(FAILED(hres)) + ERR("Could not initialize script global: %08lx\n", hres); + + /* make sure that script global is fully initialized */ + dispex_compat_mode(&script_host->window->event_target.dispex); + } }else { ERR("Could not get IWineJScript, don't use native jscript.dll\n"); } -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/10045
From: Gabriel Ivăncescu <gabrielopcode@gmail.com> This allows it to be collected immediately instead of waiting for the next cycle collection, since it can now get unlinked during our jscript GC instead. Signed-off-by: Gabriel Ivăncescu <gabrielopcode@gmail.com> --- dlls/mshtml/htmlnode.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/dlls/mshtml/htmlnode.c b/dlls/mshtml/htmlnode.c index b82ab5710de..8dcbc21c09f 100644 --- a/dlls/mshtml/htmlnode.c +++ b/dlls/mshtml/htmlnode.c @@ -1143,7 +1143,10 @@ void HTMLDOMNode_unlink(DispatchEx *dispex) HTMLDOMNode *This = HTMLDOMNode_from_DispatchEx(dispex); release_event_target(&This->event_target); - unlink_ref(&This->nsnode); + if(This->nsnode) { + nsIDOMNode_SetMshtmlNode(This->nsnode, NULL); + unlink_ref(&This->nsnode); + } if(This->doc) { HTMLDocumentNode *doc = This->doc; -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/10045
From: Gabriel Ivăncescu <gabrielopcode@gmail.com> Signed-off-by: Gabriel Ivăncescu <gabrielopcode@gmail.com> --- dlls/mshtml/tests/documentmode.js | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/dlls/mshtml/tests/documentmode.js b/dlls/mshtml/tests/documentmode.js index 9d4985fcbc8..ffafc6b4ce6 100644 --- a/dlls/mshtml/tests/documentmode.js +++ b/dlls/mshtml/tests/documentmode.js @@ -2523,7 +2523,9 @@ async_test("weakmap_obj", function() { r = external.newRefTest(); ok(r.ref === 1, "wrong ref after newRefTest: " + r.ref); - o = { val: r.get(), map: s }; + o = document.createElement("div"); + o.val = r.get(); + o.map = s; s.set(o, o); ok(r.ref > 1, "map entry released"); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/10045
Ok I basically rewrote this because the old way, on top of being hacky, was also impossible to fix for WeakMaps (I've added WeakMap test at the end of the MR). I think I gave enough info in the big commit, but to summarize: now it uses our jscript garbage collector to do the whole job. The reason it works is because it now looks at mshtml/gecko object edges so it can build the full graph and detect cycles (well, not exactly full since it can't look into gecko JS objects, because it doesn't supply us with the participant; with wine-gecko changes, that can be solved though). This means WeakMaps will work too. Even though it doesn't traverse gecko JS objects, it still fixes all leaks in the tests (except the enumerator which is split into another MR cause this one is too big already, but with that + some local hack it has 0 object leaks). Presumably that "some local hack" won't be needed when we can traverse gecko JS objects (and anyway it's not a big deal because it's mostly useful when checking tests for leaks, otherwise it will just have to delay the collection until gecko cleans the JS objects and removes their edge refs). Anyway I'm relieved that it ended up without being a dead end because it does clean all leaks and cycles. Now I realize maybe we should export it as a proper wine-gecko API, but I want to know if it's a good way forward first or maybe it's suitable like this? -- https://gitlab.winehq.org/wine/wine/-/merge_requests/10045#note_149186
Jacek Caban (@jacek) commented about dlls/jscript/jsdisp.idl:
+ void *obj; + void *participant; +}; + +struct cc_traverse_callback; +struct cc_traverse_callback_vtbl +{ + HRESULT (__stdcall *AdviseRefCount)(struct cc_traverse_callback *This, ULONG refcount); + HRESULT (__stdcall *NoteEdge)(struct cc_traverse_callback *This, IUnknown *obj); + HRESULT (__stdcall *NoteRawEdge)(struct cc_traverse_callback *This, struct cc_native_obj obj); +}; + +struct cc_traverse_callback +{ + const struct cc_traverse_callback_vtbl *vtbl; +}; Since we use IDL file anyway, why not to make it an interface?
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/10045#note_149714
Jacek Caban (@jacek) commented about dlls/mshtml/dispex.c:
+ return iface->lpVtbl == &JSDispatchHostVtbl ? impl_from_IWineJSDispatchHost(iface) : NULL; +} + +static struct cc_native_obj WINAPI cc_participant_api_canonicalize(IUnknown *obj) +{ + struct cc_native_obj cc_obj = { .obj = obj, .participant = NULL }; + DispatchEx *dispex = NULL; + IUnknown *unk; + + /* These QI do not AddRef, so they are scan-safe */ + if(IUnknown_QueryInterface(obj, &IID_nsCycleCollectionISupports, (void**)&unk) != S_OK) + return cc_obj; + + /* We can't QI here because we can't touch the refcount and that would add a ref, so inspect vtbl directly */ + if(!(dispex = unsafe_impl_from_IWineJSDispatchHost((IWineJSDispatchHost*)unk))) { + HTMLOuterWindow *outer_window = unsafe_HTMLOuterWindow_from_IHTMLWindow2((IHTMLWindow2*)unk); This is not great... Could we use the same mechanism as Gecko does and have separate participants for them? They could then just perform a safe cast and call some common implementation on that.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/10045#note_149715
Jacek Caban (@jacek) commented about dlls/mshtml/nsiface.idl:
+typedef struct nsCycleCollectionTraversalCallback nsCycleCollectionTraversalCallback; +typedef struct nsCycleCollectionParticipant nsCycleCollectionParticipant; + +typedef struct nsCycleCollectionParticipantVtbl +{ + nsresult (__stdcall *Traverse)(nsCycleCollectionParticipant *This, void *p, nsCycleCollectionTraversalCallback *cb); + void (__stdcall *Root)(nsCycleCollectionParticipant *This, void *p); + void (__stdcall *Unlink)(nsCycleCollectionParticipant *This, void *p); + void (__stdcall *Unroot)(nsCycleCollectionParticipant *This, void *p); + void (__stdcall *Trace)(nsCycleCollectionParticipant *This, void *p, const void /*TraceCallbacks*/ *cb, void *closure); + void (__stdcall *DeleteCycleCollectable)(nsCycleCollectionParticipant *This, void *p); + bool (__stdcall *CanSkipReal)(nsCycleCollectionParticipant *This, bool removingAllowed); + bool (__stdcall *CanSkipInCCReal)(nsCycleCollectionParticipant *This, void *p); + bool (__stdcall *CanSkipThisReal)(nsCycleCollectionParticipant *This, void *p); +} nsCycleCollectionParticipantVtbl;
A similar question, can it be an interface? -- https://gitlab.winehq.org/wine/wine/-/merge_requests/10045#note_149716
This looks better overall, thanks. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/10045#note_149717
On Mon Aug 24 17:39:58 2026 +0000, Jacek Caban wrote:
Since we use IDL file anyway, why not to make it an interface? I thought an interface has to follow COM rules and implement refcounting and QI/IUnknown, which seems way overkill for a simple callback. Is that not the case? IMO it would add a lot more boilerplate but if that's what you prefer I can go with it.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/10045#note_149756
On Mon Aug 24 17:39:58 2026 +0000, Jacek Caban wrote:
This is not great... Could we use the same mechanism as Gecko does and have separate participants for them? They could then just perform a safe cast and call some common implementation on that. Sorry I don't quite understand what you mean. In the generic case, this code basically retrieves a participant and an object from an input raw object. Raw object meaning it can be anything (even pure gecko external object), not ours.
But at the same time, it should return the jsdisp if it's a host object with a jsdisp, so that the caller (our jscript) can special case it. This is necessary because we don't place jsdisps into the "cc map", even if they're host objects (and thus have a mshtml host object tied to them). And this is necessary because we have to traverse their jsdisp props and that's something only jscript can do (implementation detail). If I just gave them a custom participant, how would jscript be able to know they're jsdisps and/or retrieve the jsdisp? In summary how the traversal works at a glance for the 3 "types" of objects: * Pure jsdisps are traversed just like before, and their speculative refcount is (temporarily) stored in their ref field. * HostObject jsdisps are treated specially; their speculative refcount is still stored in their (unused) ref field of jsdisp, and they are traversed like pure jsdisps for the jsdisp props, but additionally having their "host side" traversal also done (this is in ADDITION to the jsdisp traversal). (the ref field is unused since we defer it to the mshtml host object, so it's perfectly usable for the GC) * Raw XPCOM objects (e.g. pure gecko objects) are placed in the cc map, and their ref is stored there. These are the only objects for which we need a participant. I mean I could add a custom QI IID that doesn't AddRef just like Gecko does, and then obtain the jsdisp from that if you prefer...? Like a custom interface that just has one method "GetJSDisp" or something along those lines and doesn't AddRef because it *cannot* AddRef while a traversal is happening (this is a Gecko thing, this is why its two QIs there don't AddRef and are also special). -- https://gitlab.winehq.org/wine/wine/-/merge_requests/10045#note_149757
On Mon Aug 24 17:39:58 2026 +0000, Jacek Caban wrote:
A similar question, can it be an interface? In this case, it can't be made an interface because it's part of the Gecko's class vtbl, not an XPCOM interface, no? This interface wouldn't have IUnknown methods, is that possible? This is basically the vtbl interface of a C++ class (and the object member), so it must match.
I realize that using such a vtbl directly might not be completely appropriate because it's not exported (idk if compiler can, in theory, devirtualize the methods for example). I could add a proper exported interface to wine-gecko if you prefer, but at least it works this way so I know it's going in the right direction, right? -- https://gitlab.winehq.org/wine/wine/-/merge_requests/10045#note_149758
On Mon Aug 24 20:29:01 2026 +0000, Gabriel Ivăncescu wrote:
Sorry I don't quite understand what you mean. In the generic case, this code basically retrieves a participant and an object from an input raw object. Raw object meaning it can be anything (even pure gecko external object), not ours. But at the same time, it should return the jsdisp if it's a host object with a jsdisp, so that the caller (our jscript) can special case it. This is necessary because we don't place jsdisps into the "cc map", even if they're host objects (and thus have a mshtml host object tied to them). And this is necessary because we have to traverse their jsdisp props and that's something only jscript can do (implementation detail). If I just gave them a custom participant, how would jscript be able to know they're jsdisps and/or retrieve the jsdisp? In summary how the traversal works at a glance for the 3 "types" of objects: * Pure jsdisps are traversed just like before, and their speculative refcount is (temporarily) stored in their ref field. * HostObject jsdisps are treated specially; their speculative refcount is still stored in their (unused) ref field of jsdisp, and they are traversed like pure jsdisps for the jsdisp props, but additionally having their "host side" traversal also done (this is in ADDITION to the jsdisp traversal). (the ref field is unused since we defer it to the mshtml host object, so it's perfectly usable for the GC) * Raw XPCOM objects (e.g. pure gecko objects) are placed in the cc map, and their ref is stored there. These are the only objects for which we need a participant. I mean I could add a custom QI IID that doesn't AddRef just like Gecko does, and then obtain the jsdisp from that if you prefer...? Like a custom interface that just has one method "GetJSDisp" or something along those lines and doesn't AddRef because it *cannot* AddRef while a traversal is happening (this is a Gecko thing, this is why its two QIs there don't AddRef and are also special). Yes, I think that something similar to Gecko would be better as it naturally goes through vtbls without any special-casing like this. If all you need is jsdisp, you could just return `IWineJSDispatch` similar to how `IID_nsCycleCollectionISupports` works, there is no need for an actual new interface, just IID. (If some more abstraction is needed, then an additional static object could be used like `IID_nsXPCOMCycleCollectionParticipant` does.)
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/10045#note_149790
participants (3)
-
Gabriel Ivăncescu -
Gabriel Ivăncescu (＠insn)
-
Jacek Caban (＠jacek)