Bug 9128 - When an assembly is modified between calls to GetReferencedAssemblies, undefined behaviour will result
Summary: When an assembly is modified between calls to GetReferencedAssemblies, undefi...
Status: RESOLVED NOT_ON_ROADMAP
Alias: None
Product: Runtime
Classification: Mono
Component: Reflection ()
Version: unspecified
Hardware: PC Linux
: --- normal
Target Milestone: ---
Assignee: Bugzilla
URL:
Depends on:
Blocks:
 
Reported: 2012-12-23 21:55 UTC by David McFarland
Modified: 2012-12-27 20:36 UTC (History)
2 users (show)

Tags:
Is this bug a regression?: ---
Last known good build:


Attachments
hacky patch, don't commit (100.58 KB, patch)
2012-12-23 21:55 UTC, David McFarland
Details


Notice (2018-05-24): bugzilla.xamarin.com is now in read-only mode.

Please join us on Visual Studio Developer Community and in the Xamarin and Mono organizations on GitHub to continue tracking issues. Bugzilla will remain available for reference in read-only mode. We will continue to work on open Bugzilla bugs, copy them to the new locations as needed for follow-up, and add the new items under Related Links.

Our sincere thanks to everyone who has contributed on this bug tracker over the years. Thanks also for your understanding as we make these adjustments and improvements for the future.


Please create a new report on GitHub or Developer Community with your current version information, steps to reproduce, and relevant error messages or log files if you are hitting an issue that looks similar to this resolved bug and you do not yet see a matching new report.

Related Links:
Status:
RESOLVED NOT_ON_ROADMAP

Description David McFarland 2012-12-23 21:55:54 UTC
Created attachment 3127 [details]
hacky patch, don't commit

I'm on Ubuntu x64, using mono from trunk.

Instead of explaining, I'll just show the test, which is pretty simple:

===================
[Test]
public void AssemblyModification ()
{
	string testPath = Path.Combine (
		Path.GetTempPath (), "CgNet.dll");
		
	try {
		File.Copy ("CgNet.1.dll", testPath, true);
		Assembly a = Assembly.LoadFrom (testPath);
		a.GetReferencedAssemblies ();
		File.Copy ("CgNet.2.dll", testPath, true);
		a.GetReferencedAssemblies ();
	}
	finally {
		File.Delete(testPath);
	}
}
===================

Included in the patch are CgNet.1.dll and CgNet.2.dll.  The only difference between them is that the second one has an added method (duplicated from an existing one and renamed).

If you apply the patch, and run:

mono/mcs/class/corlib$ make run-test FIXTURE=System.Reflection.AssemblyTest

You should see something like:

===================
Test Case Failures:
1) MonoTests.System.Reflection.AssemblyTest.AssemblyModification : System.Globalization.CultureNotFoundException : Culture name Attribute is not supported.
Parameter name: name
===================

If you reverse the order of the copies (2.dll first), you should get:

===================
Test Case Failures:
1) MonoTests.System.Reflection.AssemblyTest.AssemblyModification : System.Globalization.CultureNotFoundException : Culture name urrent is not supported.
Parameter name: name
===================

I took a look at GetReferencedAssemblies in gdb, and found:

First call to GetReferencedAssemblies:

===================
Breakpoint 1, ves_icall_System_Reflection_Assembly_GetReferencedAssemblies
(assembly=<optimized out>) at icall.c:4506
4506                            args [0] = mono_string_new (domain,
mono_metadata_string_heap (image, cols [MONO_ASSEMBLYREF_CULTURE]));
2: count = 2
1: cols = {4, 0, 0, 0, 0, 8140, 26646, 0, 0}
===================

Second call to GetReferencedAssemblies:

===================
Breakpoint 1, ves_icall_System_Reflection_Assembly_GetReferencedAssemblies
(assembly=<optimized out>) at icall.c:4506
4506                            args [0] = mono_string_new (domain,
mono_metadata_string_heap (image, cols [MONO_ASSEMBLYREF_CULTURE]));
2: count = 2
1: cols = {1533, 17340, 1, 512, 1138361855, 1, 512, 1537, 17395}
===================

Note that these are both from the first referenced assembly (i == 0) out of two (mscorlib, System).  The data is clearly invalid on the second one, which explains the corrupt culture string.

If you change the two copies to use the same source file, it will work normally.  If you change the second copy to a Delete(), it will work normally (wtf?).  LoadFrom and ReflectionOnlyLoadFrom seem to have the same behaviour.
Comment 1 David McFarland 2012-12-23 22:22:41 UTC
Adding a File.Delete before the second copy makes it work, which makes sense if mono keeps a handle to the inode.  Ignore my 'wtf' above.
Comment 2 Zoltan Varga 2012-12-25 02:30:42 UTC
Mono uses mmap to open assemblies, so modifications to in-use assemblies are not supported. Use delete+copy or move instead.
Comment 3 David McFarland 2012-12-27 20:36:18 UTC
I created a bug for this in xbuild:

https://bugzilla.xamarin.com/show_bug.cgi?id=9146