[Xamarin.Android.Build.Tasks] Dispose DirectoryAssemblyResolver instances

[jonpryor]

Supersedes: #231
Requires: dotnet/java-interop#88

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

Commit 1d65825 bumped to Cecil 0.10.0-beta1-v2 and
Java.Interop/a1d3ecc8, which likewise uses Cecil 0.10.0-beta1-v2.

Among the various API changes in Cecil 0.10.x vs. Cecil 0.9.x is a
change to Mono.Cecil.IAssemblyResolver, implemented by
Java.Interop.Tools.Cecil.DirectoryAssemblyResolver, to implement the
IDisposable interface.

Java.Interop/a1d3ecc8 added a DirectoryAssemblyResolver.Dispose()
method, but this method didn't do anything.

Cecil 0.10.0-beta1-v2 also has a semantic change vs. Cecil 0.9.x:
AssemblyDefinition is no longer a "purely in-memory" construct.
Instead, it holds a System.IO.Stream to the assembly it's reading
(unless Mono.Cecil.ReaderParameters.InMemory is true).

This semantic change means that an assembly can't be read from and
written to at the same time via different mechanisms, so long as there
is an AssemblyDefinition instance to that file.

Which brings us to (private) Bug #44529, in which an IOException is
thrown from the <StripEmbeddedLibraries/> task:

Error executing task StripEmbeddedLibraries: System.IO.IOException: Sharing violation on path .../obj/Release/linksrc/Xamarin.Android.NUnitLite.dll
  at System.IO.FileStream..ctor (System.String path, System.IO.FileMode mode, System.IO.FileAccess access, System.IO.FileShare share, System.Int32 bufferSize, System.Boolean anonymous, System.IO.FileOptions options) [0x0025f] in <253a3790b2c44512bbca8669ecfc1822>:0
  at System.IO.FileStream..ctor (System.String path, System.IO.FileMode mode, System.IO.FileAccess access, System.IO.FileShare share) [0x00000] in <253a3790b2c44512bbca8669ecfc1822>:0
  at (wrapper remoting-invoke-with-check) System.IO.FileStream:.ctor (string,System.IO.FileMode,System.IO.FileAccess,System.IO.FileShare)
  at Mono.Cecil.ModuleDefinition.GetFileStream (System.String fileName, System.IO.FileMode mode, System.IO.FileAccess access, System.IO.FileShare share) [0x00007] in :0
  at Mono.Cecil.ModuleDefinition.Write (System.String fileName, Mono.Cecil.WriterParameters parameters) [0x00007] in :0
  at Mono.Cecil.AssemblyDefinition.Write (System.String fileName, Mono.Cecil.WriterParameters parameters) [0x00001] in :0
  at Xamarin.Android.Tasks.StripEmbeddedLibraries.Execute () [0x0034a] in <3d5202a5d4874a76a99388021bf1ab1a>:0

The underlying cause of the IOException is twofold:

1. AssemblyDefinition instances now hold a FileStream to the file
   they're reading from (and optionally writing to), and this
   FileStream will be kept open until
   AssemblyDefinition.Dispose() is called.
2. Nothing in Xamarin.Android.Build.Tasks calls
   AssemblyDefinition.Dispose().

Nor should anything in Xamarin.Android.Build.Tasks call
AssemblyDefinition.Dispose()! (Almost) All AssemblyDefinition
instances are cached within DirectoryAssemblyResolver instances,
and nothing calls DirectoryAssemblyResolver.Dispose() either!
(Not that this matters too much, becuase
DirectoryAssemblyResolver.Dispose() is currently a no-op.)

There is no sane reasoning here: so long as we're using
DirectoryAssemblyResolver, we're going to have more files open
(compared to when Cecil 0.9.x was used), which can result in file
sharing problems and the above IOException.

The way foward is:

1. Java.Interop.Tools.Cecil.DirectoryAssemblyResolver needs to be
   updated so that
   DirectoryAssemblyResolver.Dispose() is useful, i.e. that it
   will Dispose() of all held AssemblyDefinition instances.

2. Bump to Java.Interop/084e9f7f to pick up (1).

3. Xamarin.Android.Build.Tasks.dll needs to be updated so that
   DirectoryAssemblyResolver.Dispose() is used.

4. All code, in particular Xamarin.Andorid.Build.Tasks.dll, needs to
   be reviewed to ensure it doesn't do anything "stupid" with
   AssemblyDefinition instances and/or DirectoryAssemblyResolver.

   Examples of "stupid" things include:

   • Dispose()ing of the return value from
     DirectoryAssemblyResolver.Load():

       resolver.Load (assemblyName).Dispose();
     

   • Treating AssemblyDefinition instances in a manner inconsistent
     with how DirectoryAssemblyResolver loads them.
     DirectoryAssemblyResolver will be getting a new constructor
     overload which takes an Mono.Cecil.ReaderParameters parameter;
     the default will be the default Cecil behavior, which is for
     read-only behavior. Thus, trying to use the AssemblyDefinition
     in a "write" context will fail:

       var r = new DirectoryAssemblyResolver (...);
       var a = r.Load (assemblyName);
       a.Write ();   // BOOM
     

[dellis1972]

Looks ok. The Required PR for Java.Interop has been merge so we just need to update this one with the new hash (and the ReaderParameter TODO bits) and it should be good.

[lewurm]

LGTM