[Mono-bugs] [Bug 580185] assembly version redirection fails to work properly

[bugzilla_noreply at novell.com]

http://bugzilla.novell.com/show_bug.cgi?id=580185

http://bugzilla.novell.com/show_bug.cgi?id=580185#c7


Rodrigo Kumpera <rkumpera at novell.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |rkumpera at novell.com

--- Comment #7 from Rodrigo Kumpera <rkumpera at 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.