CODEWITHSUNDEEP
c#synchronizationthread-safety
Federico González
Federico González

Reputation: 450

Modify locked object within lock block

I'm having thread contention in an OLTP app. While reviewing the code involved, I found the following:

        lock (_pendingTransactions)
        {
            transaction.EndPointRequest.Request.Key = (string)MessageComparer.GenerateKey(transaction.EndPointRequest.Request);

            if (!_pendingTransactions.ContainsKey(transaction.EndPointRequest.Request.Key))
            {
                _pendingTransactions.Add(transaction.EndPointRequest.Request.Key, transaction);

                return true;
            }
            else
            {
                return false;
            }
        }

As you can see in the snippet, there is a lock on an object that is modified within the 'lock' block. Is there anything bad with that? Anyone had problems doing something like this?

Upvotes: 2

Views: 2714

Answers (3)

Remus Rusanu
Remus Rusanu

Reputation: 294307

Probably the key generation can be taken outside the lock block, to reduce the duration of the lock. Other than that, this is an almost canonical example of lock that protects a list/collection/array: acquire lock, check if the key exists, add key if not already present, release the lock.

Upvotes: 0

Jason Kresowaty
Jason Kresowaty

Reputation: 16500

Using locking in this way is often discouraged, with the recommendation being to use a dedicated lock field (class member variable). A dedicated lock field is of type Object and usually looks like this:

private object _pendingTransactionLock = new object();

If the object itself has some threading awareness, this lock variable might belong in _pendingTransaction's implementation class. Otherwise, it might belong alongside _pendingTransaction in the field's declaring class.

You don't say what type _pendingTransaction is. If this is a built-in collection class that provides a SyncRoot property, that might be a good choice to lock on.

See Jon Skeet's Choosing What to Lock On.

Upvotes: 3

bdonlan
bdonlan

Reputation: 231203

Generally speaking, one will take a lock on an object specifically because one is going to modify (or read) it, so there's nothing inherently wrong with this.

Upvotes: 0

Related Questions