[PATCH v2 1/8] vbscript: Unlink the code list from the script dispatch when it's released.
Signed-off-by: Gabriel Ivăncescu <gabrielopcode(a)gmail.com> --- v2: Unlink persistent code from the list when it is released and only re-execute after the script dispatch is released, not every time after the script is paused. This is mostly needed for the next patch to work properly, since some of the code lists will have more limited lifespans (code that is not persistent). These patches correct the implementation so that it passes the tests at the end of the series. Note that I haven't sent the linked list->array conversion yet, because that is not a functional change, just an optimization, so it will come after this behavior is corrected first (so some of it is just moved temporarily). dlls/vbscript/compile.c | 3 +++ dlls/vbscript/vbscript.c | 8 ++++++++ dlls/vbscript/vbscript.h | 4 ++++ 3 files changed, 15 insertions(+) diff --git a/dlls/vbscript/compile.c b/dlls/vbscript/compile.c index 0727979..417faf4 100644 --- a/dlls/vbscript/compile.c +++ b/dlls/vbscript/compile.c @@ -1960,6 +1960,7 @@ HRESULT compile_script(script_ctx_t *script, const WCHAR *src, const WCHAR *deli var->next = script->global_vars; script->global_vars = ctx.global_vars; + code->last_global_var = var; } if(ctx.funcs) { @@ -1967,6 +1968,7 @@ HRESULT compile_script(script_ctx_t *script, const WCHAR *src, const WCHAR *deli new_func->next = script->global_funcs; script->global_funcs = ctx.funcs; + code->last_func = new_func; } if(ctx.classes) { @@ -1981,6 +1983,7 @@ HRESULT compile_script(script_ctx_t *script, const WCHAR *src, const WCHAR *deli class->next = script->classes; script->classes = ctx.classes; + code->last_class = class; } if(TRACE_ON(vbscript_disas)) diff --git a/dlls/vbscript/vbscript.c b/dlls/vbscript/vbscript.c index 56c53b4..cff2ef5 100644 --- a/dlls/vbscript/vbscript.c +++ b/dlls/vbscript/vbscript.c @@ -131,6 +131,7 @@ IDispatch *lookup_named_item(script_ctx_t *ctx, const WCHAR *name, unsigned flag static void release_script(script_ctx_t *ctx) { class_desc_t *class_desc; + vbscode_t *code; collect_objects(ctx); clear_ei(&ctx->ei); @@ -178,6 +179,13 @@ static void release_script(script_ctx_t *ctx) IDispatchEx_Release(&script_obj->IDispatchEx_iface); } + LIST_FOR_EACH_ENTRY(code, &ctx->code_list, vbscode_t, entry) + { + if (code->last_global_var) code->last_global_var->next = NULL; + if (code->last_func) code->last_func->next = NULL; + if (code->last_class) code->last_class->next = NULL; + } + detach_global_objects(ctx); heap_pool_free(&ctx->heap); heap_pool_init(&ctx->heap); diff --git a/dlls/vbscript/vbscript.h b/dlls/vbscript/vbscript.h index fefed63..cfc9f65 100644 --- a/dlls/vbscript/vbscript.h +++ b/dlls/vbscript/vbscript.h @@ -347,6 +347,10 @@ struct _vbscode_t { unsigned bstr_cnt; heap_pool_t heap; + dynamic_var_t *last_global_var; + function_t *last_func; + class_desc_t *last_class; + struct list entry; }; -- 2.21.0
Signed-off-by: Gabriel Ivăncescu <gabrielopcode(a)gmail.com> --- It's supposed to be local to the script dispatch and destroyed with it. dlls/vbscript/compile.c | 3 ++- dlls/vbscript/vbdisp.c | 5 +++++ dlls/vbscript/vbscript.c | 9 +++++++++ dlls/vbscript/vbscript.h | 1 + 4 files changed, 17 insertions(+), 1 deletion(-) diff --git a/dlls/vbscript/compile.c b/dlls/vbscript/compile.c index 417faf4..57d6dd2 100644 --- a/dlls/vbscript/compile.c +++ b/dlls/vbscript/compile.c @@ -1992,7 +1992,6 @@ HRESULT compile_script(script_ctx_t *script, const WCHAR *src, const WCHAR *deli ctx.code = NULL; release_compiler(&ctx); - list_add_tail(&script->code_list, &code->entry); *ret = code; return S_OK; } @@ -2019,6 +2018,8 @@ HRESULT compile_procedure(script_ctx_t *script, const WCHAR *src, const WCHAR *d desc->next = script->procs; script->procs = desc; + list_add_tail(&script->script_obj->code_list, &code->entry); + *ret = desc; return S_OK; } diff --git a/dlls/vbscript/vbdisp.c b/dlls/vbscript/vbdisp.c index 1dd6d4e..7859abf 100644 --- a/dlls/vbscript/vbdisp.c +++ b/dlls/vbscript/vbdisp.c @@ -616,6 +616,10 @@ static ULONG WINAPI ScriptDisp_Release(IDispatchEx *iface) if(!ref) { assert(!This->ctx); + + while (!list_empty(&This->code_list)) + release_vbscode(LIST_ENTRY(list_head(&This->code_list), vbscode_t, entry)); + heap_free(This->ident_map); heap_free(This); } @@ -827,6 +831,7 @@ HRESULT create_script_disp(script_ctx_t *ctx, ScriptDisp **ret) script_disp->IDispatchEx_iface.lpVtbl = &ScriptDispVtbl; script_disp->ref = 1; script_disp->ctx = ctx; + list_init(&script_disp->code_list); *ret = script_disp; return S_OK; diff --git a/dlls/vbscript/vbscript.c b/dlls/vbscript/vbscript.c index cff2ef5..a968f8d 100644 --- a/dlls/vbscript/vbscript.c +++ b/dlls/vbscript/vbscript.c @@ -94,6 +94,10 @@ static void exec_queued_code(script_ctx_t *ctx) if(iter->pending_exec) exec_global_code(ctx, iter, NULL); } + LIST_FOR_EACH_ENTRY(iter, &ctx->script_obj->code_list, vbscode_t, entry) { + if(iter->pending_exec) + exec_global_code(ctx, iter, NULL); + } } IDispatch *lookup_named_item(script_ctx_t *ctx, const WCHAR *name, unsigned flags) @@ -794,6 +798,11 @@ static HRESULT WINAPI VBScriptParse_ParseScriptText(IActiveScriptParse *iface, if(FAILED(hres)) return hres; + if(dwFlags & SCRIPTTEXT_ISPERSISTENT) + list_add_tail(&This->ctx->code_list, &code->entry); + else + list_add_tail(&This->ctx->script_obj->code_list, &code->entry); + if(context) IDispatch_AddRef(code->context = context); diff --git a/dlls/vbscript/vbscript.h b/dlls/vbscript/vbscript.h index cfc9f65..2bcbb37 100644 --- a/dlls/vbscript/vbscript.h +++ b/dlls/vbscript/vbscript.h @@ -131,6 +131,7 @@ typedef struct { unsigned ident_map_size; script_ctx_t *ctx; + struct list code_list; } ScriptDisp; typedef struct _builtin_prop_t builtin_prop_t; -- 2.21.0
On 10/30/19 1:57 PM, Gabriel Ivăncescu wrote:
diff --git a/dlls/vbscript/vbscript.c b/dlls/vbscript/vbscript.c index cff2ef5..a968f8d 100644 --- a/dlls/vbscript/vbscript.c +++ b/dlls/vbscript/vbscript.c @@ -94,6 +94,10 @@ static void exec_queued_code(script_ctx_t *ctx) if(iter->pending_exec) exec_global_code(ctx, iter, NULL); } + LIST_FOR_EACH_ENTRY(iter, &ctx->script_obj->code_list, vbscode_t, entry) { + if(iter->pending_exec) + exec_global_code(ctx, iter, NULL); + } }
I'm not sure we want two different lists for those. Why can't we simply store the flag inside vbscode_t? Jacek
On 10/30/19 4:21 PM, Jacek Caban wrote:
On 10/30/19 1:57 PM, Gabriel Ivăncescu wrote:
diff --git a/dlls/vbscript/vbscript.c b/dlls/vbscript/vbscript.c index cff2ef5..a968f8d 100644 --- a/dlls/vbscript/vbscript.c +++ b/dlls/vbscript/vbscript.c @@ -94,6 +94,10 @@ static void exec_queued_code(script_ctx_t *ctx) if(iter->pending_exec) exec_global_code(ctx, iter, NULL); } + LIST_FOR_EACH_ENTRY(iter, &ctx->script_obj->code_list, vbscode_t, entry) { + if(iter->pending_exec) + exec_global_code(ctx, iter, NULL); + } }
I'm not sure we want two different lists for those. Why can't we simply store the flag inside vbscode_t?
Jacek
Well one of them is supposed to live on with the script dispatch and get destroyed with it. It's more correct to have two lists in this case. For example, we'd still have to traverse this list and free only those with the 'flag' set when we release the script dispatch, skipping the others, so we'll still need an extra loop like this. But it won't work correctly if the app holds a ref to the script dispatch. Since it will now lose access to the code the dispatch references. Not sure if it matters much in practice (will probably matter with named contexts implementation later).
Signed-off-by: Gabriel Ivăncescu <gabrielopcode(a)gmail.com> --- dlls/vbscript/vbscript.c | 1 + 1 file changed, 1 insertion(+) diff --git a/dlls/vbscript/vbscript.c b/dlls/vbscript/vbscript.c index a968f8d..63a010b 100644 --- a/dlls/vbscript/vbscript.c +++ b/dlls/vbscript/vbscript.c @@ -188,6 +188,7 @@ static void release_script(script_ctx_t *ctx) if (code->last_global_var) code->last_global_var->next = NULL; if (code->last_func) code->last_func->next = NULL; if (code->last_class) code->last_class->next = NULL; + code->pending_exec = TRUE; } detach_global_objects(ctx); -- 2.21.0
Signed-off-by: Gabriel Ivăncescu <gabrielopcode(a)gmail.com> --- Their lifespan must be limited to the script dispatch, and contained within the script dispatch's context (the pstrItemName argument when it will be implemented). When the script is stopped, they must be released since the persistent code will be re-add anyway to rebuild them. dlls/vbscript/compile.c | 31 ++++++++++++++++--------------- dlls/vbscript/interp.c | 16 +++++++++------- dlls/vbscript/vbdisp.c | 13 +++++++++++-- dlls/vbscript/vbscript.c | 14 -------------- dlls/vbscript/vbscript.h | 28 ++++++++++++++-------------- 5 files changed, 50 insertions(+), 52 deletions(-) diff --git a/dlls/vbscript/compile.c b/dlls/vbscript/compile.c index 57d6dd2..aea1533 100644 --- a/dlls/vbscript/compile.c +++ b/dlls/vbscript/compile.c @@ -1778,23 +1778,23 @@ static HRESULT compile_class(compile_ctx_t *ctx, class_decl_t *class_decl) return S_OK; } -static BOOL lookup_script_identifier(script_ctx_t *script, const WCHAR *identifier) +static BOOL lookup_script_identifier(ScriptDisp *obj, const WCHAR *identifier) { class_desc_t *class; dynamic_var_t *var; function_t *func; - for(var = script->global_vars; var; var = var->next) { + for(var = obj->global_vars; var; var = var->next) { if(!wcsicmp(var->name, identifier)) return TRUE; } - for(func = script->global_funcs; func; func = func->next) { + for(func = obj->global_funcs; func; func = func->next) { if(!wcsicmp(func->name, identifier)) return TRUE; } - for(class = script->classes; class; class = class->next) { + for(class = obj->classes; class; class = class->next) { if(!wcsicmp(class->name, identifier)) return TRUE; } @@ -1804,26 +1804,27 @@ static BOOL lookup_script_identifier(script_ctx_t *script, const WCHAR *identifi static HRESULT check_script_collisions(compile_ctx_t *ctx, script_ctx_t *script) { + ScriptDisp *obj = script->script_obj; class_desc_t *class; dynamic_var_t *var; function_t *func; for(var = ctx->global_vars; var; var = var->next) { - if(lookup_script_identifier(script, var->name)) { + if(lookup_script_identifier(obj, var->name)) { FIXME("%s: redefined\n", debugstr_w(var->name)); return E_FAIL; } } for(func = ctx->funcs; func; func = func->next) { - if(lookup_script_identifier(script, func->name)) { + if(lookup_script_identifier(obj, func->name)) { FIXME("%s: redefined\n", debugstr_w(func->name)); return E_FAIL; } } for(class = ctx->classes; class; class = class->next) { - if(lookup_script_identifier(script, class->name)) { + if(lookup_script_identifier(obj, class->name)) { FIXME("%s: redefined\n", debugstr_w(class->name)); return E_FAIL; } @@ -1958,16 +1959,16 @@ HRESULT compile_script(script_ctx_t *script, const WCHAR *src, const WCHAR *deli for(var = ctx.global_vars; var->next; var = var->next); - var->next = script->global_vars; - script->global_vars = ctx.global_vars; + var->next = script->script_obj->global_vars; + script->script_obj->global_vars = ctx.global_vars; code->last_global_var = var; } if(ctx.funcs) { for(new_func = ctx.funcs; new_func->next; new_func = new_func->next); - new_func->next = script->global_funcs; - script->global_funcs = ctx.funcs; + new_func->next = script->script_obj->global_funcs; + script->script_obj->global_funcs = ctx.funcs; code->last_func = new_func; } @@ -1981,8 +1982,8 @@ HRESULT compile_script(script_ctx_t *script, const WCHAR *src, const WCHAR *deli class = class->next; } - class->next = script->classes; - script->classes = ctx.classes; + class->next = script->script_obj->classes; + script->script_obj->classes = ctx.classes; code->last_class = class; } @@ -2015,8 +2016,8 @@ HRESULT compile_procedure(script_ctx_t *script, const WCHAR *src, const WCHAR *d desc->func_cnt = 1; desc->funcs->entries[VBDISP_CALLGET] = &code->main_code; - desc->next = script->procs; - script->procs = desc; + desc->next = script->script_obj->procs; + script->script_obj->procs = desc; list_add_tail(&script->script_obj->code_list, &code->entry); diff --git a/dlls/vbscript/interp.c b/dlls/vbscript/interp.c index 2b681a0..46046fa 100644 --- a/dlls/vbscript/interp.c +++ b/dlls/vbscript/interp.c @@ -97,6 +97,7 @@ static BOOL lookup_dynamic_vars(dynamic_var_t *var, const WCHAR *name, ref_t *re static HRESULT lookup_identifier(exec_ctx_t *ctx, BSTR name, vbdisp_invoke_type_t invoke_type, ref_t *ref) { + ScriptDisp *script_obj = ctx->script->script_obj; named_item_t *item; function_t *func; IDispatch *disp; @@ -128,7 +129,7 @@ static HRESULT lookup_identifier(exec_ctx_t *ctx, BSTR name, vbdisp_invoke_type_ } } - if(lookup_dynamic_vars(ctx->func->type == FUNC_GLOBAL ? ctx->script->global_vars : ctx->dynamic_vars, name, ref)) + if(lookup_dynamic_vars(ctx->func->type == FUNC_GLOBAL ? script_obj->global_vars : ctx->dynamic_vars, name, ref)) return S_OK; if(ctx->func->type != FUNC_GLOBAL) { @@ -162,10 +163,10 @@ static HRESULT lookup_identifier(exec_ctx_t *ctx, BSTR name, vbdisp_invoke_type_ } } - if(ctx->func->type != FUNC_GLOBAL && lookup_dynamic_vars(ctx->script->global_vars, name, ref)) + if(ctx->func->type != FUNC_GLOBAL && lookup_dynamic_vars(script_obj->global_vars, name, ref)) return S_OK; - for(func = ctx->script->global_funcs; func; func = func->next) { + for(func = script_obj->global_funcs; func; func = func->next) { if(!wcsicmp(func->name, name)) { ref->type = REF_FUNC; ref->u.f = func; @@ -207,12 +208,13 @@ static HRESULT lookup_identifier(exec_ctx_t *ctx, BSTR name, vbdisp_invoke_type_ static HRESULT add_dynamic_var(exec_ctx_t *ctx, const WCHAR *name, BOOL is_const, VARIANT **out_var) { + ScriptDisp *script_obj = ctx->script->script_obj; dynamic_var_t *new_var; heap_pool_t *heap; WCHAR *str; unsigned size; - heap = ctx->func->type == FUNC_GLOBAL ? &ctx->script->heap : &ctx->heap; + heap = ctx->func->type == FUNC_GLOBAL ? &script_obj->heap : &ctx->heap; new_var = heap_pool_alloc(heap, sizeof(*new_var)); if(!new_var) @@ -228,8 +230,8 @@ static HRESULT add_dynamic_var(exec_ctx_t *ctx, const WCHAR *name, V_VT(&new_var->v) = VT_EMPTY; if(ctx->func->type == FUNC_GLOBAL) { - new_var->next = ctx->script->global_vars; - ctx->script->global_vars = new_var; + new_var->next = script_obj->global_vars; + script_obj->global_vars = new_var; }else { new_var->next = ctx->dynamic_vars; ctx->dynamic_vars = new_var; @@ -1086,7 +1088,7 @@ static HRESULT interp_new(exec_ctx_t *ctx) return stack_push(ctx, &v); } - for(class_desc = ctx->script->classes; class_desc; class_desc = class_desc->next) { + for(class_desc = ctx->script->script_obj->classes; class_desc; class_desc = class_desc->next) { if(!wcsicmp(class_desc->name, arg)) break; } diff --git a/dlls/vbscript/vbdisp.c b/dlls/vbscript/vbdisp.c index 7859abf..211dbcd 100644 --- a/dlls/vbscript/vbdisp.c +++ b/dlls/vbscript/vbdisp.c @@ -617,9 +617,17 @@ static ULONG WINAPI ScriptDisp_Release(IDispatchEx *iface) if(!ref) { assert(!This->ctx); + release_dynamic_vars(This->global_vars); + while (This->procs) + { + class_desc_t *class_desc = This->procs; + This->procs = class_desc->next; + heap_free(class_desc); + } while (!list_empty(&This->code_list)) release_vbscode(LIST_ENTRY(list_head(&This->code_list), vbscode_t, entry)); + heap_pool_free(&This->heap); heap_free(This->ident_map); heap_free(This); } @@ -695,7 +703,7 @@ static HRESULT WINAPI ScriptDisp_GetDispID(IDispatchEx *iface, BSTR bstrName, DW } } - for(var = This->ctx->global_vars; var; var = var->next) { + for(var = This->global_vars; var; var = var->next) { if(!wcsicmp(var->name, bstrName)) { ident = add_ident(This, var->name); if(!ident) @@ -708,7 +716,7 @@ static HRESULT WINAPI ScriptDisp_GetDispID(IDispatchEx *iface, BSTR bstrName, DW } } - for(func = This->ctx->global_funcs; func; func = func->next) { + for(func = This->global_funcs; func; func = func->next) { if(!wcsicmp(func->name, bstrName)) { ident = add_ident(This, func->name); if(!ident) @@ -832,6 +840,7 @@ HRESULT create_script_disp(script_ctx_t *ctx, ScriptDisp **ret) script_disp->ref = 1; script_disp->ctx = ctx; list_init(&script_disp->code_list); + heap_pool_init(&script_disp->heap); *ret = script_disp; return S_OK; diff --git a/dlls/vbscript/vbscript.c b/dlls/vbscript/vbscript.c index 63a010b..ed71f04 100644 --- a/dlls/vbscript/vbscript.c +++ b/dlls/vbscript/vbscript.c @@ -134,15 +134,11 @@ IDispatch *lookup_named_item(script_ctx_t *ctx, const WCHAR *name, unsigned flag static void release_script(script_ctx_t *ctx) { - class_desc_t *class_desc; vbscode_t *code; collect_objects(ctx); clear_ei(&ctx->ei); - release_dynamic_vars(ctx->global_vars); - ctx->global_vars = NULL; - while(!list_empty(&ctx->named_items)) { named_item_t *iter = LIST_ENTRY(list_head(&ctx->named_items), named_item_t, entry); @@ -153,13 +149,6 @@ static void release_script(script_ctx_t *ctx) heap_free(iter); } - while(ctx->procs) { - class_desc = ctx->procs; - ctx->procs = class_desc->next; - - heap_free(class_desc); - } - if(ctx->host_global) { IDispatch_Release(ctx->host_global); ctx->host_global = NULL; @@ -192,8 +181,6 @@ static void release_script(script_ctx_t *ctx) } detach_global_objects(ctx); - heap_pool_free(&ctx->heap); - heap_pool_init(&ctx->heap); } static void destroy_script(script_ctx_t *ctx) @@ -973,7 +960,6 @@ HRESULT WINAPI VBScriptFactory_CreateInstance(IClassFactory *iface, IUnknown *pU } ctx->safeopt = INTERFACE_USES_DISPEX; - heap_pool_init(&ctx->heap); list_init(&ctx->objects); list_init(&ctx->code_list); list_init(&ctx->named_items); diff --git a/dlls/vbscript/vbscript.h b/dlls/vbscript/vbscript.h index 2bcbb37..22e8cc8 100644 --- a/dlls/vbscript/vbscript.h +++ b/dlls/vbscript/vbscript.h @@ -120,6 +120,13 @@ struct _vbdisp_t { VARIANT props[1]; }; +typedef struct _dynamic_var_t { + struct _dynamic_var_t *next; + VARIANT v; + const WCHAR *name; + BOOL is_const; +} dynamic_var_t; + typedef struct _ident_map_t ident_map_t; typedef struct { @@ -132,6 +139,13 @@ typedef struct { script_ctx_t *ctx; struct list code_list; + + dynamic_var_t *global_vars; + function_t *global_funcs; + class_desc_t *classes; + class_desc_t *procs; + + heap_pool_t heap; } ScriptDisp; typedef struct _builtin_prop_t builtin_prop_t; @@ -165,13 +179,6 @@ static inline VARIANT *get_arg(DISPPARAMS *dp, DWORD i) return dp->rgvarg + dp->cArgs-i-1; } -typedef struct _dynamic_var_t { - struct _dynamic_var_t *next; - VARIANT v; - const WCHAR *name; - BOOL is_const; -} dynamic_var_t; - struct _script_ctx_t { IActiveScriptSite *site; LCID lcid; @@ -188,13 +195,6 @@ struct _script_ctx_t { EXCEPINFO ei; - dynamic_var_t *global_vars; - function_t *global_funcs; - class_desc_t *classes; - class_desc_t *procs; - - heap_pool_t heap; - struct list objects; struct list code_list; struct list named_items; -- 2.21.0
Signed-off-by: Gabriel Ivăncescu <gabrielopcode(a)gmail.com> --- This is temporary (instead of using an array which will come later), to keep it simpler and with less changes. dlls/vbscript/compile.c | 35 +++------------------------------- dlls/vbscript/vbscript.c | 41 ++++++++++++++++++++++++++++++++++++++++ dlls/vbscript/vbscript.h | 3 +++ 3 files changed, 47 insertions(+), 32 deletions(-) diff --git a/dlls/vbscript/compile.c b/dlls/vbscript/compile.c index aea1533..7dac715 100644 --- a/dlls/vbscript/compile.c +++ b/dlls/vbscript/compile.c @@ -1954,38 +1954,9 @@ HRESULT compile_script(script_ctx_t *script, const WCHAR *src, const WCHAR *deli return compile_error(script, hres); } - if(ctx.global_vars) { - dynamic_var_t *var; - - for(var = ctx.global_vars; var->next; var = var->next); - - var->next = script->script_obj->global_vars; - script->script_obj->global_vars = ctx.global_vars; - code->last_global_var = var; - } - - if(ctx.funcs) { - for(new_func = ctx.funcs; new_func->next; new_func = new_func->next); - - new_func->next = script->script_obj->global_funcs; - script->script_obj->global_funcs = ctx.funcs; - code->last_func = new_func; - } - - if(ctx.classes) { - class_desc_t *class = ctx.classes; - - while(1) { - class->ctx = script; - if(!class->next) - break; - class = class->next; - } - - class->next = script->script_obj->classes; - script->script_obj->classes = ctx.classes; - code->last_class = class; - } + code->global_vars = ctx.global_vars; + code->funcs = ctx.funcs; + code->classes = ctx.classes; if(TRACE_ON(vbscript_disas)) dump_code(&ctx); diff --git a/dlls/vbscript/vbscript.c b/dlls/vbscript/vbscript.c index ed71f04..aa00381 100644 --- a/dlls/vbscript/vbscript.c +++ b/dlls/vbscript/vbscript.c @@ -82,6 +82,47 @@ static inline BOOL is_started(VBScript *This) static HRESULT exec_global_code(script_ctx_t *ctx, vbscode_t *code, VARIANT *res) { + ScriptDisp *obj = ctx->script_obj; + + if (code->global_vars) + { + dynamic_var_t *var; + + for (var = code->global_vars; var->next; var = var->next); + + var->next = obj->global_vars; + obj->global_vars = code->global_vars; + code->last_global_var = var; + } + + if (code->funcs) + { + function_t *func; + + for (func = code->funcs; func->next; func = func->next); + + func->next = obj->global_funcs; + obj->global_funcs = code->funcs; + code->last_func = func; + } + + if (code->classes) + { + class_desc_t *class = code->classes; + + for (;;) + { + class->ctx = ctx; + if (!class->next) + break; + class = class->next; + } + + class->next = obj->classes; + obj->classes = code->classes; + code->last_class = class; + } + code->pending_exec = FALSE; return exec_script(ctx, TRUE, &code->main_code, NULL, NULL, res); } diff --git a/dlls/vbscript/vbscript.h b/dlls/vbscript/vbscript.h index 22e8cc8..74381c4 100644 --- a/dlls/vbscript/vbscript.h +++ b/dlls/vbscript/vbscript.h @@ -348,8 +348,11 @@ struct _vbscode_t { unsigned bstr_cnt; heap_pool_t heap; + dynamic_var_t *global_vars; dynamic_var_t *last_global_var; + function_t *funcs; function_t *last_func; + class_desc_t *classes; class_desc_t *last_class; struct list entry; -- 2.21.0
Signed-off-by: Gabriel Ivăncescu <gabrielopcode(a)gmail.com> --- Needed for next patch. dlls/vbscript/vbscript.c | 42 ++++++++++++++++++++++++++-------------- 1 file changed, 28 insertions(+), 14 deletions(-) diff --git a/dlls/vbscript/vbscript.c b/dlls/vbscript/vbscript.c index aa00381..53afd0b 100644 --- a/dlls/vbscript/vbscript.c +++ b/dlls/vbscript/vbscript.c @@ -73,6 +73,32 @@ static void change_state(VBScript *This, SCRIPTSTATE state) IActiveScriptSite_OnStateChange(This->ctx->site, state); } +static HRESULT init_ctx(VBScript *vbscript) +{ + script_ctx_t *ctx; + HRESULT hr; + + if (vbscript->ctx) return S_OK; + + ctx = heap_alloc_zero(sizeof(*ctx)); + if (!ctx) return E_OUTOFMEMORY; + + ctx->safeopt = INTERFACE_USES_DISPEX; + list_init(&ctx->objects); + list_init(&ctx->code_list); + list_init(&ctx->named_items); + + hr = init_global(ctx); + if (FAILED(hr)) + { + heap_free(ctx); + return hr; + } + + vbscript->ctx = ctx; + return S_OK; +} + static inline BOOL is_started(VBScript *This) { return This->state == SCRIPTSTATE_STARTED @@ -975,7 +1001,6 @@ static const IObjectSafetyVtbl VBScriptSafetyVtbl = { HRESULT WINAPI VBScriptFactory_CreateInstance(IClassFactory *iface, IUnknown *pUnkOuter, REFIID riid, void **ppv) { - script_ctx_t *ctx; VBScript *ret; HRESULT hres; @@ -994,20 +1019,9 @@ HRESULT WINAPI VBScriptFactory_CreateInstance(IClassFactory *iface, IUnknown *pU ret->ref = 1; ret->state = SCRIPTSTATE_UNINITIALIZED; - ctx = ret->ctx = heap_alloc_zero(sizeof(*ctx)); - if(!ctx) { + if(FAILED(hres = init_ctx(ret))) + { heap_free(ret); - return E_OUTOFMEMORY; - } - - ctx->safeopt = INTERFACE_USES_DISPEX; - list_init(&ctx->objects); - list_init(&ctx->code_list); - list_init(&ctx->named_items); - - hres = init_global(ctx); - if(FAILED(hres)) { - IActiveScript_Release(&ret->IActiveScript_iface); return hres; } -- 2.21.0
When the engine is closed, even the persistent code is removed. Signed-off-by: Gabriel Ivăncescu <gabrielopcode(a)gmail.com> --- Removing the context altogether is necessary for the future when it will be ref counted (patch that will come after these). dlls/vbscript/vbscript.c | 22 ++++++++++++++++++++-- 1 file changed, 20 insertions(+), 2 deletions(-) diff --git a/dlls/vbscript/vbscript.c b/dlls/vbscript/vbscript.c index 53afd0b..e5d745b 100644 --- a/dlls/vbscript/vbscript.c +++ b/dlls/vbscript/vbscript.c @@ -255,7 +255,6 @@ static void destroy_script(script_ctx_t *ctx) while(!list_empty(&ctx->code_list)) release_vbscode(LIST_ENTRY(list_head(&ctx->code_list), vbscode_t, entry)); - release_script(ctx); heap_free(ctx); } @@ -279,6 +278,11 @@ static void decrease_state(VBScript *This, SCRIPTSTATE state) change_state(This, state); release_script(This->ctx); This->thread_id = 0; + if(state == SCRIPTSTATE_CLOSED) + { + destroy_script(This->ctx); + This->ctx = NULL; + } break; case SCRIPTSTATE_CLOSED: break; @@ -454,7 +458,6 @@ static ULONG WINAPI VBScript_Release(IActiveScript *iface) if(!ref) { decrease_state(This, SCRIPTSTATE_CLOSED); - destroy_script(This->ctx); heap_free(This); } @@ -472,6 +475,9 @@ static HRESULT WINAPI VBScript_SetScriptSite(IActiveScript *iface, IActiveScript if(!pass) return E_POINTER; + if(FAILED(hres = init_ctx(This))) + return hres; + if(This->ctx->site) return E_UNEXPECTED; @@ -798,9 +804,13 @@ static ULONG WINAPI VBScriptParse_Release(IActiveScriptParse *iface) static HRESULT WINAPI VBScriptParse_InitNew(IActiveScriptParse *iface) { VBScript *This = impl_from_IActiveScriptParse(iface); + HRESULT hr; TRACE("(%p)\n", This); + if(FAILED(hr = init_ctx(This))) + return hr; + if(This->is_initialized) return E_UNEXPECTED; This->is_initialized = TRUE; @@ -966,12 +976,16 @@ static HRESULT WINAPI VBScriptSafety_GetInterfaceSafetyOptions(IObjectSafety *if DWORD *pdwSupportedOptions, DWORD *pdwEnabledOptions) { VBScript *This = impl_from_IObjectSafety(iface); + HRESULT hr; TRACE("(%p)->(%s %p %p)\n", This, debugstr_guid(riid), pdwSupportedOptions, pdwEnabledOptions); if(!pdwSupportedOptions || !pdwEnabledOptions) return E_POINTER; + if(FAILED(hr = init_ctx(This))) + return hr; + *pdwSupportedOptions = SUPPORTED_OPTIONS; *pdwEnabledOptions = This->ctx->safeopt; return S_OK; @@ -981,12 +995,16 @@ static HRESULT WINAPI VBScriptSafety_SetInterfaceSafetyOptions(IObjectSafety *if DWORD dwOptionSetMask, DWORD dwEnabledOptions) { VBScript *This = impl_from_IObjectSafety(iface); + HRESULT hr; TRACE("(%p)->(%s %x %x)\n", This, debugstr_guid(riid), dwOptionSetMask, dwEnabledOptions); if(dwOptionSetMask & ~SUPPORTED_OPTIONS) return E_FAIL; + if(FAILED(hr = init_ctx(This))) + return hr; + This->ctx->safeopt = (dwEnabledOptions & dwOptionSetMask) | (This->ctx->safeopt & ~dwOptionSetMask) | INTERFACE_USES_DISPEX; return S_OK; } -- 2.21.0
On 10/30/19 1:57 PM, Gabriel Ivăncescu wrote:
When the engine is closed, even the persistent code is removed.
Signed-off-by: Gabriel Ivăncescu<gabrielopcode(a)gmail.com> ---
Removing the context altogether is necessary for the future when it will be ref counted (patch that will come after these).
Why will it be necessary? I'm not sure that initializing the context in SetInterfaceSafetyOptions implementation is an improvement. Can we just remove persistent code from the context when script engine is closed instead? Are safety options really lost on script close? Jacek
On 10/30/19 4:25 PM, Jacek Caban wrote:
On 10/30/19 1:57 PM, Gabriel Ivăncescu wrote:
When the engine is closed, even the persistent code is removed.
Signed-off-by: Gabriel Ivăncescu<gabrielopcode(a)gmail.com> ---
Removing the context altogether is necessary for the future when it will be ref counted (patch that will come after these).
Why will it be necessary? I'm not sure that initializing the context in SetInterfaceSafetyOptions implementation is an improvement. Can we just remove persistent code from the context when script engine is closed instead? Are safety options really lost on script close?
Jacek
We can't remove only the persistent code because of the reason I mentioned in the comments :-) When we will ref count the script context (in the next patch series), we'll need access to the script context from another place (i.e. the TypeInfo). The script will, of course, have to initialize another context. Re-using the same context will clash with the one the TypeInfo is still holding. That's not going to work. The only reason I initialize the context is because it is needed to be allocated there. This patch sets it to NULL when it destroys it, so those places would cause a page fault otherwise. Of course, I *could* be creating a new context as soon as it is destroyed, but that can potentially fail. I'm not sure that *closing* a script having chance to fail is a good idea. So instead it just sets it to NULL and then it gets initialized on demand. The other places have the thread_id check, so they cannot be without a context, hence I didn't add init_ctx call there.
Signed-off-by: Gabriel Ivăncescu <gabrielopcode(a)gmail.com> --- dlls/vbscript/tests/vbscript.c | 194 +++++++++++++++++++++++++++++++++ 1 file changed, 194 insertions(+) diff --git a/dlls/vbscript/tests/vbscript.c b/dlls/vbscript/tests/vbscript.c index fdbfc44..a678710 100644 --- a/dlls/vbscript/tests/vbscript.c +++ b/dlls/vbscript/tests/vbscript.c @@ -624,6 +624,199 @@ static void test_scriptdisp(void) ok(!ref, "ref = %d\n", ref); } +static void test_code_persistence(void) +{ + IActiveScriptParse *parser; + IDispatchEx *script_disp; + IActiveScript *vbscript; + HRESULT hr; + DISPID id; + ULONG ref; + BSTR str; + + vbscript = create_vbscript(); + + hr = IActiveScript_QueryInterface(vbscript, &IID_IActiveScriptParse, (void**)&parser); + ok(hr == S_OK, "Could not get IActiveScriptParse iface: %08x\n", hr); + test_state(vbscript, SCRIPTSTATE_UNINITIALIZED); + test_safety(vbscript); + + SET_EXPECT(GetLCID); + hr = IActiveScript_SetScriptSite(vbscript, &ActiveScriptSite); + ok(hr == S_OK, "SetScriptSite failed: %08x\n", hr); + CHECK_CALLED(GetLCID); + + SET_EXPECT(OnStateChange_INITIALIZED); + hr = IActiveScriptParse_InitNew(parser); + ok(hr == S_OK, "InitNew failed: %08x\n", hr); + CHECK_CALLED(OnStateChange_INITIALIZED); + test_state(vbscript, SCRIPTSTATE_INITIALIZED); + + str = a2bstr( + "x = 1\n" + "dim y\ny = 2\n"); + hr = IActiveScriptParse_ParseScriptText(parser, str, NULL, NULL, NULL, 0, 0, 0, NULL, NULL); + ok(hr == S_OK, "ParseScriptText failed: %08x\n", hr); + SysFreeString(str); + + str = a2bstr("dim z\nz = 3\n"); + hr = IActiveScriptParse_ParseScriptText(parser, str, NULL, NULL, NULL, 0, 0, SCRIPTTEXT_ISPERSISTENT, NULL, NULL); + ok(hr == S_OK, "ParseScriptText failed: %08x\n", hr); + SysFreeString(str); + + /* Pending code does not add identifiers to the global scope */ + script_disp = get_script_dispatch(vbscript); + id = 0; + get_disp_id(script_disp, "x", DISP_E_UNKNOWNNAME, &id); + ok(id == -1, "id = %d, expected -1\n", id); + id = 0; + get_disp_id(script_disp, "y", DISP_E_UNKNOWNNAME, &id); + ok(id == -1, "id = %d, expected -1\n", id); + id = 0; + get_disp_id(script_disp, "z", DISP_E_UNKNOWNNAME, &id); + ok(id == -1, "id = %d, expected -1\n", id); + IDispatchEx_Release(script_disp); + + /* Uninitialized state removes code without SCRIPTTEXT_ISPERSISTENT */ + SET_EXPECT(OnStateChange_UNINITIALIZED); + hr = IActiveScript_SetScriptState(vbscript, SCRIPTSTATE_UNINITIALIZED); + ok(hr == S_OK, "SetScriptState(SCRIPTSTATE_UNINITIALIZED) failed: %08x\n", hr); + CHECK_CALLED(OnStateChange_UNINITIALIZED); + test_no_script_dispatch(vbscript); + + SET_EXPECT(GetLCID); + SET_EXPECT(OnStateChange_INITIALIZED); + hr = IActiveScript_SetScriptSite(vbscript, &ActiveScriptSite); + ok(hr == S_OK, "SetScriptSite failed: %08x\n", hr); + CHECK_CALLED(GetLCID); + CHECK_CALLED(OnStateChange_INITIALIZED); + + SET_EXPECT(OnStateChange_CONNECTED); + SET_EXPECT(OnEnterScript); + SET_EXPECT(OnLeaveScript); + hr = IActiveScript_SetScriptState(vbscript, SCRIPTSTATE_CONNECTED); + ok(hr == S_OK, "SetScriptState(SCRIPTSTATE_CONNECTED) failed: %08x\n", hr); + CHECK_CALLED(OnStateChange_CONNECTED); + CHECK_CALLED(OnEnterScript); + CHECK_CALLED(OnLeaveScript); + test_state(vbscript, SCRIPTSTATE_CONNECTED); + + script_disp = get_script_dispatch(vbscript); + id = 0; + get_disp_id(script_disp, "x", DISP_E_UNKNOWNNAME, &id); + ok(id == -1, "id = %d, expected -1\n", id); + id = 0; + get_disp_id(script_disp, "y", DISP_E_UNKNOWNNAME, &id); + ok(id == -1, "id = %d, expected -1\n", id); + id = 0; + get_disp_id(script_disp, "z", S_OK, &id); + ok(id != -1, "id = -1\n"); + IDispatchEx_Release(script_disp); + + /* Uninitialized state does not remove persistent code, even if it was executed */ + SET_EXPECT(OnStateChange_DISCONNECTED); + SET_EXPECT(OnStateChange_INITIALIZED); + SET_EXPECT(OnStateChange_UNINITIALIZED); + hr = IActiveScript_SetScriptState(vbscript, SCRIPTSTATE_UNINITIALIZED); + ok(hr == S_OK, "SetScriptState(SCRIPTSTATE_UNINITIALIZED) failed: %08x\n", hr); + CHECK_CALLED(OnStateChange_DISCONNECTED); + CHECK_CALLED(OnStateChange_INITIALIZED); + CHECK_CALLED(OnStateChange_UNINITIALIZED); + test_no_script_dispatch(vbscript); + + SET_EXPECT(GetLCID); + SET_EXPECT(OnStateChange_INITIALIZED); + hr = IActiveScript_SetScriptSite(vbscript, &ActiveScriptSite); + ok(hr == S_OK, "SetScriptSite failed: %08x\n", hr); + CHECK_CALLED(GetLCID); + CHECK_CALLED(OnStateChange_INITIALIZED); + + script_disp = get_script_dispatch(vbscript); + id = 0; + get_disp_id(script_disp, "z", DISP_E_UNKNOWNNAME, &id); + ok(id == -1, "id = %d, expected -1\n", id); + IDispatchEx_Release(script_disp); + + SET_EXPECT(OnStateChange_CONNECTED); + SET_EXPECT(OnEnterScript); + SET_EXPECT(OnLeaveScript); + hr = IActiveScript_SetScriptState(vbscript, SCRIPTSTATE_CONNECTED); + ok(hr == S_OK, "SetScriptState(SCRIPTSTATE_CONNECTED) failed: %08x\n", hr); + CHECK_CALLED(OnStateChange_CONNECTED); + CHECK_CALLED(OnEnterScript); + CHECK_CALLED(OnLeaveScript); + test_state(vbscript, SCRIPTSTATE_CONNECTED); + + script_disp = get_script_dispatch(vbscript); + id = 0; + get_disp_id(script_disp, "z", S_OK, &id); + ok(id != -1, "id = -1\n"); + IDispatchEx_Release(script_disp); + + SET_EXPECT(OnStateChange_DISCONNECTED); + SET_EXPECT(OnStateChange_INITIALIZED); + SET_EXPECT(OnStateChange_UNINITIALIZED); + hr = IActiveScript_SetScriptState(vbscript, SCRIPTSTATE_UNINITIALIZED); + ok(hr == S_OK, "SetScriptState(SCRIPTSTATE_UNINITIALIZED) failed: %08x\n", hr); + CHECK_CALLED(OnStateChange_DISCONNECTED); + CHECK_CALLED(OnStateChange_INITIALIZED); + CHECK_CALLED(OnStateChange_UNINITIALIZED); + + SET_EXPECT(GetLCID); + SET_EXPECT(OnStateChange_INITIALIZED); + hr = IActiveScript_SetScriptSite(vbscript, &ActiveScriptSite); + ok(hr == S_OK, "SetScriptSite failed: %08x\n", hr); + CHECK_CALLED(GetLCID); + CHECK_CALLED(OnStateChange_INITIALIZED); + + str = a2bstr("dim y\ny = 2\n"); + hr = IActiveScriptParse_ParseScriptText(parser, str, NULL, NULL, NULL, 0, 0, SCRIPTTEXT_ISPERSISTENT, NULL, NULL); + ok(hr == S_OK, "ParseScriptText failed: %08x\n", hr); + SysFreeString(str); + + /* Closing the script engine removes all code (even if it's pending and persistent) */ + SET_EXPECT(OnStateChange_CLOSED); + hr = IActiveScript_Close(vbscript); + ok(hr == S_OK, "Close failed: %08x\n", hr); + CHECK_CALLED(OnStateChange_CLOSED); + test_state(vbscript, SCRIPTSTATE_CLOSED); + test_no_script_dispatch(vbscript); + + SET_EXPECT(OnStateChange_INITIALIZED); + SET_EXPECT(GetLCID); + hr = IActiveScript_SetScriptSite(vbscript, &ActiveScriptSite); + ok(hr == S_OK, "SetScriptSite failed: %08x\n", hr); + CHECK_CALLED(OnStateChange_INITIALIZED); + CHECK_CALLED(GetLCID); + test_state(vbscript, SCRIPTSTATE_INITIALIZED); + + SET_EXPECT(OnStateChange_CONNECTED); + hr = IActiveScript_SetScriptState(vbscript, SCRIPTSTATE_CONNECTED); + ok(hr == S_OK, "SetScriptState(SCRIPTSTATE_CONNECTED) failed: %08x\n", hr); + CHECK_CALLED(OnStateChange_CONNECTED); + test_state(vbscript, SCRIPTSTATE_CONNECTED); + + script_disp = get_script_dispatch(vbscript); + id = 0; + get_disp_id(script_disp, "y", DISP_E_UNKNOWNNAME, &id); + ok(id == -1, "id = %d, expected -1\n", id); + id = 0; + get_disp_id(script_disp, "z", DISP_E_UNKNOWNNAME, &id); + ok(id == -1, "id = %d, expected -1\n", id); + IDispatchEx_Release(script_disp); + + IActiveScriptParse_Release(parser); + + SET_EXPECT(OnStateChange_DISCONNECTED); + SET_EXPECT(OnStateChange_INITIALIZED); + SET_EXPECT(OnStateChange_CLOSED); + ref = IActiveScript_Release(vbscript); + ok(!ref, "ref = %d\n", ref); + CHECK_CALLED(OnStateChange_DISCONNECTED); + CHECK_CALLED(OnStateChange_INITIALIZED); + CHECK_CALLED(OnStateChange_CLOSED); +} + static void test_vbscript(void) { IActiveScriptParseProcedure2 *parse_proc; @@ -1215,6 +1408,7 @@ START_TEST(vbscript) test_vbscript_initializing(); test_named_items(); test_scriptdisp(); + test_code_persistence(); test_RegExp(); test_RegExp_Replace(); }else { -- 2.21.0
Hi Gabriel, This patch adds extra complexity for unlinking. However, I imagine that once you change the representation of global functions and variables, they will not be needed. We'd probably need such list inside vbscode_t, but it will never be linked/unlinked to a global list (global list will in fact not exist). I would expect that it would be easier to first replace lists with arrays inside script_ctx_t and consider moving things to script dispatch later. Thanks, Jacek
Hi Jacek, On 10/30/19 4:20 PM, Jacek Caban wrote:
Hi Gabriel,
This patch adds extra complexity for unlinking. However, I imagine that once you change the representation of global functions and variables, they will not be needed. We'd probably need such list inside vbscode_t, but it will never be linked/unlinked to a global list (global list will in fact not exist).
I would expect that it would be easier to first replace lists with arrays inside script_ctx_t and consider moving things to script dispatch later.
Thanks,
Jacek
Sorry I'm not sure I understand. vbscode_t already stores the list of its own global variables and functions. They just get linked to the script_ctx_t lists when they are being executed (after this patch series, now it's when they are compiled which is wrong). The only alternative I can think of is to copy the variables/functions over when adding them to the context/dispatch globals. Unlinking them is, IMO, simpler than copying but that's just me. If we replace them with arrays this won't really change the fact we'll have to make copies. Is that what you have in mind?
On 10/30/19 3:33 PM, Gabriel Ivăncescu wrote:
Hi Jacek,
On 10/30/19 4:20 PM, Jacek Caban wrote:
Hi Gabriel,
This patch adds extra complexity for unlinking. However, I imagine that once you change the representation of global functions and variables, they will not be needed. We'd probably need such list inside vbscode_t, but it will never be linked/unlinked to a global list (global list will in fact not exist).
I would expect that it would be easier to first replace lists with arrays inside script_ctx_t and consider moving things to script dispatch later.
Thanks,
Jacek
Sorry I'm not sure I understand. vbscode_t already stores the list of its own global variables and functions.
They just get linked to the script_ctx_t lists when they are being executed (after this patch series, now it's when they are compiled which is wrong).
The only alternative I can think of is to copy the variables/functions over when adding them to the context/dispatch globals. Unlinking them is, IMO, simpler than copying but that's just me.
If we replace them with arrays this won't really change the fact we'll have to make copies. Is that what you have in mind?
Now I'm confused, you wanted them to be arrays because of ITypeInfo requirements from the beginning. Your later tests confirmed that it may be a good idea, so I assumed that that's your plan. Arrays could be just store pointers to existing structs, so no copying is required (although we may want to copy some parts of them for a different reason). Jacek
On 10/30/19 4:52 PM, Jacek Caban wrote:
On 10/30/19 3:33 PM, Gabriel Ivăncescu wrote:
Hi Jacek,
On 10/30/19 4:20 PM, Jacek Caban wrote:
Hi Gabriel,
This patch adds extra complexity for unlinking. However, I imagine that once you change the representation of global functions and variables, they will not be needed. We'd probably need such list inside vbscode_t, but it will never be linked/unlinked to a global list (global list will in fact not exist).
I would expect that it would be easier to first replace lists with arrays inside script_ctx_t and consider moving things to script dispatch later.
Thanks,
Jacek
Sorry I'm not sure I understand. vbscode_t already stores the list of its own global variables and functions.
They just get linked to the script_ctx_t lists when they are being executed (after this patch series, now it's when they are compiled which is wrong).
The only alternative I can think of is to copy the variables/functions over when adding them to the context/dispatch globals. Unlinking them is, IMO, simpler than copying but that's just me.
If we replace them with arrays this won't really change the fact we'll have to make copies. Is that what you have in mind?
Now I'm confused, you wanted them to be arrays because of ITypeInfo requirements from the beginning. Your later tests confirmed that it may be a good idea, so I assumed that that's your plan. Arrays could be just store pointers to existing structs, so no copying is required (although we may want to copy some parts of them for a different reason).
Jacek
Ah yes indeed you are absolutely right. I was thinking that such pointer indirection wasn't desirable, for some reason. But if that's good to go then I'll implement them that way, then.
participants (2)
-
Gabriel Ivăncescu -
Jacek Caban