Enforce mutex contract

[qwwdfsad]

No description provided.

[dkhalanskyjb]

I'd like to question the premise of this PR. In essence, I disagree that #2401 is necessarily a bug.

First, an abstract discussion. null is a special value, so it is expected that it may be treated differently. In a Mutex, both lock and unlock can accept it. We, therefore, need to define the behavior of the four possible balanced pairs of calls to lock and unlock:

• lock(null); unlock(null) is supposed to work fine.
• lock(owner1); unlock(owner2) is ok iff owner1 === owner2. Also no questions here.
• lock(null); unlock(owner). We'll say that X is true iff this succeeds.
• lock(owner); unlock(null), the subject of this PR. We'll say that Y is true iff this succeeds.

We can then draw a table with the possible values of X and Y:

1. X and Y are both true. This means that null has the semantics of "this call has nothing to say about the ownership".
2. X and Y are both false. This is what's being proposed here. Then null has no special semantics, and the call to unlock matching a lock requires exactly the same value.
3. X is true, Y is false. Then null has the semantics of the least significant owner, someone whose rights can be freely violated.
4. X is false, Y is true. This is the case currently. Then null has the semantics of the most significant owner, someone who can take away anyone's lock.

Each of these options is semantically simple and clear, so I think that the only way to elevate one option over another is to evaluate the use cases.

Now, for the argument that option 4 can be more useful in some cases than option 2. For the common case of balanced lock and unlock calls guarding a code segment, the users should stick to mutex.withLock. If they have to resort to the functionality affected by this PR, their use case is already fancy and they probably store a reference to a mutex as a field somewhere, possibly in a class that implements Closeable and releases the mutex on a close. In that case, I don't think it's inconceivable that the mutex may have varied owners, possibly changing throughout its lifetime (this is tricky to implement, but can be done correctly). Then the easiest way to implement close() is to call a wildcard unlock(null).

So, I think that unlock(null) having a special behavior can be useful in practice, and it's possible that such functionality is in use in places where it's cumbersome to replace it. Then this PR would break that functionality for reasons that are unclear to me.

I found an interesting tidbit in #2401:

This behavior becomes even trickier when the owner is operated as Any? in the user code, increasing the chances for null to sneak in.

This is a good point, but isn't it possible to fix this with changes that are less breaking? For example, by replacing unlock(owner: Any?) with unlock() + unlock(owner: Any)? This would require some actions on part of the users affected by the problem we're discussing here, but this doesn't seem like a problem to me.

The bottom line: could you please describe why this change is beneficial? Is it beneficial enough to warrant breakage in runtime in places where programmers consciously used the wildcard property of unlock(null)?