LINQ to SQL DAL, is this thread safe?

Question

My code is written with C# and the data layer is using LINQ to SQL that fill/load detached object classes.

I have recently changed the code to work with multi threads and i'm pretty sure my DAL isn't thread safe.

Can you tell me if PopCall() and Count() are thread safe and if not how do i fix them?

public class DAL
{
   //read one Call item from database and delete same item from database. 
    public static OCall PopCall()
    {
        using (var db = new MyDataContext())
        {
            var fc = (from c in db.Calls where c.Called == false select c).FirstOrDefault();
            OCall call = FillOCall(fc);
            if (fc != null)
            {
                db.Calls.DeleteOnSubmit(fc);
                db.SubmitChanges();
            }
            return call;
        }
    }

    public static int Count()
    {
        using (var db = new MyDataContext())
        {
            return (from c in db.Calls select c.ID).Count();
        }
    }

    private static OCall FillOCall(Model.Call c)
    {
        if (c != null)
            return new OCall { ID = c.ID, Caller = c.Caller, Called = c.Called };
        else return null;
    }
}

Detached OCall class:

public class OCall
{
    public int ID { get; set; }
    public string Caller { get; set; }
    public bool Called { get; set; }
}

Answer 1

Count() is thread-safe. Calling it twice at the same time, from two different threads will not harm anything. Now, another thread might change the number of items during the call, but so what? Another thread might change the number of item a microsecond after it returns, and there's nothing you can do about it.

PopCall on the other hand, does has a possibility of threading problems. One thread could read fc, then before it reaches the SubmitChanges(), another thread may interceded and do the read & delete, before returning to the first thread, which will attempt to delete the already deleted record. Then both calls will return the same object, even though it was your intension that a row only be returned once.

Answer 2

Individually they are thread-safe, as they use isolated data-contexts etc. However, they are not an atomic unit. So it is not safe to check the count is > 0 and then assume that there is still something there to pop. Any other thread could be mutating the database.

If you need something like this, you can wrap in a TransactionScope which will give you (by default) the serializable isolation level:

using(var tran = new TransactionScope()) {
    int count = OCall.Count();
    if(count > 0) {
        var call = Count.PopCall();
        // TODO: something will call, assuming it is non-null
    }
}

Of course, this introduces blocking. It is better to simply check the FirstOrDefault().

Note that PopCall could still throw exceptions - if another thread/process deletes the data between you obtaining it and calling SubmitChanges. The good thing about it throwing here is that you shouldn't find that you return the same record twice.

The SubmitChanges is transactional, but the reads aren't, unless spanned by a transaction-scope or similar. To make PopCall atomic without throwing:

public static OCall PopCall()
{
    using(var tran = new TrasactionScope())
        using (var db = new MyDataContext())
        {
            var fc = (from c in db.Calls where c.Called == false select c).FirstOrDefault();

            OCall call = FillOCall(fc);

            if (fc != null)
            {
                db.Calls.DeleteOnSubmit(fc);
                db.SubmitChanges();
            }

            return call;
        }
        tran.Complete();
    }
}

Now the FirstOrDefault is covered by the serializable isolation-level, so doing the read will take a lock on the data. It would be even better if we could explicitly issue an UPDLOCK here, but LINQ-to-SQL doesn't offer this.

Answer 3

Unfortunately no amount of Linq-To-Sql trickery, nor SqlClient isolation levels, nor System.Transactions can make the PopCall() thread safe, where 'thread safe' really means 'concurrent safe' (ie. when concurrency occurs on the database server, outside the controll and scope of the client code/process). And neither is any sort of C# locking and synchronization going to help you. You just need to deeply internalize how a relational storage engine works in order to get this doen correctly. Using tables as queues (as you do it here) is notoriously tricky , deadlock prone, and really hard to get it correct.

Even less fortunate, your solution is going to have to be platform specific. I'm only going to explain the right way to do it with SQL Server, and that is to leverage the OUTPUT clause. If you want to get a bit more details why this is the case, read this article Using tables as Queues. Your Pop operation must occur atomically in the database with a calls like this:

WITH cte AS (
 SELECT TOP(1) ... 
 FROM Calls WITH (READPAST)
 WHERE Called = 0)
DELETE
FROM cte
OUTPUT DELETED.*;

Not only this, but the Calls table has to be organized with a leftmost clustered key on the Called column. Why this is the case, is again explained in the article I referenced before.

In this context the Count call is basically useless. Your only way to check correctly for an item available is to Pop, asking for Count is just going to put useless stress on the database to return a COUNT() value which means nothing under a concurrent environment.