http://bugzilla.novell.com/show_bug.cgi?id=580185
http://bugzilla.novell.com/show_bug.cgi?id=580185#c7 Rodrigo Kumpera <rkump...@novell.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |rkump...@novell.com --- Comment #7 from Rodrigo Kumpera <rkump...@novell.com> 2010-02-22 16:02:36 UTC --- We're not sure if this is the way to implement this. But, in the mean time, here is a review of your patch. @@ -130,6 +130,7 @@ const MonoBundledAssembly **bundles; /* Loaded assembly binding info */ static GSList *loaded_assembly_bindings = NULL; +static GSList *parsed_assembly_bindings = NULL; parsed_assembly_bindings is not a very helpful name. It takes quite some reading of the code to figure out that it means to be the entries from parsed domain configuration files. Please add a comment to describe that. @@ -295,7 +296,7 @@ get_publisher_policy_info (MonoImage *image, MonoAssemblyName *aname, MonoAssemb /* Check that the most important elements/attributes exist */ if (!binding_info->name || !binding_info->public_key_token [0] || !binding_info->has_old_version_bottom || - !binding_info->has_new_version || !assembly_binding_maps_name (binding_info, aname)) { + !binding_info->has_new_version || !assembly_binding_maps_name (binding_info, aname)) { White space noise. + if (domain && domain->setup->configuration_file) { + gchar *domain_config_file = mono_string_to_utf8 (domain->setup->configuration_file); + mono_config_parse_assembly_bindings (domain_config_file, aname->major, aname->minor, NULL, assembly_binding_info_parsed); + info = get_parsed_binding_info_with_copy_to_loaded (aname); + + if (info && info->name && info->public_key_token [0] && info->has_old_version_bottom && + info->has_new_version && assembly_binding_maps_name (info, aname)) + info->is_valid = TRUE; + } This code calls get_parsed_binding_info_with_copy_to_loaded without holding the loader lock. The string domain_config_file is leaked. + + if (!info) { + info = g_new0 (MonoAssemblyBindingInfo, 1); + info->major = aname->major; + info->minor = aname->minor; + } + if (!info->is_valid) { + ppimage = mono_assembly_load_publisher_policy (aname); + if (ppimage) { + get_publisher_policy_info (ppimage, aname, info); + mono_image_close (ppimage); + } Can't it happen that the domain part returned an invalid binding info and this piece of code overwrite it - which doesn't look like the expected behavior. A major concern I have is that the whole thing is global even thou we have per-domain configuration files. -- Configure bugmail: http://bugzilla.novell.com/userprefs.cgi?tab=email ------- You are receiving this mail because: ------- You are the QA contact for the bug. You are the assignee for the bug. _______________________________________________ mono-bugs maillist - mono-bugs@lists.ximian.com http://lists.ximian.com/mailman/listinfo/mono-bugs