0

Assuming there is the following C# POCO:

public class Gift
{
   public int Id { get; set; }
   public string Owner { get; set; }
}

And the following Service:

public class GiftClaimService
{
    private ISomeRepository _someRepository; // Going to be injected or whatever

    public bool ClaimGift(int giftId, string myName)
    {
        var gift = _someRepository.Get(giftId);
        if (gift != null && gift.Owner == null)
        {
            gift.Owner = myName;
            _someRepository.Save(gift);
            return true;
        }
        return false;
    }
}

I hope this is easy to unterstand. Any user calling "ClaimGift" with a correct ID is able to claim the gift, in this case, his name is put as the Owner of the Gift. The repository used is out of scope, it doesn't matter if a Database etc.

Now this scenario obviously suffers some concurrency issues. Two or more users, having own instances of the GiftClaimService and the ISomeRepository, might attempt to claim the gift at virtually the same time, causing some issues with the method (e.g. overriding the Owner multiple times, returing the wrong result or whatever you can imagine).

I'd like to lock that up, by creating a "global" lock object which will prevent others (other threads/instances etc.) from aquiring the same lock, limited to the type (Gift) and it's Id. Of couse this might run "simulataneously" (though not in the same instance) for different Ids.

The lock is released by it's owner once the changes are committed (by calling Save on the repository). For others it behaves like usual locks: The processing is on hold at this command. The new method would look like this:

public bool ClaimGift(int giftId, string myName)
{
    var result = false;

    // Lock it up (Pseudo-command)... I have no idea how this could work?
    lock(typeof(Gift), giftId)
    {
        var gift = _someRepository.Get(giftId);
        if (gift != null && gift.Owner == null)
        {
            gift.Owner = myName;
            _someRepository.Save(gift);
            result = true;
        }
    }

    return result;
}

This is how i wish it could look like, is there any common technique/pattern how to solve this?

5
  • simply add a timestamp column on the gift table, and then EF will check when updating if that columns value has been changed or not, and you will get a nice exception then. Nov 27, 2014 at 14:53
  • @hazimdikenli OK I already implemented this kind of optimistic concurrency in EF before, but It's just not the same, this one is about attempting to prevent this scenario at all, plus the Repository is neither EF nor SQL Server (Currently, its MongoDB but I don't want this solution to be Database-specific)
    – thmshd
    Nov 27, 2014 at 14:57
  • what about adding a global array/list of claims in progress with the gift Id, and accessing it with a locker object. Nov 27, 2014 at 15:02
  • Why not just use a normal lock, are you going to get a significant performance gain with this theoretical id specific lock? Nov 27, 2014 at 15:14
  • is the injected repository instance is going to be same you may think of alternative solutions.If not I think it is best if this is controlled at the database level. Nov 27, 2014 at 15:40

2 Answers 2

1
void Main()
{
    var s = new GiftClaimService ();
    s.ClaimGift(1,"Me");
}

public class Gift
{
   public int Id { get; set; }
   public string Owner { get; set; }
}
public interface ISomeRepository
{
    Gift Get(int giftId);
    void Save(Gift gift);
}
public class GiftClaimService
{
    private ISomeRepository _someRepository; // Going to be injected or whatever
   public readonly static Dictionary<int, string> _claimsInProgress = new Dictionary<int, string>();
   private static readonly object _locker = new object();

    public bool ClaimGift(int giftId, string myName)
    {
        lock (_locker)
        {
        if ( _claimsInProgress.ContainsKey(giftId))
            return false;
        }

        var gift = _someRepository.Get(giftId);
        if (gift == null)
            return false; // no such gift

        lock (_locker)
        {
            if ( _claimsInProgress.ContainsKey(giftId))
                return false;// someone claimed it just now

            _claimsInProgress.Add(giftId, myName);
        }
        bool retValue;
        if (gift.Owner == null)
        {
            gift.Owner = myName;
            _someRepository.Save(gift);
            retValue = true;
        }
        else
            retValue = false;

        lock (_locker)
        {
            _claimsInProgress.Remove(giftId);
        }           

        return retValue;
    }
}
1
  • Thank you, while this is a first approach, it's not exactly what I want, because it's actually not locking. Here, the method assumes that when the lock is set for the given giftId, it should return because "someone claimed it just now". This assumption is not inevitably right. (in this simplified scenario maybe, but imagine this is maybe not claimed for free but sold, and the process would also check the user's balance etc.)
    – thmshd
    Dec 2, 2014 at 14:35
0

If you want to handle a web-farm scenario, this would have to be handled at the database level. Otherwise you could try the code below:

public class GiftClaimService
{
    private static readonly object Lock = new object();
    private ISomeRepository _someRepository; // Going to be injected or whatever

    public bool ClaimGift(int giftId, string myName)
    {
        lock (Lock)
        {
            var gift = _someRepository.Get(giftId);
            if (gift != null && gift.Owner == null)
            {
                gift.Owner = myName;
                _someRepository.Save(gift);
                return true;
            }
            return false;
        }
    }
}
1
  • 1
    web-farm is a good point. But maybe it still has not to be at database level. It just has to be a level "global enough" across the instances... just some thought.
    – thmshd
    Nov 27, 2014 at 15:34

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

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