Ensuring Thread-Safety On Static Methods In C#

Question

I've got some code I currently have in a static class/method, but I wanted to check that it would be thread-safe. From what I've read I think this should be ok, but something in the back of my mind is saying it might not be. My web page's data processing stage uses an external web service to create order records, and this could be quite slow: possibly 30-40 seconds to, possibly, 5 or 10 minutes (this is out of my hands) so I was going to fire a return page back to the user, then start a new thread and then e-mail the user once the processing was complete. This is currently in a static class/method. Provided all my objects are created within the particular method (aside from system default values, which would be common) that method should be thread-safe, shouldn't it. So, for example, if I had

public static class ProcessOrder()
{
    public static int GetOrderMaxSize()
    {
        return (....gets and parses ConfigurationManager.AppSettings["MaxOrderSize"]...);
    }

    public static bool CreateOrder(Order order)
    {
        XmlDocument xmlDoc = GetOrderXML(order);
        bool check = false;
        using (CreateOrderXML.Create xmlCo = new CreateOrderXML.Create())
        {
            xmlCo.Timeout = 60000;
            System.Text.UTF8Encoding encoding = new System.Text.UTF8Encoding();

            string xmlString = "";
            using (StringWriter stringWriter = new StringWriter())
            {
                using (XmlWriter xmlWriter = XmlWriter.Create(stringWriter))
                {
                    xmlDoc.WriteTo(xmlWriter);
                    xmlWriter.Flush();
                    xmlString = stringWriter.GetStringBuilder().ToString();
                }
            }

            byte[] bXMLOrder = encoding.GetBytes(xmlString);
            byte[] breturnMessage;

            check = xmlCo.Create(bXMLOrder, out breturnMessage);
            .... do something with return message
        }
        return check;
    }

    private static XmlDocument GetOrderXML(Order order)
    {
        ... creates an XML object for the order
    }
}

(the CreateOrderXML is a service reference to the Web Service URL/method) would that be thread-safe, especially for long-running (primarily at the xmlCo.Create(....) stage) concurrent threads? I understand that if I started putting in class members and then used them in the method, this would definitely introduce a problem with different threads overwriting the values, but so long as the objects are created within the methods, they should be ok, shouldn't they?

Solution 1

It doesn't look like you are accessing any shared data there; you are requesting remote resources, and building a unique set of data with each execution of this method. There's no need for synchronization there.

Each execution of the method here is creating local variables - it's own copies. So nothing is ever being shared.

Solution 2

If your static method doesn't access any static (class) data, it should be thread-safe. The only possible points of contention would be external resources that it might use (files, for instance, or other system resources), and data that is passed in. And only you know the context of that sort of usage.

Usage of things like that that might be in contention can be serialized withe lock or other primatives. Don't forget to serialize resources in the same order lest you deadlock. If you have one method that uses resource A and B:

lock( latch_a )
{
   process(object_a) ;
   lock ( latch_b )
   {
     process(object_a,object_b) ;
   }
}

and another method that does the inverse:

lock( latch_b )
{
   process(object_b) ;
   lock ( latch_a )
   {
     process(object_a,object_b) ;
   }
}

At some point your two threads will deadlock, when each of them requires a resource the other needs bofore it can relinquish access to said resource.

Edited to note: See the C# documentation for the lock statement for details. In general, the locked object represents (and might be) the shared resource access to which is being serialized. A common pattern is to do something like:

class Widget
{
   private static readonly object X = new object() ;

   public void Foo()
   {
     lock(X)
     {
       // Do work using shared resource
     }
     return ;
   }

}