Is this locking correct in ModuleDefinition?

[omajid]

cecil/Mono.Cecil/ModuleDefinition.cs

Lines 387 to 393 in 8e1ae7b

	if (assembly_resolver.value == null) {
	lock (module_lock) {
	assembly_resolver = Disposable.Owned (new DefaultAssemblyResolver () as IAssemblyResolver);
	}
	}
	return assembly_resolver.value;

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]

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