Improve thread safety of shared strings * Switch from Str_Clone to Str_new_from_trusted_utf8 to guarantee that shared, immortal objects own their strings exclusively. * Make sure that the unique copy and the thread-safe string wrappers are updated if the string is changed.
Project: http://git-wip-us.apache.org/repos/asf/lucy-clownfish/repo Commit: http://git-wip-us.apache.org/repos/asf/lucy-clownfish/commit/41ec26e6 Tree: http://git-wip-us.apache.org/repos/asf/lucy-clownfish/tree/41ec26e6 Diff: http://git-wip-us.apache.org/repos/asf/lucy-clownfish/diff/41ec26e6 Branch: refs/heads/master Commit: 41ec26e6238cd1d3d9708724768d9d83cd370589 Parents: 904bfa9 Author: Nick Wellnhofer <[email protected]> Authored: Sun May 10 03:27:32 2015 +0200 Committer: Nick Wellnhofer <[email protected]> Committed: Tue May 12 20:05:12 2015 +0200 ---------------------------------------------------------------------- runtime/core/Clownfish/Class.c | 41 +++++++++++++------------- runtime/core/Clownfish/LockFreeRegistry.c | 3 +- runtime/core/Clownfish/Method.c | 6 ++-- 3 files changed, 26 insertions(+), 24 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/lucy-clownfish/blob/41ec26e6/runtime/core/Clownfish/Class.c ---------------------------------------------------------------------- diff --git a/runtime/core/Clownfish/Class.c b/runtime/core/Clownfish/Class.c index d35fb94..e1fb3b1 100644 --- a/runtime/core/Clownfish/Class.c +++ b/runtime/core/Clownfish/Class.c @@ -44,6 +44,9 @@ size_t Class_offset_of_parent = offsetof(Class, parent); +static void +S_set_name(Class *self, const char *utf8, size_t size); + static Method* S_find_method(Class *self, const char *meth_name); @@ -179,19 +182,12 @@ Class_bootstrap(const ClassSpec *specs, size_t num_specs) * Pass 3: * - Inititalize name and method array. * - Register class. - * - * We use a "wrapped" String for `name` because it's effectively - * threadsafe: the sole reference is owned by an immortal object and any - * INCREF spawns a copy. */ for (size_t i = 0; i < num_specs; ++i) { const ClassSpec *spec = &specs[i]; Class *klass = *spec->klass; - klass->name_internal = Str_newf("%s", spec->name); - klass->name - = Str_new_wrap_trusted_utf8(Str_Get_Ptr8(klass->name_internal), - Str_Get_Size(klass->name_internal)); + S_set_name(klass, spec->name, strlen(spec->name)); klass->methods = (Method**)MALLOCATE((spec->num_novel_meths + 1) * sizeof(Method*)); @@ -272,10 +268,7 @@ S_simple_subclass(Class *parent, String *name) { subclass->class_alloc_size = parent->class_alloc_size; subclass->methods = (Method**)CALLOCATE(1, sizeof(Method*)); - subclass->name_internal = Str_Clone(name); - subclass->name - = Str_new_wrap_trusted_utf8(Str_Get_Ptr8(subclass->name_internal), - Str_Get_Size(subclass->name_internal)); + S_set_name(subclass, Str_Get_Ptr8(name), Str_Get_Size(name)); memcpy(subclass->vtable, parent->vtable, parent->class_alloc_size - offsetof(Class, vtable)); @@ -361,16 +354,13 @@ Class_add_to_registry(Class *klass) { return false; } else { - String *class_name = Str_Clone(klass->name); - bool retval = LFReg_register(Class_registry, class_name, (Obj*)klass); - DECREF(class_name); - return retval; + return LFReg_register(Class_registry, klass->name, (Obj*)klass); } } bool Class_add_alias_to_registry(Class *klass, const char *alias_ptr, - size_t alias_len) { + size_t alias_len) { if (Class_registry == NULL) { Class_init_registry(); } @@ -379,10 +369,7 @@ Class_add_alias_to_registry(Class *klass, const char *alias_ptr, return false; } else { - String *class_name = Str_Clone(alias); - bool retval = LFReg_register(Class_registry, class_name, (Obj*)klass); - DECREF(class_name); - return retval; + return LFReg_register(Class_registry, alias, (Obj*)klass); } } @@ -417,6 +404,18 @@ Class_Exclude_Host_Method_IMP(Class *self, const char *meth_name) { method->is_excluded = true; } +static void +S_set_name(Class *self, const char *utf8, size_t size) { + /* + * We use a "wrapped" String for `name` because it's effectively + * threadsafe: the sole reference is owned by an immortal object and any + * INCREF spawns a copy. + */ + self->name_internal = Str_new_from_trusted_utf8(utf8, size); + self->name = Str_new_wrap_trusted_utf8(Str_Get_Ptr8(self->name_internal), + Str_Get_Size(self->name_internal)); +} + static Method* S_find_method(Class *self, const char *name) { size_t name_len = strlen(name); http://git-wip-us.apache.org/repos/asf/lucy-clownfish/blob/41ec26e6/runtime/core/Clownfish/LockFreeRegistry.c ---------------------------------------------------------------------- diff --git a/runtime/core/Clownfish/LockFreeRegistry.c b/runtime/core/Clownfish/LockFreeRegistry.c index 8a7b07f..377b819 100644 --- a/runtime/core/Clownfish/LockFreeRegistry.c +++ b/runtime/core/Clownfish/LockFreeRegistry.c @@ -70,7 +70,8 @@ FIND_END_OF_LINKED_LIST: if (!new_entry) { new_entry = (LFRegEntry*)MALLOCATE(sizeof(LFRegEntry)); new_entry->hash_sum = hash_sum; - new_entry->key = (String*)INCREF(key); + new_entry->key = Str_new_from_trusted_utf8(Str_Get_Ptr8(key), + Str_Get_Size(key)); new_entry->value = INCREF(value); new_entry->next = NULL; } http://git-wip-us.apache.org/repos/asf/lucy-clownfish/blob/41ec26e6/runtime/core/Clownfish/Method.c ---------------------------------------------------------------------- diff --git a/runtime/core/Clownfish/Method.c b/runtime/core/Clownfish/Method.c index 931427b..5fc02da 100644 --- a/runtime/core/Clownfish/Method.c +++ b/runtime/core/Clownfish/Method.c @@ -35,7 +35,8 @@ Method_init(Method *self, String *name, cfish_method_t callback_func, * a "wrapped" string because that is effectively threadsafe: an INCREF * results in a copy and the only reference is owned by an immortal * object. */ - self->name_internal = Str_Clone(name); + self->name_internal + = Str_new_from_trusted_utf8(Str_Get_Ptr8(name), Str_Get_Size(name)); self->name = Str_new_wrap_trusted_utf8(Str_Get_Ptr8(self->name_internal), Str_Get_Size(self->name_internal)); @@ -63,7 +64,8 @@ Method_Set_Host_Alias_IMP(Method *self, String *name) { if (self->host_alias) { THROW(ERR, "Can't Set_Host_Alias more than once"); } - self->host_alias_internal = Str_Clone(name); + self->host_alias_internal + = Str_new_from_trusted_utf8(Str_Get_Ptr8(name), Str_Get_Size(name)); self->host_alias = Str_new_wrap_trusted_utf8(Str_Get_Ptr8(self->host_alias_internal), Str_Get_Size(self->host_alias_internal));
