Forward
I would love any comments you have, any ideas, any flaws you can find, and any suggestions you might have regarding the code below. If this is similar to other implementations, I would love to know about them, since this is something I came up with by myself, after not finding any solution 'out there', but I know there is nothing new here, just my way of doing things.
Also, this is my first time posting here, so please go easy on me if I did it wrong.
Description
Writing code for windows WinForms application, usually entails working asynchronously, and sometimes throwing some threads in the air, so that the UI will still be responsive, IO will happen in the background, data will stay fresh, and your users will love instead of hate you for the bugs you throw at them.
C# has the 'lock' keyword to help with creating Atomic operations, but what I find frustrating, is that most often then not, the data itself need to be guarded, but the sync syntax can be easily forgotten.
So I've created the following code for myself to manage object locking, via generics and extension methods, which allow me to easily wrap any data type I need to be kept safe, and use it only via the Synced context.
First the extension methods (or rather a single extension method with overloaded parameter signature to make it's use more natural) DoLocked
.
DoLocked will perform a locked action, after verifying that the object is 'ready for work', with the option to 'wait' (or not) on the lock, will release the lock when the action has completed (or in case of an exception), and will let you know if the action was triggered or not.
So basically any object, for ex. ComplexClass complexObject
, you want to performed an operation that is locked (with all the features mentioned above), the code will look something similar to:
complexObject.DoLocked(syncObj,
() => { if( isReady ) return true; },
true,
obj => {
...
obj.Method();
...
obj.Property = value;
...
};
the getIsReady is a Func which tests readiness to enter the read (before even attempting to wait for the lock), when null, it is 'always ready'.
Next step is the Synced generic class, which will hold the syncObject along with the content (now locked) object. The idea is not to abstract the internal object, but wrap it in a package that makes it clear how to use it right.
The Code:
using System;
using System.Collections.Generic;
using System.Linq;
using System.Text;
using System.Threading;
namespace IdleSharp.Sys.ThreadSafe
{
public static class SyncedDoExt
{
private static readonly Random random = new Random();
private static bool alwaysReady() { return true; }
// A note about the parameter order.
// Usually, it is preferred to have the most common param in the first (left most) postion.
// However, since the common argument is an action, which will likely be an in line lambda
// expression more often then not, I have decided to place the action always last.
// This is to make calls to DoLocked with multi-lines lambda inline methods more readable.
public static bool DoLocked<T>(
this T subject, object syncObj,
Func<bool> getIsReady, bool wait, Action<T> action
) where T : class
{
System.Diagnostics.Trace.Assert(
null != syncObj, "SyncObject must not be null!!!");
bool lockWasTaken = false;
try
{
Monitor.TryEnter(syncObj,
wait ? Timeout.Infinite : 0, ref lockWasTaken);
if (!lockWasTaken) return false;
getIsReady = getIsReady ?? alwaysReady;
// My original version
//while (lockWasTaken && wait && !getIsReady())
//{
// Monitor.Exit(syncObj);
// System.Threading.Thread.Sleep(random.Next(20, 70));
// lockWasTaken = false;
// Monitor.TryEnter(syncObj, Timeout.Infinite, ref lockWasTaken);
//}
//
// Code implementing Trevor Pilley's suggestion to use SpinWait instead
// After better understanding it with the help of:
// http://www.albahari.com/threading/part5.aspx#_SpinLock_and_SpinWait
if (!getIsReady()) System.Threading.SpinWait.SpinUntil(() =>
{
Monitor.Exit(syncObj);
lockWasTaken = false;
Monitor.TryEnter(syncObj, Timeout.Infinite, ref lockWasTaken);
return lockWasTaken && getIsReady();
});
// I now see that testing getIsReady here again is unnecessary, commenting it out.
// if (getIsReady())
// {
action(subject);
return true;
// }
return false;
}
finally
{
if (lockWasTaken) Monitor.Exit(syncObj);
}
}
// Alternative parameters overloading
public static bool DoLocked<T>(
this T subject, object syncObj, Func<bool> getIsReady, Action<T> action
) where T : class
{
return DoLocked(subject, syncObj, getIsReady, false, action);
}
public static bool DoLocked<T>(
this T subject, object syncObj, bool wait, Action<T> action
) where T : class
{
return DoLocked(subject, syncObj, (Func<bool>)null, wait, action);
}
public static bool DoLocked<T>(
this T subject, object syncObj, Action<T> action
) where T : class
{
return DoLocked(subject, syncObj, (Func<bool>)null, false, action);
}
// The method signatures below are the same as above, except that the subject object
// itself is also the lock object
public static bool DoLocked<T>(
this T subject, Func<bool> getIsReady, Action<T> action
) where T : class
{
return DoLocked(subject, subject, getIsReady, false, action);
}
public static bool DoLocked<T>(
this T subject, bool wait, Action<T> action
) where T : class
{
return DoLocked(subject, subject, (Func<bool>)null, wait, action);
}
public static bool DoLocked<T>(
this T subject, Action<T> action
) where T : class
{
return DoLocked(subject, subject,(Func<bool>)null, false, action);
}
}
// Easy container to hold lock objects with content objects, allowing a uniform
// access abstraction.
public class Synced<T> where T: class
{
public bool AutoWait { get; set; }
object _syncLock;
public T InnerObject { get; private set; }
public Synced(T source, object syncLock, bool autoWait)
{
AutoWait = AutoWait;
_syncLock = syncLock;
InnerObject = source;
}
public bool DoLocked(Action<T> action)
{
return InnerObject.DoLocked(_syncLock, null, AutoWait, action);
}
public bool DoLocked(bool wait, Action<T> action)
{
return InnerObject.DoLocked(_syncLock, null, wait, action);
}
public bool DoLocked(Func<bool> getIsReady, Action<T> action)
{
return InnerObject.DoLocked(_syncLock, getIsReady, AutoWait, action);
}
}
// Non-generic static class, for easy creation of Synced<T> objects.
public static class Synced
{
public static Synced<T> MakeSynched<T>(
T source, object syncLock, bool autoWait
) where T : class
{
return new Synced<T>(source, syncLock, autoWait);
}
public static Synced<T> MakeSynched<T>(
T source, object syncLock
) where T : class
{
return new Synced<T>(source, syncLock, false);
}
public static Synced<T> MakeSynched<T>(
T source, bool autoWait
) where T : class
{
return new Synced<T>(source, source, autoWait);
}
public static Synced<T> MakeSynched<T>(T source) where T : class
{
return new Synced<T>(source, source, false);
}
}
}
getIsReady
is actually used? Why would you try to acquire the lock when business logic is not ready for it? \$\endgroup\$