1
\$\begingroup\$

I'm trying to release a lock after some timeout. The lock is acquired to do any database changes so that multiple requests will not update the database simultaneously.

Below is the code

public static readonly _lockObject = new Object();

public bool UpdateData(ModifiedData modifiedData)
{
    var success = false;
    lock(_lockObject)
    {
        var thread = new Thread(()=>
        {
            try
            {
                SessionManager.Instance.OpenConversation(true); // Starts a session using nHibernate
                //Modifications

                success = true;
                SessionManager.Instance.EndConversation(true);

            }
            catch(ThreadAbortException ex)
            {
                //Do I need to handle any condition here
            }
        });
        thread.Start();
        if (!thread.Join(TimeSpan.FromMilliseconds(1000)))  // Timeout
        {
           if (!success)
              thread.Abort();
           else
           {
             thread.Join();
           }
        }
        else
        {
            if (!success)
             {
                    throw new Exception("Should Not occur");
             }
        }
    }
}

are there any better approaches available?? I tried with Action.BeginInvoke but in that not able to kill or stop action execution after some timeout.

\$\endgroup\$
  • \$\begingroup\$ This is not even a static method. It would do nothing to stop two or more users from making simultaneous requests. Not sure about your modifications but Command has a timeout option. \$\endgroup\$ – paparazzo May 18 '18 at 14:24
  • \$\begingroup\$ Should "The lock is acquired" actually read "The lock is required"? \$\endgroup\$ – Sᴀᴍ Onᴇᴌᴀ May 18 '18 at 15:32
  • \$\begingroup\$ indeed it's a static method I forgot to add. \$\endgroup\$ – Rudresha Parameshappa May 18 '18 at 18:02
  • 3
    \$\begingroup\$ I have rolled back the last edit. Please see what you may and may not do after receiving answers. \$\endgroup\$ – Vogel612 May 18 '18 at 18:05
3
\$\begingroup\$

First you shouldn't make your lock object public if it's for internal working of the class. Typically they are marked as private.

The lock statement is just wrapper around the Monitor class. The Monitor class has a built in timeout. To use it we need to write a bit more code but less than what you currently have

private static readonly object _lockObject = new object();

public bool UpdateData()
{
    bool lockWasTaken = false;
    try
    {
        Monitor.TryEnter(_lockObject, TimeSpan.FromSeconds(1), ref lockWasTaken);
        if (lockWasTaken)
        {
            // do work here
        }

    }
    finally
    {
        if (lockWasTaken)
        {
            Monitor.Exit(_lockObject);
        }
    }

    return lockWasTaken;
}

Also if you are going to throw an exception you should make it useful and shouldn't normally use the standard Exception class. If it's timing out then throw the TimeoutException. This make it easier for callers to know it timed out and they can decide if they can capture it and retry.

\$\endgroup\$
  • \$\begingroup\$ Actually, other threads need to wait until the thread which has taken the lock either completes or we need to explicitly stop the thread from further execution. It's not new threads should be sent false. \$\endgroup\$ – Rudresha Parameshappa May 18 '18 at 18:04
  • \$\begingroup\$ @RudreshaParameshappa Then using Monitor but with Wait & Pulse methods plus setting timeout in nHibernate should achieve what you want. \$\endgroup\$ – Łukasz Ciosek May 27 '18 at 12:13

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.