From: Gabriel Ivăncescu gabrielopcode@gmail.com
Objects that have had a weakref will become "zombies" with zero refcount, until we clean them up during a garbage collector run. This is to prevent a new object from sharing the same address, since it refers to the old object in the WeakMap until it's cleaned. It also doesn't hurt performance like scanning the WeakMaps on every such object's release would, and the object is otherwise completely unlinked anyway, so it doesn't keep unnecessary refs in the meantime.
Signed-off-by: Gabriel Ivăncescu gabrielopcode@gmail.com --- dlls/jscript/dispex.c | 28 ++++++++++++++ dlls/jscript/error.c | 1 + dlls/jscript/jscript.c | 1 + dlls/jscript/jscript.h | 4 ++ dlls/jscript/jscript.rc | 1 + dlls/jscript/resource.h | 1 + dlls/jscript/set.c | 83 ++++++++++++++++++++++++++++++++++++++++- 7 files changed, 117 insertions(+), 2 deletions(-)
diff --git a/dlls/jscript/dispex.c b/dlls/jscript/dispex.c index b5fc3699212..20865a30569 100644 --- a/dlls/jscript/dispex.c +++ b/dlls/jscript/dispex.c @@ -690,6 +690,26 @@ static void unlink_props(jsdisp_t *jsdisp) } }
+static void fixup_weak_refs(script_ctx_t *ctx) +{ + jsdisp_t *obj, *obj2; + + if(list_empty(&ctx->zombie_objects)) + return; + + LIST_FOR_EACH_ENTRY(obj, &ctx->objects, jsdisp_t, entry) + if(obj->builtin_info->fixup_weak_refs) + obj->builtin_info->fixup_weak_refs(obj); + + LIST_FOR_EACH_ENTRY_SAFE(obj, obj2, &ctx->zombie_objects, jsdisp_t, entry) { + if(obj->builtin_info->destructor) + obj->builtin_info->destructor(obj); + else + free(obj); + } + list_init(&ctx->zombie_objects); +} +
/* @@ -943,6 +963,7 @@ HRESULT gc_run(script_ctx_t *ctx) jsdisp_release(obj); }
+ fixup_weak_refs(ctx); ctx->gc_is_unlinking = FALSE; ctx->gc_last_tick = GetTickCount(); return S_OK; @@ -2244,6 +2265,13 @@ void jsdisp_free(jsdisp_t *obj) if(obj->prototype) jsdisp_release(obj->prototype);
+ /* If we possibly have weak refs to this object, keep it around as a zombie, + so that it only gets actually freed after the weak refs are removed. */ + if(obj->has_weak_refs) { + list_add_tail(&obj->ctx->zombie_objects, &obj->entry); + return; + } + if(obj->builtin_info->destructor) obj->builtin_info->destructor(obj); else diff --git a/dlls/jscript/error.c b/dlls/jscript/error.c index c3768668178..cf0ece42f15 100644 --- a/dlls/jscript/error.c +++ b/dlls/jscript/error.c @@ -483,6 +483,7 @@ jsdisp_t *create_builtin_error(script_ctx_t *ctx) case JS_E_OBJECT_NONEXTENSIBLE: case JS_E_NONCONFIGURABLE_REDEFINED: case JS_E_NONWRITABLE_MODIFIED: + case JS_E_KEY_NOT_OBJECT: case JS_E_PROP_DESC_MISMATCH: case JS_E_INVALID_WRITABLE_PROP_DESC: constr = ctx->type_error_constr; diff --git a/dlls/jscript/jscript.c b/dlls/jscript/jscript.c index 401f6ca85b0..8896fff720c 100644 --- a/dlls/jscript/jscript.c +++ b/dlls/jscript/jscript.c @@ -748,6 +748,7 @@ static HRESULT WINAPI JScript_SetScriptSite(IActiveScript *iface, ctx->acc = jsval_undefined(); list_init(&ctx->named_items); list_init(&ctx->objects); + list_init(&ctx->zombie_objects); heap_pool_init(&ctx->tmp_heap);
hres = create_jscaller(ctx); diff --git a/dlls/jscript/jscript.h b/dlls/jscript/jscript.h index d907da3a36c..371b76cb6f6 100644 --- a/dlls/jscript/jscript.h +++ b/dlls/jscript/jscript.h @@ -172,6 +172,7 @@ typedef struct { HRESULT (*idx_get)(jsdisp_t*,unsigned,jsval_t*); HRESULT (*idx_put)(jsdisp_t*,unsigned,jsval_t); HRESULT (*gc_traverse)(struct gc_ctx*,enum gc_traverse_op,jsdisp_t*); + void (*fixup_weak_refs)(jsdisp_t*); } builtin_info_t;
struct jsdisp_t { @@ -179,6 +180,7 @@ struct jsdisp_t {
LONG ref;
+ BOOLEAN has_weak_refs; BOOLEAN extensible; BOOLEAN gc_marked;
@@ -371,6 +373,7 @@ struct _script_ctx_t { struct _call_frame_t *call_ctx; struct list named_items; struct list objects; + struct list zombie_objects; IActiveScriptSite *site; IInternetHostSecurityManager *secmgr; DWORD safeopt; @@ -550,6 +553,7 @@ static inline HRESULT disp_call_value(script_ctx_t *ctx, IDispatch *disp, jsval_ #define JS_E_NONCONFIGURABLE_REDEFINED MAKE_JSERROR(IDS_NONCONFIGURABLE_REDEFINED) #define JS_E_NONWRITABLE_MODIFIED MAKE_JSERROR(IDS_NONWRITABLE_MODIFIED) #define JS_E_WRONG_THIS MAKE_JSERROR(IDS_WRONG_THIS) +#define JS_E_KEY_NOT_OBJECT MAKE_JSERROR(IDS_KEY_NOT_OBJECT) #define JS_E_PROP_DESC_MISMATCH MAKE_JSERROR(IDS_PROP_DESC_MISMATCH) #define JS_E_INVALID_WRITABLE_PROP_DESC MAKE_JSERROR(IDS_INVALID_WRITABLE_PROP_DESC)
diff --git a/dlls/jscript/jscript.rc b/dlls/jscript/jscript.rc index fbf965fb7e8..e5289a550a3 100644 --- a/dlls/jscript/jscript.rc +++ b/dlls/jscript/jscript.rc @@ -77,6 +77,7 @@ STRINGTABLE IDS_NONCONFIGURABLE_REDEFINED "Cannot redefine non-configurable property '|'" IDS_NONWRITABLE_MODIFIED "Cannot modify non-writable property '|'" IDS_WRONG_THIS "'this' is not a | object" + IDS_KEY_NOT_OBJECT "'key' is not an object" IDS_PROP_DESC_MISMATCH "Property cannot have both accessors and a value"
IDS_COMPILATION_ERROR "Microsoft JScript compilation error" diff --git a/dlls/jscript/resource.h b/dlls/jscript/resource.h index 0ac457d740d..ad771b67475 100644 --- a/dlls/jscript/resource.h +++ b/dlls/jscript/resource.h @@ -75,6 +75,7 @@ #define IDS_NONCONFIGURABLE_REDEFINED 0x13D6 #define IDS_NONWRITABLE_MODIFIED 0x13D7 #define IDS_WRONG_THIS 0x13FC +#define IDS_KEY_NOT_OBJECT 0x13FD /* FIXME: This is not compatible with native, but we would * conflict with IDS_UNSUPPORTED_ACTION otherwise */ #define IDS_PROP_DESC_MISMATCH 0x1F00 diff --git a/dlls/jscript/set.c b/dlls/jscript/set.c index f2de6924468..a92d3e42506 100644 --- a/dlls/jscript/set.c +++ b/dlls/jscript/set.c @@ -631,8 +631,31 @@ static int weakmap_compare(const void *k, const struct rb_entry *e) return (a > b) - (a < b); }
+static HRESULT get_weakmap_this(script_ctx_t *ctx, jsval_t vthis, WeakMapInstance **ret) +{ + jsdisp_t *jsdisp; + + if(!is_object_instance(vthis)) + return JS_E_OBJECT_EXPECTED; + if(!(jsdisp = to_jsdisp(get_object(vthis))) || !is_class(jsdisp, JSCLASS_WEAKMAP)) { + WARN("not a WeakMap object passed as 'this'\n"); + return throw_error(ctx, JS_E_WRONG_THIS, L"WeakMap"); + } + + *ret = CONTAINING_RECORD(jsdisp, WeakMapInstance, dispex); + return S_OK; +} + +static struct weakmap_entry *get_weakmap_entry(WeakMapInstance *weakmap, jsdisp_t *key) +{ + struct rb_entry *entry; + if(!(entry = rb_get(&weakmap->map, key))) return NULL; + return CONTAINING_RECORD(entry, struct weakmap_entry, entry); +} + static void release_weakmap_entry(struct weakmap_entry *entry) { + /* We can't clear the has_weak_refs flag on the key, because there might be other weak refs */ jsval_release(entry->value); free(entry); } @@ -661,8 +684,50 @@ static HRESULT WeakMap_get(script_ctx_t *ctx, jsval_t vthis, WORD flags, unsigne static HRESULT WeakMap_set(script_ctx_t *ctx, jsval_t vthis, WORD flags, unsigned argc, jsval_t *argv, jsval_t *r) { - FIXME("\n"); - return E_NOTIMPL; + jsdisp_t *key = (argc >= 1 && is_object_instance(argv[0])) ? to_jsdisp(get_object(argv[0])) : NULL; + jsval_t value = argc >= 2 ? argv[1] : jsval_undefined(); + struct weakmap_entry *entry; + WeakMapInstance *weakmap; + HRESULT hres; + + hres = get_weakmap_this(ctx, vthis, &weakmap); + if(FAILED(hres)) + return hres; + + TRACE("%p (%p %s)\n", weakmap, key, debugstr_jsval(value)); + + if(!key) + return JS_E_KEY_NOT_OBJECT; + + if(key->ctx != ctx) { + FIXME("different ctx not supported\n"); + return JS_E_KEY_NOT_OBJECT; + } + + if((entry = get_weakmap_entry(weakmap, key))) { + jsval_t val; + hres = jsval_copy(value, &val); + if(FAILED(hres)) + return hres; + + jsval_release(entry->value); + entry->value = val; + }else { + if(!(entry = malloc(sizeof(*entry)))) + return E_OUTOFMEMORY; + + entry->key = key; + hres = jsval_copy(value, &entry->value); + if(FAILED(hres)) { + free(entry); + return hres; + } + key->has_weak_refs = TRUE; + rb_put(&weakmap->map, key, &entry->entry); + } + + if(r) *r = jsval_undefined(); + return S_OK; }
static HRESULT WeakMap_has(script_ctx_t *ctx, jsval_t vthis, WORD flags, unsigned argc, jsval_t *argv, @@ -711,6 +776,19 @@ static HRESULT WeakMap_gc_traverse(struct gc_ctx *gc_ctx, enum gc_traverse_op op return S_OK; }
+static void WeakMap_fixup_weak_refs(jsdisp_t *dispex) +{ + WeakMapInstance *weakmap = (WeakMapInstance*)dispex; + struct weakmap_entry *entry, *entry2; + + RB_FOR_EACH_ENTRY_DESTRUCTOR(entry, entry2, &weakmap->map, struct weakmap_entry, entry) { + if(!entry->key->ref) { + rb_remove(&weakmap->map, &entry->entry); + release_weakmap_entry(entry); + } + } +} + static const builtin_prop_t WeakMap_prototype_props[] = { {L"clear", WeakMap_clear, PROPF_METHOD}, {L"delete", WeakMap_delete, PROPF_METHOD|1}, @@ -739,6 +817,7 @@ static const builtin_info_t WeakMap_info = { NULL, NULL, WeakMap_gc_traverse, + WeakMap_fixup_weak_refs };
static HRESULT WeakMap_constructor(script_ctx_t *ctx, jsval_t vthis, WORD flags, unsigned argc, jsval_t *argv,