'lock' syntax improvement

[andrey-messi]

See an example here https://docs.microsoft.com/en-us/dotnet/csharp/language-reference/keywords/lock-statement .

private Object thisLock = new Object();
here thisLock is just an object for technical needs and it should not be necessary to explicitly declare thisLock because we have only one function that changes a balance (we should not synchronize many functions). Then it can be done implicitly just by writing a lock without parameter:
lock {...}
instead of
lock (thisLock) {...}
(a compiler can implicitly create a global static unaccessible from my code field shared between all 'locks without parameter' in my solution)

If I write
lock (new object())
then there is no locking at all so all threads can execute code inside lock (new object()) simultaneously. Obviously a compiler should display an error or at least a warning, but it displays nothing.

for example we have to add another function that changes balance by another rule, so we have 2 functions that change a balance. Then we can not use 'lock without parameter' syntax because we need to synchronize 2 functions. So we create a field
private Object thisLock = new Object();
and use
lock (thisLock) {...}
in every function. But it seems much more intuitively to use
lock (balance) {...}
because both functions changes a balance and we syncronize them for a balance never being less than zero (and we don't create a technical field thisLock that pollute our code). But we can't use lock (balance) because balance is an integer (value type) and lock (that is Monitor.Enter) have only 'object' (reference type) parameter. I don't know why it can not use a value type but it seems for me that it should be possible to use value type in this situation. Please explain me if I am wrong.

[svick]

it should not be necessary to explicitly declare thisLock because we have only one function that changes a balance (we should not synchronize many functions). Then it can be done implicitly just by writing a lock without parameter

Synchronizing more than one function is very common and I don't understand why it would be something that should not be done. On the contrary, I'd expect that using a lock object only in a single method is the less common case.

And I think that making it clear on which object you're locking is a good thing. Writing correct concurrent code is hard, and while this suggestion would make it more succinct, I think it would also make it harder to understand, which is not a good thing.

a compiler can implicitly create a global static unaccessible from my code field shared between all 'locks without parameter' in my solution

Absolutely not. The result of this would be unnecessary contention and possibly even deadlocks.

If I write lock (new object()) then there is no locking at all so all threads can execute code inside lock (new object()) simultaneously. Obviously a compiler should display an error or at least a warning, but it displays nothing.

What makes you think this is a common error? Though it sounds like a decent candidate for an analyzer.

But we can't use lock (balance) because balance is an integer (value type) and lock (that is Monitor.Enter) have only 'object' (reference type) parameter. I don't know why it can not use a value type but it seems for me that it should be possible to use value type in this situation.

An int is represented using just the 4 bytes, there is no space in there for the additional bookkeeping required for lock. On the other hand, any reference type includes an object header, which is what allows you to use lock with them.

[andrey-messi]

I don't understand why it would be something that should not be done.

I didn't mean to forbid "Synchronizing more than one function". I just meant to add a syntax sugar for the situation when you had not to synchronize more that 1 function (just like in an example at https://docs.microsoft.com/en-us/dotnet/csharp/language-reference/keywords/lock-statement ).

An int is represented using just the 4 bytes, there is no space in there for the additional bookkeeping required for lock

But I think it is theoretically possible for them to have that space (if CLR-developers had thought about it while starting to architect CLR)?

[HaloFour]

I'm with @svick. I think that offering such a short-hand would make it too easy to do the Wrong Thing™ and make bad assumptions regarding the scope of the synchronization context. In languages like Java where synchronized can be very easily applied to method signatures it's too common to see code like this and the result is contention and/or deadlocks. I think by forcing the developer to use an instance that they have to maintain forces them to put some thought into the scope of the synchronized block and how much other code will be impacted.

But I think it is theoretically possible for them to have that space (if CLR-developers had thought about it while starting to architect CLR)?

No. An int is 32-bit to the CPU. The CLR can't change that. Not without totally wrecking all native arithmetic operations and tanking performance.

Locking on any value type makes no sense. Value types exhibit copy-by-value semantics meaning every assignment is a completely new copy of all of the data for that value type. Even if there was extra space there for some reason any data written into that space would only exist on that copy. It'd be lost to the caller of the method performing the lock to begin with. You'd need the CLR to maintain a completely different mechanism through which to manage the bookkeeping which would be external to the value type. It's simply not a problem worth solving.

[svick]

I just meant to add a syntax sugar for the situation when you had not to synchronize more that 1 function (just like in an example at https://docs.microsoft.com/en-us/dotnet/csharp/language-reference/keywords/lock-statement ).

I think that example shows why synchronizing just one method is not common: most bank accounts support not only withdrawals, but also deposits or checking your balance. While that code might be good enough as a documentation example, I don't think it's representative of real code.

But I think it is theoretically possible for them to have that space (if CLR-developers had thought about it while starting to architect CLR)?

A lot of things are possible in theory. That doesn't mean they are practical (doubling memory requirements for int would be a big deal) or that they make sense (how would it behave for ints that are local variables?).

[andrey-messi]

(how would it behave for ints that are local variables?).

It would be the same as lock(new object()) , i.e. no syncronizing. It would make a sence only if int is a field. And a field that is really synchronized like 'balance' in msdn example. There no need to use another int (technical variable) as lock parameter because the idea is to remove technical field 'thisLock'.

[HaloFour]

@andrey-messi

It would make a sence only if int is a field.

It doesn't matter if an int is a field or a local. In both cases the data is copied onto the stack and the called function has no concept of where that data originated. Nor can the called function modify the original data in any way. All it can do is modify its copy.

I also want to mention that while you may think that lock (balance) {...} feels more intuitive, it's actually very incorrect. That implies the notion of locking data, which is very much not what you're doing. You're locking sections of code. That fact that the code in question might be modifying balance isn't relevant (although the fact that it is modified makes it completely unsuitable for synchronization, even if value types were somehow supported.)

[andrey-messi]

although the fact that it is modified makes it completely unsuitable for synchronization,

Why ?

[mikedn]

That implies the notion of locking data, which is very much not what you're doing. You're locking sections of code.

This sounds rather odd, it's actually data that is being locked and not code. Well, sort of.

The whole point of locking is to ensure exclusive access to a shared resource. The resource can be anything (memory, file etc.) but in the typical case it's memory. Or, in high level, language specific terms - variables. So yes, you're not locking "data" as that's what you're storing in memory/variables. But you're not locking code either. It's just that there's no way to actually lock memory for exclusive access so this is done cooperatively, by having all pieces of code that read/update the shared resource acquire the lock that corresponds to the shared resource.

From the above you can also derive a few interesting observations:

• Locking on a value doesn't make sense since that's "data" stored in a variable. If you really want, you could have something like lock (ref balance) that would mean "give me exclusive access to the variable balance" and not "give me exclusive access to the value of variable balance" which doesn't make sense. But it's quite difficult to implement such an address based lock so this just won't happen.
• This shared memory resource is not necessarily represented by a single variable. There can be multiple variables involved and then it's not clear which one is supposed to be used as a lock. You'd need to pick up one of them and make sure it's used consistently as a lock. At this point it seems preferable to use a dedicated variable and object such as thisLock in order to avoid confusion.
• As mentioned, shared resources do not necessarily consist of memory/variables. It's rare for lock to be used in such cases but if it's used you'd need a thisLock style object anyway.

[whoisj]

Time to read up on CRITICAL_SECTION and CONDITION_VARIABLE.

[bondsbw]

Sometimes one method is the only place the resource is used. But it can be called on multiple threads simultaneously, so the method needs to lock the resource only from itself.

Something like this:

private void AMethod()
{
    lock
    {
        // use A resource
    }
}
private void BMethod()
{
    lock
    {
        // use B resource
    }
}

could be sugar for something like:

private static System.Object <AMethod>k__lockingField = new System.Object();
private void AMethod()
{
    lock (<AMethod>k__lockingField)
    {
        // use A resource
    }
}

private static System.Object <BMethod>k__lockingField = new System.Object();
private void BMethod()
{
    lock (<BMethod>k__lockingField)
    {
        // use B resource
    }
}

(For multiple locks in a method, there would be one field per lock.)

[CyrusNajmabadi]

(a compiler can implicitly create a global static unaccessible from my code field shared between all 'locks without parameter' in my solution)

This seems likely to not be useful. Having one global lock here is def not a recommended approach and i don't think should be something the language pushes people toward.

[mikedn]

Sometimes one method is the only place the resource is used

Locks are (or at least are supposed to be) rather rare. You can have a 100,000 lines program with only 10 locks in it. Locks that are only used in a single method are even more rare. It would be difficult to justify adding such syntax and the hidden field machinery it involves.

Besides, when it comes to thread synchronization it is preferable to be explicit. In this regard, even the fact that one can lock on arbitrary objects is pretty dubious. Having the compiler autogenerate lock objects is not any better.

[HaloFour]

@andrey-messi

Why ?

A value type's identity is its value. When you modify the value you overwrite the "instance" entirely. It's like calling new each time. It'd be about as useful as thisLock = new object() on every iteration.

@mikedn

The whole point of locking is to ensure exclusive access to a shared resource.

No, it's to ensure exclusive access to a section/block of code representing a logical atomic action. That code may (or may not) access/mutate shared state/resources. I think it's pretty poignant that the C# documentation on lock doesn't mention any kind of resources at all. But I do think that we're splitting hairs here and don't want to start a debate over semantics. I certainly agree with all of your other points.

Oddly the Java documentation seems to imply that synchronized has something to do with accessing an instance's fields, which is a patently untrue in Java as it is in .NET, but maybe that's why I so frequently see synchronized used incorrectly.

@bondsbw

I think it's a bad idea to have the compiler try to guess as to what you intend to protect between those methods and whether or not they are really isolated from one another. To be safe the compiler would/should have to lock both methods using the same synchronization context. This is what Java does, by locking on this, which is wrong on so many levels (and something that .NET docs state very clearly not to do.)

[mikedn]

I think it's pretty poignant that the C# documentation on lock doesn't mention any kind of resources at all.

The C# "documentation" about lock (and more generally, any MS written documentation about threading aspects in .NET) is the last thing I'd use in argument about threading. It tends to mix up some things, gloss other things and generally lack precision and clarity. Relying on such documentation results in people making absolutely ridiculous statements such as

No, it's to ensure exclusive access to a section/block of code representing a logical atomic action.

A word of advice - don't post such nonsense ever again. If you're not familiar with the subject that's being discussed then not saying anything is the best course of action.

[CyrusNajmabadi]

I think we can all keep the discussion civil, right?

[CyrusNajmabadi]

Sniping at people isn't helpful. We can all discuss this in a friendly and constructive manner. :) Let's table any sort of personal discussion and just address the subject at hand in a professional manner.

[mikedn]

I think we can all keep the discussion civil, right?

Of course. That includes people acting in responsible manner and not posting incorrect/misleading information. And admitting that they are wrong or not familiar with what is being discussed when that happens.

Unfortunately some here seem to think that commenting on each and every issue gives them the authority to claim that they are correct even if they can't offer any reasonable proof.

[HaloFour]

@CyrusNajmabadi

I agree.

@mikedn

I apologize for returning fire. I think that we're probably in agreement on most of these points and I'm probably mincing words here and there.

even the fact that one can lock on arbitrary objects is pretty dubious.

That is weird. Borrowed from Java, I assume, perhaps for supporting an implementation of synchronized methods. I'd think it make things easier to have a type dedicated to the purpose.

[svick]

@mikedn

The C# "documentation" about lock (and more generally, any MS written documentation about threading aspects in .NET) is the last thing I'd use in argument about threading. It tends to mix up some things, gloss other things and generally lack precision and clarity.

If you encounter something in the C# or .Net documentation on docs.microsoft.com that you think should be improved, feel free to create an issue in the dotnet/docs repo (or even submit a Pull Request). I'm sure the documentation team will appreciate it.

[andrey-messi]

I'd think it make things easier to have a type dedicated to the purpose.

May be not even a type. I could be good to have a new keyword 'locks' for a declaring all locks:
class Account { locks{lockObject1, lockObject2} private void AMethod() { lock(lockObject1) { ... } } private void BMethod() { lock(lockObject2) { ... } } }
If a keyword 'locks' in a class level then this locks can be accessible only in 'lock's from this class. If there are declared out of class then they are accessible from all classes. If nessecary they can even be declared in a method level and visible inside a method. And of cource they should be accessible only as lock parameter and not for other rows of code.

[CyrusNajmabadi]

May be not even a type. I could be good to have a new keyword 'locks' for a declaring all locks:

What problems would this be solving?

If a keyword 'locks' in a class level then this locks can be accessible only in 'lock's from this class.

That can already be provided today by just using "private".

And of cource they should be accessible only as lock parameter and not for other rows of code.

How is this valuable? People will still be able to 'lock' on non-'locks' objects. So this approach won't actually stop that from happening. :)

[andrey-messi]

People will still be able to 'lock' on non-'locks' objects

forbid to lock a regular field\variable and allow to lock only anyone from locks-section.

[CyrusNajmabadi]

Basically, what i'm really lacking here is a deep understanding of:

1. what problem(s) are we trying to solve here?
2. why are these actually problems?
3. why is this proposal the right way to even solve these problems (if we even accept they are problems)?

I'm not even past step '1' yet.

[CyrusNajmabadi]

forbid to lock a regular field\variable

This would be a massive breaking change and would never be allowed.

[CyrusNajmabadi]

forbid to lock a regular field\variable and allow to lock only anyone from locks-section.

@andrey-messi If you want thsi, you can write an analyzer for precisely this purpose. It would look like this:

class C
{
    [AndreyLock]
    private static readonly object myLock1 = new object();
}

Your analyzer would do whatever domain specific checks you wanted. For example, it could:

1. Check that you are only locking fields that have the AndreyLock attribute on it.
2. Check that you don't use any AndreyLock instances for any purpose other than locking.

You could even do it this way:

class C
{
    private static readonly AndreyLock myLock1 = new AndreyLock();
}

And do analysis on this. The point is: you don't need any language changes here. What you're describing is domain specific patterns for how you want your code to operate. THe best way to do this would be to just write your own analyzer.

[mikedn]

forbid to lock a regular field\variable and allow to lock only anyone from locks-section.

Apart from the fact that this would be a breaking change this is simply not what happens today. You're not locking on a field\variable, you're locking on an object. You'd be locking on a variable if you had something like lock (ref x), but you don't.

[mikedn]

May be not even a type.

The reason why it was suggested that a dedicated type might have been better are not in any way related to the lock syntax and any changes one my suggest around this syntax. Note that this syntax is just syntactic sugar for:

bool lockTaken = false;
try
{
    Monitor.Enter(thisLock, ref lockTaken);
    ... 
}
finally
{
    if (lockTaken)
        Monitor.Exit(thisLock);
}

No matter how you change the language the above code still works with arbitrary types.

Locking on arbitrary types is potentially problematic because it's easy to get things wrong and the documentation needs to calls this out - don't lock on this, don't lock on strings etc. None of this would have been necessary if locking was restricted to a specific type (e.g. System.Threading.Lock). But the ship has sailed long ago and nothing can be done about this.

Funnily enough, AOT framework such as .NET Native and CoreRT do have a Lock type under the covers and map arbitrary object instances to Lock instances by using a ConditionalWeakTable...

[MkazemAkhgary]

lock only statement becomes problematic when you want possibly more than one lock. it is not exactly known how you would want to manage those locks. for every scenario it could be different so lock only statement means is only covering one scenario and is not useful in general.

[mikedn]

@svick Thanks, I managed to find time to dump some of my thoughts on threading documentation in dotnet/docs#4992. I'd send a PR but I'm not confident enough that I can write good prose :)

[DavesApps]

@andrey-messi for your number 3 item if you take a look at this other feature request might handle locking on the variable name like you want, in a way that allows additional flexibility: See #892