Unfortunately it turns out that that lock isn't sufficient. If two threads try to read the methods of the same type definition, then they will each create their own Collection<MethodDefinition>. The lock makes sure there's no race on the actual reading, but you end up with duplicate MethodDefinitions, and only one of the collections "wins" and gets registered with the TypeDefinition. Now if you call IsGetter on one of the method definitions which isn't among the winners, it will resolve the method semantics for all methods in the type, and then assume that this also resolved the semantics for the method itself and calls the semantics getter recursively. But it doesn't since that method instance isn't actually part of the type definition -> StackOverflowException. Took me quite a bit to figure this out since the crash occurs only sometimes, and StackOverflowExceptions aren't easy to debug...
Any idea how to solve this? I'm about to add locks to each and every getter in Cecil which returns something where the object identity matters... Well, or probably better change our decompiler to not use Cecil's TypeReference/MethodReference/etc. in the opcode operands. Then we could use a global lock for the initial Cecil -> ILAst step and do all further transformations in parallel (using NRefactory's thread-safe type system). And I guess if this issue isn't fixed in official Cecil, you should back out that module_lock again so that people don't think its thread-safe when it isn't... Daniel On 2/16/2011 00:54, Jb Evain wrote: > On Tue, Feb 15, 2011 at 10:10 PM, Jb Evain <[email protected]> wrote: >> That seems painful. We can try to add one lock in >> ModuleDefinition.Read(Func<>) > For instance: > > readonly object module_lock = new object (); > > internal TRet Read<TItem, TRet> (TItem item, Func<TItem, > MetadataReader, TRet> read) > { > lock (module_lock) { > var position = reader.position; > var context = reader.context; > > var ret = read (item, reader); > > reader.position = position; > reader.context = context; > > return ret; > } > } > > Makes it easy to read from a module in different threads. But it does > have a perf overhead. Maybe this should simply be a ReadParameters, > well, parameter. > > Jb >
signature.asc
Description: OpenPGP digital signature
