7

I'm trying to work out how .NET Code Contracts interact with the lock keyword, using the following example:

public class TestClass
{
  private object o1 = new object();
  private object o2 = new object();

  private void Test()
  {
    Contract.Requires(this.o1 != null);
    Contract.Requires(this.o2 != null);
    Contract.Ensures(this.o1 != null);

    lock (this.o2) {
      this.o1 = new object();
    }
  }
}

When I run the Code Contract static analysis tool, it prints a a warning: Ensures unproven: this.o1 != null

If I do any of:

  • change the o2 in the lock to o1,
  • change the o1 inside the lock block to o2,
  • adding a second line inside the lock block assigning a new object to o2
  • change lock (this.o2) to if (this.o2 != null),
  • remove the lock statement entirely.

the warning disappears.

However, changing the line inside the lock block to var temp = new object(); (thus removing all references to o1 from the method) still causes the warning:

private void Test()
  {
    Contract.Requires(this.o1 != null);
    Contract.Requires(this.o2 != null);
    Contract.Ensures(this.o1 != null);

    lock (this.o2) {
      var temp = new object();
    }
  }

So there are two questions:

  1. Why is this warning occurring?
  2. What can be done to prevent it (bearing in mind this is a toy example and there's actually stuff happening inside the lock in the real code)?
Philip C
  • 1,819
  • 28
  • 50
  • Apparently Code Contracts doesn't consider the instantiation of `o1` that automatically occurs during constructor execution as sufficient evidence ensuring o1!=null. Ensures is a postcondition, and the `lock` implies that the method will be called from multiple threads. – Robert Harvey Apr 22 '13 at 15:40
  • 1
    @RobertHarvey But as I said, if you change the `this.o1 = new object()` into `this.o2 = new object()`, it's fine. It can't ensure anything in that case that can't also be ensured in the original case, and it still involves a lock. And the `Requires` should be enough evidence that `o1` isn't null at the start of the method. – Philip C Apr 23 '13 at 07:12

2 Answers2

2

Here's how the static checker treats locks and invariants:

If you lock something with the form lock(x.foo) { ... }, the static checker considers x.foo as the protecting lock of x. At the end of the lock scope, it assume that other threads might access x and modify it. As a result, the static checker assumes that all fields of x only satisfy the object invariant after the lock scope, nothing more.

Note that this is not considering all thread interleavings, just interleavings at end of lock scopes.

Manuel Fahndrich
  • 665
  • 4
  • 12
1

Add the following code to your class:

    [ContractInvariantMethod]
    private void ObjectInvariants()
    {
        Contract.Invariant(o1 != null);
    }

I couldn't tell you why, but in this case the static verifier does appear to consider private field assignment and an absence of methods that modify it as sufficient evidence that the field will not be modified. It could be a bug in the verifier? Might be worth reporting on the Code Contracts forum.

Stephen J. Anderson
  • 436
  • 3
  • 6
  • I can see how that would fix things, but in my real class, it's not actually an invariant - it's just always true before that method is called, and the method doesn't alter that fact. – Philip C Apr 24 '13 at 09:19