Write lock for properties

[crener]

Wouldn't it be great if you had a lock which let multiple read actions into a critical section but blocked read actions once a write action is queued, this would help reduce the overhead of locks on commonly accessed resources by minimising the amount of waiting threads. In order to define these sections there needs to be a clear indication of intention as to whether a value is being written or read, such as proportions.

lock int Value
{
     get { return someClassValue; } 
     set { someClassValue = value; } 
}

So by adding the lock before a normal property deceleration means that multiple threads can enter the getter but once a thread wants to enter the setter it blocks new threads entering either set or get until the setter thread leaves the setter method. If threads are already in the getter methods when a thread wants to set it waits until all threads leave.

[benaadams]

ReaderWriterLockSlim?

https://docs.microsoft.com/en-us/dotnet/api/system.threading.readerwriterlockslim?view=netcore-2.1

[crener]

ReaderWriterLockSlim?

Ooooh I googled around a bit but somehow didn't find that...
That's exactly the functionality I'm describing. I still think there is value in adding it to properties as it would reduce the amount of code required to get the same functionality without limiting flexibility but maybe that's just me?

[Joe4evr]

I still think there is value in adding it to properties as it would reduce the amount of code required to get the functionality without

I disagree. Anything of that nature is domain-specific and should therefor be done as explicitly as possible. The language as a whole shouldn't be in the business of promoting one specific synchronization mechanism, since that'll lead to people not thinking about which one is actually best suited to their domain (a mistake of the lock statement, IMO).

[Neme12]

Also, sooner or later you will need to add another piece of code in that property outside the lock and you'd have to abandon this syntax altogether.

[svick]

This sounds very similar to MethodImplOptions.Synchronized in .Net and synchronized in Java (except for the reader-writers part). As far as I know, both are generally considered a bad practice. I don't think it makes sense to expand such practices.

[whoisj]

Would be nice if the lock keyword could be used to acquire and release lock objects, in much the same way it works for Monitor.

Instead of say...

        private readonly Mutex _mutex;

        public Method() {
            _mutex.WaitOne();
            try {
                // operation under lock
            } finally {
                _mutex.ReleaseMutex();
            }
        }

One would be able to...

        private readonly Mutex _mutex;

        public Method() {
            lock(_mutex) {
                // operation under lock
            }
        }

... it is just so much cleaner and easier to read.

[HaloFour]

@whoisj

using System;
using System.Threading;

public static class MutexExtensions {
    public static AcquiredMutex Lock(this Mutex mutex) {
        mutex.WaitOne();
        return new AcquiredMutex(mutex);
    }
    
    public class AcquiredMutex : IDisposable {
        private readonly Mutex mutex;
        
        public AcquiredMutex(Mutex mutex) => this.mutex = mutex;
        
        void IDisposable.Dispose() => mutex.ReleaseMutex();
    }
}

public class C {
    public void M(Mutex mutex) {
        using (mutex.Lock()) {
            // operation under lock
        }
    }
}

With #1623 we could avoid the allocation as well.

[whoisj]

@HaloFour I use that pattern all of the time, but it is still a work around for a "missing" language feature.

[svick]

@whoisj That code already compiles. Even though it does the wrong thing, I think it would be considered a breaking change to make it do the right thing.

@HaloFour If you change AcquiredMutex to a struct, using won't box it to call its Dispose (it uses constrained.callvirt instead). So you can already avoid that allocation.

[HaloFour]

@whoisj

I use that pattern all of the time, but it is still a work around for a "missing" language feature.

Maybe. It works well enough for me and I have extension methods defined for most of the synchronization classes in the BCL or other major libraries.

The issue with adding any kind of structural support to lock is that it would change existing code since lock (mutex) is perfectly legal today. Probably the only way to mitigate that would be to have the "structure" defined by a specific interface or shape that would be added alongside the feature so at least that potential ambiguity is very limited. The other problems are that each synchronization type works a little differently and that lock can't convey the other policy you often likely want to incorporate. It's very simple to add an overload above to pass a timeout.

@svick

So there will be no allocations.

Oh that's nice to know. I often use Rx's Disposable.Create for these simple cases but it might be worth porting over to structs.

[whoisj]

@whoisj That code already compiles. Even though it does the wrong thing, I think it would be considered a breaking change to make it do the right thing.

I know. Language flaw - doubt it can be overcome. 😞 Would have been nice if all of the language features relied on interfaces the way using does.

• lock could have been ILockable relying on ILockable.AcquireLock() and ILockable.ReleaseLock().
• fixed could have been IPinnable relying on IPinnable.GetGcHandle().

Pipe dreams, I know.

[benaadams]

fixed could have been IPinnable relying on IPinnable.GetGcHandle().

It now binds to ref [readonly] T GetPinnableReference() in C# 7.3 https://github.com/dotnet/csharplang/blob/master/proposals/csharp-7.3/pattern-based-fixed.md

[yaakov-h]

If lock relied on a shape or interface then you wouldn’t be able to lock on any of the objects that you can today.

[whoisj]

It now binds to ref [readonly] T GetPinnableReference() in C# 7.3 https://github.com/dotnet/csharplang/blob/master/proposals/csharp-7.3/pattern-based-fixed.md

WHAAAT?! 😁 😍 OMG OMG OMG... I need a cold shower, I'll be back in a bit. 😄

[Joe4evr]

@svick

If you change AcquiredMutex to a struct, using won't box it to call its Dispose

Now, if only that worked when the object is purely exposed as IDisposable. Oh well, guess that's a bit too much to expect.

[Korporal]

@crener - Your example isn't very representative though. You gain nothing by locking around a single field accessor like you do in that property. A lock is ideally used to avoid state being inconsistent while some algorithm runs, in that example you'd see no difference between using a lock or not using a lock.

[Korporal]

@whoisj - It seems that the C# lock was designed to help with locking within a single AppDomain which is why it's content to use (under the hood) a Win32 CriticalSection. Concurrent access across > 1 AppDomain is where a thing like a Mutex would come in. I guess concurrency across multiple AppDomains is something that has no inherent language support.

[bbarry]

@yaakov-h lock does depend on a shape and @whoisj you could rewrite it as a using syntax; it does use aquire and release semantics, they are just on a bit that is hidden from normal type operations we regularly use outside of locking:

public static class LockingExtensions {
    public static AcquiredLock Lock(this object lockobj) {
        Monitor.Enter(lockobj);
        return new AcquiredMutex(lockobj);
    }
    
    public readonly struct AcquiredLock : IDisposable {
        private readonly object _lock;
        
        public AcquiredLock(object _lock) => this._lock = _lock;
        
        void IDisposable.Dispose() => Monitor.Exit(_lock);
    }
}

[whoisj]

@bbarry thanks, as mentioned to @HaloFour above I'm aware of that pattern, it's just extra boilerplate which I seem to have to add to every project. 🤷‍♂️