# Parallel locking construct

I needed an object I can use to serialize specific operations in an otherwise parallel workflow.

The specific use case I was solving for is request idempotency; a great example would be double-clicking a "Process Payment" button on a web form. I might have 100 payment processing requests in flight (parallel process) at any given time, but if two or three come in with the same credit card info, I want to make sure I process them serially (so I can deduplicate, etc.). In that case, a suitable key value might be the credit card number.

I came up with the Keybox class below. (Serializer is so overloaded...)

public class Keybox : IKeybox
{
private readonly HashSet<string> Keys = new HashSet<string>();

public void Lock(string key, int timeout = 60, int tick = 100)
{
//timeout is specified in seconds, tick in milliseconds
//timeout = ConvertTimeoutToTicks(timeout, tick);
//This logic is not important for the question
timeout = 600;

var @event = new ManualResetEvent(false);
var counter = 0;
do
{
@event.Reset();

if (!Keys.Contains(key))
{
lock (Keys)
{
if (!Keys.Contains(key))
{
return;
}
}
}

.ContinueWith(t => @event.Set());

@event.WaitOne();
} while (counter++ < timeout);

throw new Exception($"Unable to process '{key}'; timeout expired."); } public void Unlock(string key) { Keys.Remove(key); } } Test code: public void Test() { var box = new Keybox(); var key = "EFF82671-DE8F-46B9-B1F1-0FB8308052C3"; Parallel.ForEach( Enumerable.Range(1, 3), (i) => { box.Lock(key); Console.WriteLine($"{i}: Running");
Console.WriteLine(\$"{i}: Ending");
box.Unlock(key);
}
);
}

which produces output like

1: Running
1: Ending
3: Running
3: Ending
2: Running
2: Ending

The main question is: is this an implementation of a specific design pattern? And the follow-on to that question is: is there a BCL class that already offers this functionality?

I'm also interested in general code review feedback.

public void Lock(string key, int timeout = 60, int tick = 100)
{
...
//timeout = ConvertTimeoutToTicks(timeout, tick);
...
timeout = 600;

Looks like you didn't finish tidying up the code.

//timeout is specified in seconds, tick in milliseconds

Why perpetuate the common mistake in the standard libraries? TimeSpan exists: use it! I wish Microsoft would [Obsolete] every method in .Net which takes an int or a long for a timespan and replace it with a version taking a TimeSpan.

var counter = 0;
do
{
...
} while (counter++ < timeout);

Ok, now I'm even more confused. Is timeout supposed to be a timeout or a multiplier of tick?

if (!Keys.Contains(key))
{
lock (Keys)
{
if (!Keys.Contains(key))
{
return;
}
}
}

Just no. .Net's memory model fixes the subtle double-checked locking problem in Java, but only where the test is a simple boolean field. HashSet cannot guarantee that multi-threaded access will work correctly if one of the threads modifies it, so the unlocked test must be removed. However, I can offer an improvement which should compensate: use the return value of Add:

lock (Keys)
{
{
return;
}
}

Similarly, Unlock should wrap the current body in a lock statement.

What's the lifecycle of that ManualResetEvent? It's IDisposable, so you should take care of tidying it up yourself, probably with a using statement.

• +1. Fantastic advice about using TimeSpan instead of int or long. FWIW, the commented lines were indeed tidied up, per this site's admonition to "post working code samples" where possible. – arootbeer May 23 '17 at 13:03
• This is the first time I've worked with WaitHandles, and I thought about checking whether it was IDisposable after I put the code up for review, but didn't get back to it. – arootbeer May 23 '17 at 13:03