I tested it myself by setting breakpoints in the MetadataReader. Accessing an AssemblyDefinition from multiple threads is NOT safe, not even with Immediate reading mode: method bodies are still initialized lazily. And I also realized that immediate reading mode is not a solution for us; it uses up way too much memory: With only a few assemblies loaded (18 assemblies; less than I usually have loaded in Reflector), the whole app in deferred reading mode takes 50 MB RAM, but in immediate reading mode takes 150 MB. Considering that the user will usually browse only a few types, deferred mode looks like the better choice for us. So what should I do now? Litter my whole code with locks whenever I access some Cecil object? There are hundreds of such places in this app... Copy everything I need into my own thread-safe data structures? This is the approach we use in SharpDevelop, but we don't need any method bodies there.
Or should I try to patch Cecil to make it safe for multi-threaded read access? It seems that I could get away with adding a lock to all the MetadataReader.ReadXXX methods. That way the properties would still be fast once initialized, only the first access will involve locking. From an API point of view, an application that analyses (but doesn't change) an AssemblyDefinition should be able to treat it as an immutable object, so it should be able to read from it using any number of threads. Sorry Simon for hijacking this thread; this won't help with reading/writing assemblies faster (that would be very hard to do in parallel), but I hope we can at least find some solution that allows using Cecil in parallel applications (e.g. running some analysis for each type in the assembly using a Parallel.ForEach). Attached is a simple program that demonstrates how Cecil crashes when accessing a module from multiple threads (on my quadcore, it crashes 100% of the time with various types of exceptions). Daniel On 2/15/2011 19:33, Daniel Grunwald wrote: > What I'm currently trying to do is to use my own thread-safe resolver > implementation. > Basically I load assemblies on separate threads (just loading shouldn't > invoke the resolver, correct?). > > But what's also interesting is: > Is it safe to access an AssemblyDefinition (for reading only) from > multiple threads? > > I think this really ought to be safe, at least with ReadingMode.Immediate. > The lazy initialization pattern used by the Cecil properties seems to be > safe - it would be better if the field was marked volatile, but that > doesn't make a difference in this case on the current .NET framework > implementation. > What seems unsafe is the MetadataReader's use of shared state (the > context and reading position); but if I use immediate reading mode, the > MetadataReader shouldn't be used after the initialization, correct? > > Jb please correct me if I'm wrong, I don't want to release a decompiler > GUI with race conditions (and yes, I do a lot of multithreading to keep > the UI responsive). It seems to work fine so far - but then again, it > also seems fine with deferred reading mode, which means I'm just > (un)lucky and didn't hit the race condition so far. > > Daniel > > > On 2/15/2011 13:09, Jb Evain wrote: >> Let me be perfectly clear : maybe. >> >> Seriously, I don't think there are many things easily parallelizable >> in Cecil without adding lots of locking. >> >> On Tuesday, February 15, 2011, Simon Cropp <[email protected]> wrote: >>> JB >>> >>> Do you have an plans to make use of multiple threads when reading and >>> writing assemblies with cecil? >>> >>> -- >>> -- >>> mono-cecil
using System;
using System.Collections.Generic;
using System.Linq;
using System.Threading.Tasks;
using Mono.Cecil;
class Program
{
public static void Main(string[] args)
{
// Both Deferred and Immediate will crash.
var readerParams = new ReaderParameters { ReadingMode =
ReadingMode.Deferred };
var mscorlib =
ModuleDefinition.ReadModule(typeof(object).Assembly.Location, readerParams);
Parallel.ForEach(mscorlib.Types, delegate(TypeDefinition t) {
var m = t.Methods.FirstOrDefault();
if (m != null && m.Body != null) {
m.Body.Instructions.FirstOrDefault();
}
});
}
// Use this non-threaded ForEach instead of the Parallel.ForEach to
verify that the code doesn't crash when no threading is involved.
static void ForEach<T>(IEnumerable<T> e, Action<T> a)
{
foreach (var i in e)
a(i);
}
}
signature.asc
Description: OpenPGP digital signature
