Является ли этот класс потоком безопасным?

Question

Я реализую этот класс как одноэлементный. Я плохо разбираюсь в безопасности потоков. Хотелось убедиться, что класс GenerateOrderID является потокобезопасным. Более конкретно, переменная orderCount не может одновременно увеличиваться на разные объекты и вызывать подсчет.

public class OrderIDGenerator 

{ 

    private static readonly OrderIDGenerator instance = new OrderIDGenerator(); 
    private int orderCount; 


    private OrderIDGenerator() 
    { 
     orderCount = 1; 
    } 


    public static OrderIDGenerator Instance 
    { 
     get { return instance; } 
    } 

    public string GenerateOrderID() 
    { 
     return String.Format("{0:yyyyMMddHHmmss}{1}", DateTime.Now, orderCount++); 
    } 

} 

Answer 1

Это не так. Операция после инкремента не является атомарной. Вам нужно будет внести следующие изменения:

Заменить метод GenerateOrderID с этим:

public string GenerateOrderID() 
{ 
    return String.Format("{0:yyyyMMddHHmmss}{1}", 
         DateTime.Now, 
         Interlocked.Increment(ref orderCount)); 
} 

И инициализировать orderCount до 0 вместо 1, так как Interlocked.Increment возвращает измененное значение. (Другими словами, Interlocked.Increment(ref foo) идентичен во всех отношениях ++foo за исключением того, что атомная, и поэтому потокобезопасной.)

Обратите внимание, что Interlocked.Increment является гораздо более эффективным, чем использование lock для синхронизации потоков, хотя lock будет по-прежнему Работа. См. this question.

Кроме того, не используйте синглтоны.

Answer 2

Я бы пометил orderCount как volatile (чтобы предотвратить предположения оптимизации компилятора) и использовать блокировку вокруг приращения. Неустойчивый, вероятно, перебор, но это не повредит.

public class OrderIDGenerator 
{ 

    private static readonly OrderIDGenerator instance = new OrderIDGenerator(); 
    private volatile int orderCount; 
    private static object syncRoot = new object(); 


    private OrderIDGenerator() 
    { 
     orderCount = 1; 
    } 


    public static OrderIDGenerator Instance 
    { 
     get { return instance; } 
    } 

    public string GenerateOrderID() 
    { 
     lock (syncRoot) 
      return String.Format("{0:yyyyMMddHHmmss}{1}", DateTime.Now, orderCount++); 
    } 
} 

Answer 3

Вы просто можете иметь его быть поточно с небольшой модификацией

public class OrderIDGenerator { 
    private static readonly OrderIDGenerator instance = new OrderIDGenerator(); 
    private int orderCount; 
    private static readonly object _locker = new object(); 

    private OrderIDGenerator() { 
      orderCount = 1; 
    } 

    public static OrderIDGenerator Instance { 
      get { return instance; } 
    } 

    public string GenerateOrderID() { 
     string orderId = ""; 
     lock (_locker) { 
      orderID = String.Format("{0:yyyyMMddHHmmss}{1}", DateTime.Now, orderCount++); 
     } 
     return orderID; 
    } 
} 

Answer 4

Этого класс не THREADSAFE.

Приращение не является атомной операцией. Поэтому вполне возможно, что переключение контекста происходит как раз в то время:

Start with orderCount at 1. 
Two threads try to GenerateOrderID() at once: 

Thread 1    | Thread 2 
---------------------------------------------- 
read orderCount = 1 | 
        --> 
        | read orderCount = 1 
        | add: 1 + 1 
        | write orderCount = 2 
        <-- 
add: 1 + 1   | 
write orderCount = 2 | 
return timestamp1 | 
        --> 
        | return timestamp1 

Теперь у вас есть дубликат заказа ID с заказом COUNT скинули.

Чтобы это исправить, заблокировать orderCount в то время как доступ к нему:

public string GenerateOrderID() 
{ 
    lock(this){ 
     return String.Format("{0:yyyyMMddHHmmss}{1}", DateTime.Now, orderCount++); 
    } 
} 

«замок» оператор позволяет только один нить за один раз, для каждого объекта заперта на. Если один поток находится в GenerateOrderID(), то до его завершения ваш OrderIDGenerator будет заблокирован, а следующий поток, пытающийся потребовать блокировку, должен будет дождаться завершения первого потока.

Подробнее о заявлении lockhere.