2

The documentation of the lock statement is pretty straightforward:

lock (x)
{
    // Your code...
}

where x is an expression of a reference type.

So I should not be allowed to pass a value type as locker for a lock. I noticed though that I can use a value type that implements an interface. In other words I can do this:

IDisposable locker = default(DisposableStruct);
lock (locker) Console.WriteLine("Thread safe");

struct DisposableStruct : IDisposable
{
    public void Dispose() { }
}

This was surprising. I figured out that the reason is that the value type is boxed. According to the documentation:

Boxing is the process of converting a value type to the type object, or to any interface type implemented by this value type.

My question is if there are any caveats with using boxed value types as lockers for the lock statement. Is there any possibility for the reference-type wrapper to change somehow during the execution of the program, causing the thread-safe code to fail?

Update: Here is an example of what worries me. Is it guaranteed that if I run the code bellow a million times, the correct output (1,000,000,000) will always be displayed?

IComparable<int> boxedValueType = 0;
int sharedState = 0;
var tasks = Enumerable.Range(0, 10).Select(_ => Task.Run(() =>
{
    for (int i = 0; i < 100_000_000; i++)
        lock (boxedValueType)
            sharedState++;
})).ToArray();
Task.WaitAll(tasks);
Console.WriteLine(sharedState);
Theodor Zoulias
  • 34,835
  • 7
  • 69
  • 104
  • Never use a lock object that does anything else than serve as a lock object. `private readonly object lock = new object();` Having had to debug code that heavily used anything else as locks was a nightmare and ended in replacing all locks with dedicated lock objects. – Fildor Jul 03 '19 at 06:31
  • 1
    I think you are overthinking `lock` statement. – SᴇM Jul 03 '19 at 06:32
  • _"When you synchronize thread access to a shared resource, lock on a dedicated object instance (for example, private readonly object balanceLock = new object();) or another instance that is unlikely to be used as a lock object by unrelated parts of the code."_ from [lock statement](https://learn.microsoft.com/en-us/dotnet/csharp/language-reference/keywords/lock-statement#remarks). I found the second part ( "or another instance that is unlikely to be used as a lock [elsewhere]") to be **very** unhelpful. I'd _always_ go for a dedicated object. – Fildor Jul 03 '19 at 06:34
  • @Fildor yesterday I wrote [a method](https://stackoverflow.com/a/56862796/11178549) that needs thread synchronization, and I already have a variable of type `IEnumerator>` in the scope of the method. It is impossible for any other part of the program to get a reference to this variable. I also know that some enumerators are structs ([example](https://learn.microsoft.com/en-us/dotnet/api/system.collections.generic.list-1.enumerator)). I ended up using this variable as a locker. Do you think it's a bad idea and I should use a dedicated `new object()` instead? – Theodor Zoulias Jul 03 '19 at 06:49
  • I would. It will be disposed in case of an exception. I wouldn't even want to have to think through the possible consequences. – Fildor Jul 03 '19 at 06:57
  • @Fildor by not knowing the consequences of certain coding decisions you risk writing overly defensive code, that ends up more bloated than it needs to be. This is the reason I made this technical question, to learn the consequences and so be able to write better code with confidence. – Theodor Zoulias Jul 03 '19 at 07:05
  • There's an obvious huge caveat -- boxing a value type instance twice *does not yield the same object reference*, making it extremely simple to bollix things up by not noticing you should have reused a particular boxed instance. This is the exact reason why locking on value types is outright forbidden (it never makes sense), and while locking on an explicitly boxed value type is not forbidden (as there's no way to consistently detect it at compile time and it *might* make sense) it's almost as dangerous. – Jeroen Mostert Jul 04 '19 at 12:27
  • @JeroenMostert is it guaranteed though that if you box a value type once, and then lock on this boxed value a billion times, you'll always lock on the same object reference? I updated my question with sample code demonstrating this case. – Theodor Zoulias Jul 04 '19 at 17:32
  • 1
    Yes, that's guaranteed. It could not be otherwise -- that would imply that a managed reference to an object would suddenly change its stored value. The runtime has no mechanism for that, and it would break code that tested for object equality by reference (among many other things). Once you *have* the `IComparable` and hold on to it, it's just as good an object to lock on as any other reference object. The conceptual description of boxing found in the [spec](https://docs.microsoft.com/dotnet/csharp/language-reference/language-specification/types#boxing-and-unboxing) matches this. – Jeroen Mostert Jul 04 '19 at 19:07
  • 2
    The major problem/weakness of this approach, of course, is that it's very easy to inadvertently create a *different* boxed instance and lock on that somewhere else. If you are in control of all the code (the code that boxes and the code that locks) then you can avoid getting bitten, but because it's so easy to create inadvertent copies of value types, and so easy to implicitly box them, it looks dangerous even when it's safe. I'd accept the "overhead" of a `new object()` every time because that's a pattern that's *obviously correct*, as opposed to *not obviously incorrect*. – Jeroen Mostert Jul 04 '19 at 19:13
  • @TheodorZoulias *It is impossible for any other part of the program to get a reference to this variable.* You are calling `enumerator.MoveNext()` and `enumerator.Current` in your code. By doing that you are giving reference to the *other part of the program*. How do you know that it did not lock on it? – user4003407 Jul 04 '19 at 19:31
  • @JeroenMostert thanks for the informative comments! If you wish post them as an answer so that I can accept it. – Theodor Zoulias Jul 04 '19 at 20:28
  • @PetSerAl you are right that technically is is possible for the `IEnumerable` that provided the `IEnumerator` to hold a reference to the enumerator, and then issue locks in it. But this would be a very wicked thing to do! Also it would only be an issue if the enumerator is reference type. If it's a value type then the provider cannot hold a reference to it. In that case I am locking on a boxed wrapper of the enumerator, that is created inside my method, and no other part of the program knows about it. – Theodor Zoulias Jul 04 '19 at 20:36
  • 1
    @TheodorZoulias I am not talking `IEnumerable` having reference to `IEnumerator`. I am taking that any instance method of reference type have implicit `this` parameter, and thus can do `lock(this)`. So, if you calling any instance method (which can include instance constructor, if it is not trivial) of reference type, then you are not only owner of that reference. It is much harder to accidentally expose the reference, when it is private object, which is not used for anything else, but locking. Also I see no reason why boxing will take less resources than `new object()`. – user4003407 Jul 04 '19 at 21:18
  • @PetSerAl I see your point. I am not trying so much to preserve resources, as to keep my code clean. For this reason I have adopted the habit of reusing local variables of things like `List` as lockers for accessing the same things. For example `lock (list) list.Add(x)` seems more streamlined to me than `lock (lockerOfList) list.Add(x)`. But your analysis is making me to reconsider my practices. – Theodor Zoulias Jul 04 '19 at 22:59
  • _"not knowing the consequences of certain coding decisions"_ - I did not say that. I know them pretty well. That's why I recommend _not_ to do, what you did here. – Fildor Jul 05 '19 at 11:04
  • @Fildor you said that you wouldn't even want to think about the consequences of locking on a disposable object. I guess that this means that you don't know the consequences. – Theodor Zoulias Jul 05 '19 at 11:24
  • :D Not exactly. I meant basically what Jeroen said. I use a dedicated object and know I am safe. And my coworkers won't accidentally break it. I consider this even more "clean" than reusing something as a lock. It is readable, it is correct and other coders won't be surprised. Is this "defensive" ? - maybe. But I can focus on the task, not on the tools while the disadvantages are negligible. – Fildor Jul 05 '19 at 13:25
  • @Fildor these are all fair points. I get it that locking on a non-dedicated object is risky for more than one reasons. But since you mentioned the case of disposable objects, lets not leave it unanswered. From what I know the consequences of locking on a disposable object are ... none. A disposed object is as good as any other object for locking purposes. – Theodor Zoulias Jul 05 '19 at 13:47
  • 1
    Yes, I meanwhile looked it up. Disposing is in fact no problem. – Fildor Jul 05 '19 at 13:58

0 Answers0