WCF service with XML based storage. Concurrency issues?

Question

I programmed a simple WCF service that stores messages sent by users and sends these messages to the intended user when asked for. For now, the persistence is implemented by creating username.xml files with the following structure:

<messages recipient="username">
  <message sender="otheruser">
     ...
  </message
</messages>

It is possible for more than one user to send a message to the same recipient at the same time, possibly causing the xml file to be updated concurrently. The WCF service is currently implemented with basicHttp binding, without any provisions for concurrent access.

What concurrency risks are there? How should I deal with them? A ReadWrite lock on the xml file being accessed? Currently the service runs with 5 users at the most, this may grow up to 50, but no more.

EDIT: As stated above the client will instantiate a new service class with every call it makes. (InstanceContext is PerCall, ConcurrencyMode irrelevant) This is inherent to the use of basicHttpBinding with default settings on the service.

The code below:

   public class SomeWCFService:ISomeServiceContract
        {
          ClassThatTriesToHoldSomeInfo useless;

          public SomeWCFService() 
          {
             useless=new ClassThatTriesToHoldSomeInfo();
          } 

          #region Implementation of ISomeServiceContract
          public void IncrementUseless()
          {
            useless.Counter++;
          }
          #endregion
        }

behaves is if it were written:

  public class SomeWCFService:ISomeServiceContract
        {
          ClassThatTriesToHoldSomeInfo useless;

          public SomeWCFService() 
          {} 

          #region Implementation of ISomeServiceContract
          public void IncrementUseless()
          {
            useless=new ClassThatTriesToHoldSomeInfo();
            useless.Counter++;
          }
          #endregion
        }

So concurrency is never an issue until you try to access some externally stored data as in a database or in a file. The downside is that you cannot store any data between method calls of the service unless you store it externally.

Answer 1

You need to read the file before adding to it and writing to disk, so you do have a (fairly small) risk of attempting two overlapping operations - the second operation reads from disk before the first operation has written to disk, and the first message will be overwritten when the second message is committed.

A simple answer might be to queue your messages to ensure that they are processed serially. When the messages are received by your service, just dump the contents into an MSMQ queue. Have another single-threaded process which reads from the queue and writes the appropriate changes to the xml file. That way you can ensure you only write one file at a time and resolve any concurrency issues.

Answer 2

If your WCF service is a singleton service and guaranteed to be that way, then you don't need to do anything. Since WCF will allow only one request at a time to be processed, concurrent access to the username files is not an issue unless the operation that serves that request spawns multiple threads that access the same file. However, as you can imagine, a singleton service is not very scalable and not something you want in your case I assume.

If your WCF service is not a singleton, then concurrent access to the same user file is a very realistic scenario and you must definitely address it. Multiple instances of your service may concurrently attempt to access the same file to update it and you will get a 'can not access file because it is being used by another process' exception or something like that. So this means that you need to synchronize access to user files. You can use a monitor (lock), ReaderWriterLockSlim, etc. However, you want this lock to operate on per file basis. You don't want to lock the updates on other files out when an update on a different file is going on. So you will need to maintain a lock object per file and lock on that object e.g.

//when a new userfile is added, create a new sync object
fileLockDictionary.Add("user1file.xml",new object());

//when updating a file
lock(fileLockDictionary["user1file.xml"])
{
   //update file.
}

Note that that dictionary is also a shared resource that will require synchronized access.

Now, dealing with concurrency and ensuring synchronized access to shared resources at the appropriate granularity is very hard not only in terms of coming up with the right solution but also in terms of debugging and maintaining that solution. Debugging a multi-threaded application is not fun and hard to reproduce problems. Sometimes you don't have an option but sometimes you do. So, Is there any particular reason why you're not using or considering a database based solution? Database will handle concurrency for you. You don't need to do anything. If you are worried about the cost of purchasing a database, there are very good proven open source databases out there such as MySQL and PostgreSQL that won't cost you anything.

Another problem with the xml file based approach is that updating them will be costly. You will be loading the xml from a user file in memory, create a message element, and save it back to file. As that xml grows, that process will take longer, require more memory, etc. It will also hurt your scalibility because the update process will hold onto that lock longer. Plus, I/O is expensive. There are also benefits that come with a database based solution: transactions, backups, being able to easily query your data, replication, mirroring, etc.

I don't know your requirements and constraints but I do think that file-based solution will be problematic going forward.

Answer 3

Well, it just so happens that I've done something almost exactly the same, except that it wasn't actually messages...

Here's how I'd handle it.

Your service itself talks to a central object (or objects), which can dispatch message requests based on the sender.

The object relating to each sender maintains an internal lock while updating anything. When it gets a new request for a modification, it then can read from disk (if necessary), update the data, and write to disk (if necessary).

Because different updates will be happening on different threads, the internal lock will be serialized. Just be sure to release the lock if you call any 'external' objects to avoid deadlock scenarios.

If I/O becomes a bottleneck, you can look at different strategies involving putting messages in one file, separate files, not immediately writing them to disk, etc. In fact, I'd think about storing the messages for each user in a separate folder for exactly that reason.

The biggest point is, that each service instance acts as, essentially, an adapter to the central class, and that only one instance of one class will ever be responsible for reading/writing messages for a given recipient. Other classes may request a read/write, but they do not actually perform it (or even know how it's performed). This also means that their code is going to look like 'AddMessage(message)', not 'SaveMessages(GetMessages.Add(message))'.

That said, using a database is a very good suggestion, and will likely save you a lot of headaches.

Answer 4

The basic problem is when you access a global resource (like a static variable, or a file on the filesystem) you need to make sure you lock that resource or serialize access to it somehow.

My suggestion here (if you want to just get it done quick without using a database or anything, which would be better) would be to insert your messages into a Queue structure in memory from your service code.

public MyService : IMyService
{
     public static Queue queue = new Queue();
     public void SendMessage(string from, string to, string message)
     {
          Queue syncQueue = Queue.Synchronized(queue);
          syncQueue.Enqueue(new Message(from, to, message));
     }
}

Then somewhere else in your app you can create a background thread that reads from that queue and writes to the filesystem one update at a time.

void Main()
{


    Timer timer = new Timer();
    timer.Tick += (o, e)
       {
           Queue syncQueue = Queue.Synchronized(MyService.queue);
           while(syncQueue.Count > 0)
           {
                Message message = syncQueue.Dequeue() as Message;
                WriteMessageToXMLFile(message);
           }
           timer.Start();
       };
    timer.Start();

    //Or whatever you do here
    StartupService();
}

It's not pretty (and I'm not 100% sure it compiles) but it should work. It sort of follows the "get it done with the tools I have, not the tools I want" kind of approach I think you are looking for.

The clients are also off the line as soon as possible, rather than waiting for the file to be written to the filesystem before they disconnect. This can also be bad... clients might not know their message didn't get delivered should your app go down after they disconnect and the background thread hasn't written their message yet.

Other approaches on here are just as valid... I wanted to post the serialization approach, rather than the locking approach others have suggested.

HTH, Anderson