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();
        }
    }
}

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