Dummy is not thread safe

Jan 16, 2011 at 10:16 AM

I think Dummy class is not thread safe. It has lazy initialization of properties. For example:

 

    private static IPropertyDefinition/*?*/ property;

    public static ITypeDefinition Type {
      [DebuggerNonUserCode]
      get {
        if (Dummy.type == null)
          Dummy.type = new DummyType();
        return Dummy.type;
      }
    }

It can cause bug if two thread concurrently enter in "if".

I think, using of lazy initialization (in this case) is a mistake. Or you must use InterlocedCompereExchange.

 

 

Jan 16, 2011 at 11:03 AM

And one more question...

 

How (correctly) chech result of ITypeReference.ResolvedType property?

 

Coordinator
Jan 16, 2011 at 5:02 PM

Yes, this is a bug since it is all to easy to depend on the object identities of the Dummy objects.

One way to not do so, is to use (ITypeReference.Resolvedtype is Dummy) rather than (ITypeReference.ResolvedType == Dummy.Type). However the latter is the more efficient expression.

I'll put in the necessary locks.

Jan 16, 2011 at 10:51 PM

All Dummy types is internal. But we can chech resolvedType.InternedKey != 0

Coordinator
Jan 16, 2011 at 10:58 PM

The Dummy type itself is not internal. So you'd do mytypeRef.ResolvedType == Dummy.TypeReference or !(mytypeRef.ResolvedType is Dummy).

The threading problem should now be fixed.

Jan 17, 2011 at 6:25 AM
hermanv wrote:

The Dummy type itself is not internal. So you'd do mytypeRef.ResolvedType == Dummy.TypeReference or !(mytypeRef.ResolvedType is Dummy).

I think, !(mytypeRef.ResolvedType is Dummy) is not correct. The Dummy.Type return instace of DummyType. But DummyType is not subtype of Dummy.
The threading problem should now be fixed.
OK, thank you.
Coordinator
Jan 17, 2011 at 2:54 PM

There has been a recent change that made all of the internal DummyXXX types be subtypes of the public Dummy type.

Jan 18, 2011 at 8:07 AM
Edited Jan 18, 2011 at 8:08 AM

The current fix solves the safety, but it might stop property getter inlining, thus lowering the performance.

I would suggest using private static readonly fields and construct instances inline at declaration. There are not so many fields, anyway.

These will compile into a type initializer that is thread-safe by definition.