Pytanie wielowątkowe - dodawanie elementu do listy statycznej

Question

OK, pytanie początkujących wielowątkowych:

Mam lekcję Singleton. Klasa ma statyczną listę i zasadniczo działa tak:

class MyClass { 
    private static MyClass _instance; 
    private static List<string> _list; 
    private static bool IsRecording; 

    public static void StartRecording() { 
     _list = new List<string>(); 
     IsRecording = true; 
    } 

    public static IEnumerable<string> StopRecording() { 
     IsRecording = false; 
     return new List<string>(_list).AsReadOnly(); 
    } 

    public MyClass GetInstance(){ 

    } 

    public void DoSomething(){ 
     if(IsRecording) _list.Add("Something"); 
    } 
} 

Zasadniczo użytkownik może zadzwonić StartRecording(), aby zainicjować listy, a następnie wszystkie połączenia do metody instancji może dodać rzeczy do listy. Jednak wiele wątków może przechowywać instancję w MyClass, więc wiele wątków może dodawać wpisy do listy.

Jednak tworzenie i odczyt list są pojedynczymi operacjami, więc zwykły problem z pisarzem i pisarzem w sytuacjach wielowątkowych nie ma zastosowania. Jedyny problem, jaki mogłem zauważyć, to kolejność wstawiania, która jest dziwna, ale to nie jest problem.

Czy mogę pozostawić kod bez zmian, czy też muszę przedsięwziąć środki ostrożności w przypadku wielowątkowości? Powinienem dodać, że w prawdziwej aplikacji nie jest to lista łańcuchów, ale lista obiektów niestandardowych (tak więc kod jest _list.Add (nowy obiekt (somedata))), ale te obiekty przechowują tylko dane, bez kodu poza wywołaniem do DateTime.Now.

Edit: Wyjaśnienia po kilka odpowiedzi: DoSomething nie może być statyczny (klasa jest tu w skrócie, jest wiele rzeczy dzieje się, że jest za pomocą instancji zmiennych, ale te stworzone przez konstruktora, a potem tylko do odczytu) . Czy jest wystarczająco dobry, aby zrobić

lock(_list){ 
    _list.Add(something); 
} 

and 

lock(_list){ 
    return new List<string>(_list).AsReadOnly(); 
} 

czy muszę jakąś głębszą magię?

Answer 1

Z pewnością musisz zablokować _list. A ponieważ tworzysz wiele wystąpień na _list nie można zablokować na _list sobie, lecz należy użyć coś takiego:

private static object _listLock = new object(); 

Jak na bok, aby wykonać kilka najlepszych praktyk:

• DoSomething(), jak pokazano, może być statyczne i tak powinno być.

• dla klas bibliotecznych zalecany wzorzec jest, aby członkowie statyczne thread-safe, które miałyby zastosowanie do StartRecording(), StopRecording() i DoSomething().

Chciałbym również StopRecording() ustawić _list = null i sprawdzić dla wartości null w DoSomething().

I zanim zapytasz, wszystko to zajmuje tak mało czasu, że naprawdę nie ma powodów, aby wykonać tę czynność. Nie.

Answer 2

Możliwe, choć trudne, napisanie połączonej listy, która umożliwia jednoczesne wstawianie z wielu wątków bez blokady, ale to nie jest to. Po prostu nie jest bezpiecznie wywoływać _list.Add równolegle i mieć nadzieję na najlepsze. W zależności od tego, jak jest napisane, możesz stracić jedną lub obie wartości lub uszkodzić całą strukturę. Po prostu zablokuj.

Answer 3

Kolekcje .NET nie są w pełni wątkowe. Od MSDN: "Wielu czytelników może odczytać kolekcję z pewnością, jednak każda modyfikacja kolekcji daje niezdefiniowane wyniki dla wszystkich wątków, które uzyskują dostęp do kolekcji, w tym wątków czytnika." Możesz postępować zgodnie z sugestiami na tej stronie MSDN, aby zapewnić dostęp do wątku bezpieczny.

Jednym z problemów, które prawdopodobnie napotkasz przy obecnym kodzie, jest wywołanie StopRecording, podczas gdy inny wątek znajduje się w środku DoSomething. Ponieważ tworzenie nowej listy z istniejącej wymaga jej wyliczenia, najprawdopodobniej pojawi się stara "Zmodyfikowano kolekcję, operacja wyliczania nie może wykonać".

Najważniejsze: przećwicz bezpieczne nawlekanie!

Answer 4

Musisz zablokować listę, jeśli dodaje ją wiele wątków.

Kilka uwag ...

Może jest to powód, by nie, ale chciałbym zaproponować co klasa statyczna i dlatego wszyscy jej członkowie statycznych. Nie ma żadnego prawdziwego powodu, przynajmniej z tego, co pokazałeś, aby wymagać od klientów MyClass wywoływania metody GetInstance() tylko po to, aby mogli wywołać metodę instancji, DoSomething() w tym przypadku.

Nie widzę problemu, który uniemożliwia wielokrotne wywoływanie metody StartRecording(). Możesz rozważyć umieszczenie tam zameldowania, aby w razie nagrania nie tworzyć nowej listy, wyciągając dywan z nóg wszystkich.

Wreszcie, kiedy zamknąć listę, nie rób to tak:

static object _sync = new object(); 
lock(_sync){ 
    _list.Add(new object(somedata)); 
} 

zminimalizować czas spędzony wewnątrz zamka przesuwając nowe stworzenie obiektu poza zamkiem.

static object _sync = new object(); 
object data = new object(somedata); 
lock(_sync){ 
    _list.Add(data); 
} 

EDIT

Mówiłeś, że DoSomething() nie może być statyczna, ale założę się, że można. Nadal możesz używać obiektu MyClass wewnątrz funkcji DoSomething() w celu wykonania jakichkolwiek czynności związanych z instancją. Ale z punktu widzenia użyteczności programowania nie wymagaj od użytkowników, aby MyClass wywoływał najpierw GetInstance(). Rozważ to:

class MyClass { 
    private static MyClass _instance; 
    private static List<string> _list; 
    private static bool IsRecording; 
    public static void StartRecording() 
    { 
     _list = new List<string>(); 
     IsRecording = true; 
    } 
    public static IEnumerable<string> StopRecording() 
    { 
     IsRecording = false; 
     return new List<string>(_list).AsReadOnly(); 
    } 
    private static MyClass GetInstance() // make this private, not public 
    { return _instance; } 
    public static void DoSomething() 
    { 
     // use inst internally to the function to get access to instance variables 
     MyClass inst = GetInstance(); 
    } 
} 

Spowoduje to, użytkownicy MojaKlasa może iść z

MyClass.GetInstance().DoSomething(); 

do

MyClass.DoSomething();