Waiting for a lock to release with ManualResetEvent and Quartz

Question

Follow-up to: Waiting for a lock to release with Thread.Sleep()?

I've found the time I tried to rewrite my WaitForLock-Method to utilize the Quartz.NET Scheduler which I've been using for some months now for other stuff. It got a little bit more complicated but at least it now misses the dreaded Thread.Sleep() completely, which is a big improvement for me.

Though, it might now be too complicated.

/// <summary>Waits for the lock to be released within the given timeout.</summary>
/// <param name="lockName">The name of the lock.</param>
/// <param name="timeout">The timeout in seconds.</param>
/// <returns>True if the Lock was released.</returns>
public bool WaitForLock(String lockName, Int32 timeout)
{
 // IsLocked(String) does query the database for the status
 if(!locker.IsLocked(lockName))
 return true;
 using(ManualResetEvent reset = new ManualResetEvent(False)
 {
 ScheduleLockWaiter(lockName, timeout, reset);
 reset.WaitOne(timeout * 1000);
 // WARNING: This overload is only available in: 4, 3.5 SP1, 3.0 SP2, 2.0 SP2
 // I spend a half day trying to figure that out.
 }
 return !locker.IsLocked(lockName);
}
/// <summary>Create and schedule the job to wait for the lock.</summary>
/// <param name="lockName">The name of the lock.</param>
/// <param name="repeat">The times it shall repeat.</param>
/// <param name="reset">The ManualResetEvent to report on.</param>
private void ScheduleLockWaiter(String lockName, Int32 repeat, ManualResetEvent reset)
{
 // Utilizing Quartz.NET
 String name = "LockJob_" + lockName;
 Trigger trigger = TriggerUtils.MakeSecondlyTrigger("LockTrigger_" + lockName, 1, repeat - 1);
 JobDetail job = new JobDetail(name, _lockJobGroup, typeof(LockJob));
 job.JobDataMap.Add("Locker", _locker);
 job.JobDataMap.Add("Reset", reset);
 job.JobDataMap.Add("LockName", lockName);
 if(_scheduler.GetJobDetail(name, _lockJobGroup) != null)
 _scheduler.UnscheduleJob(name, _lockJobGroup);
 _scheduler.ScheduleJob(job, trigger);
}
// Further down the road, our Job-Class
public class LockJob : IJob
{
 public void Execute(JobExecutionContext context)
 {
 ILocker locker = (ILocker)context.JobDetail.JobDataMap.Get("Locker");
 ManualResetEvent reset = (ManualResetEvent)context.JobDetail.JobDataMap.Get("reset");
 String lockName = (String)context.JobDetail.JobDataMap.Get("LockName");
 if(!locker.IsLocked(lockName))
 {
 context.Scheduler.UndscheduleJob(context.Trigger.Name, context.Trigger.Group);
 reset.Set();
 }
 }
}

I'm not quite sure if I have improved something, or created a beast which will devour me someday. Your thoughts on this?

Update: Since I still have the comment from Brian Reichle in my ears, I've moved on to make the waiting and acquiring of the lock one atomic operation.

Also the scheduled job has changed to directly acquire the Lock. Yes, I know that it tries to lock it twice on success (I just realized that, but did not see a way to change that).

/// <summary>Tries to acquire the Lock within the given timeout.</summary>
/// <param name="lockName">The name of the lock.</param>
/// <param name="timeout">The timeout in seconds.</param>
/// <returns>True if the Lock could be acquired.</returns>
public bool WaitForLock(String lockName, Int32 timeout)
{
 // boolean ILocker.Lock(String lockName)
 // Returns true if it was able to engage the lock.
 if(locker.Lock(lockName))
 return true; // Easy way out
 using(ManualResetEvent reset = new ManualResetEvent(False)
 {
 ScheduleLockWaiter(lockName, timeout, reset);
 reset.WaitOne(timeout * 1000, false);
 }
 return locker.Lock(lockName);
}
// Further down the road, our Job-Class
public class LockJob : IJob
{
 public void Execute(JobExecutionContext context)
 {
 ILocker locker = (ILocker)context.JobDetail.JobDataMap.Get("Locker");
 ManualResetEvent reset = (ManualResetEvent)context.JobDetail.JobDataMap.Get("reset");
 String lockName = (String)context.JobDetail.JobDataMap.Get("LockName");
 if(locker.Lock(lockName))
 {
 context.Scheduler.UndscheduleJob(context.Trigger.Name, context.Trigger.Group);
 reset.Set();
 }
 }
}

• vgru: I don't see any benefits in using Quartz.Net vs polling it like you did in the first question. This is a far more complicated way of polling in 1s intervals. I am not sure how your Locker code looks like, but making WaitForLock a member of that class (Locker) would allow it to signal any blocked threads whenever a lock is released. This is probably what Travis had in mind: the purpose of ManualResetEvent is to provide synchronization without polling. You should post your code for the Locker.Lock method, it would be easier.
• Bill Barry: What version of .NET are you working with? I am asking because 4 provides some new thread primitives which may be of use.

Answer 1

I read your original question and this one and don't see how using ManualResetEvent/Quartz adds anything valuable. As far as I understand the whole point of sleeping is just to avoid polling the DB too frequently.

Here some pseudocode...

create table lock (name varchar(50) primary key)
bool tryLockNonBlocking(name_to_lock) {
 try {
 insert into lock (name) values (:name_to_lock)
 commit
 return true
 } catch(UniqueConstraintViolation) {
 return false
 } 
}
void releaseLock(name_to_release) {
 delete from lock where name = :name_to_release
 commit
}
final POLL_PERIOD = 100 // msecs
bool tryLockWithTimeout(name_to_lock, timeoutSeconds) {
 waitTimeMS = timeoutSeconds * 1000
 while(true) {
 boolean gotLock = tryLockNonBlocking()
 if(gotLock)
 return true
 waitTime -= POLL_PERIOD
 if(waitTime > 0) 
 Sleep(POLL_PERIOD)
 else
 return false
 }
}

Is that what you are trying to achieve?

NB. This is just a very rough draft ... you might want to add some lock owner and check it in