search

jbevain / cecil

Cecil is a library to inspect, modify and create .NET programs and libraries.
MIT License
2.71k stars 619 forks source link

Is this locking correct in ModuleDefinition? #951

Open omajid opened 2 months ago

omajid commented 2 months ago

https://github.com/jbevain/cecil/blob/8e1ae7b4ea67ccc38cb8db3ded6802643109ffd7/Mono.Cecil/ModuleDefinition.cs#L387-L393

It seems like this sequence of events might be possible, resulting in creation of an DefaultAssemblyResolver instance that's never disposed:

  1. Thread 1 sees assembly_resolver.value == null
  2. Thread 2 sees assembly_resolver.value == null
  3. Thread 1 enters the lock block and sets assembly_resolver to a new instance
  4. Thread 2 enters the lock block and re-sets assembly_resolver to a new instance

The instance in step 3 is now lost and was never disposed.

ltrzesniewski commented 2 months ago

I agree, using if/lock/if would be appropriate as it's a standard pattern, or just changing = to ??= in this case.