Domanda multi-threading - aggiunta di un elemento a un elenco statico

Question

OK, domanda multi-threading newbie:

Ho una classe Singleton. La classe ha un elenco statico ed essenzialmente funziona così:

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"); 
    } 
} 

Fondamentalmente un utente può chiamare StartRecording() per inizializzare un elenco e quindi tutte le chiamate a un metodo grado possono aggiungere cose alla lista. Tuttavia, più thread possono contenere un'istanza su MyClass, quindi più thread possono aggiungere voci all'elenco.

Tuttavia, sia la creazione di liste che la lettura sono operazioni singole, quindi il solito problema di Reader-Writer nelle situazioni multi-threading non si applica. L'unico problema che ho potuto vedere è che l'ordine di inserimento è strano, ma non è un problema.

Posso lasciare il codice così com'è o devo prendere delle precauzioni per il multithreading? Dovrei aggiungere che nella vera applicazione questo non è un elenco di stringhe ma un elenco di oggetti personalizzati (quindi il codice è _list.Add (new Object (somedata))), ma questi oggetti contengono solo dati, nessun codice oltre a una chiamata a DateTime.Now.

Edit: Chiarimenti seguenti alcune risposte: DoSomething non può essere statico (la classe qui è abbreviato, c'è un sacco di roba in corso che utilizza istanze-variabili, ma queste creati dal costruttore e poi solo in lettura) . E 'abbastanza buono per fare

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

and 

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

o ho bisogno un po' di magia più profondo?

Answer 1

È certamente necessario bloccare la lista. E dal momento che si sta creando più istanze per _list Non è possibile bloccare il _list se stessa, ma si dovrebbe usare qualcosa come:

private static object _listLock = new object(); 

Per inciso, a seguire alcune best practice:

• DoSomething(), come mostrato, può essere statico e quindi dovrebbe essere.

• per le classi di libreria il modello consigliato è di rendere membri statici thread-safe, che si applica a StartRecording(), StopRecording() e DoSomething().

Vorrei anche fare StopRecording() set _list = null e controllare per nulla in DoSomething().

E prima che tu lo chieda, tutto questo richiede così poco tempo che non ci sono davvero motivi di prestazioni non per farlo.

Answer 2

È possibile, anche se difficile, scrivere un elenco collegato che consente l'inserimento simultaneo da più thread senza un blocco, ma non è questo. Non è sicuro chiamare _list.Add in parallelo e sperare per il meglio. A seconda di come è stato scritto, potresti perdere uno o entrambi i valori o corrompere l'intera struttura. Basta bloccarlo.

Answer 3

Le raccolte .NET non sono completamente thread-safe. Da MSDN: "Più lettori possono leggere la raccolta con sicurezza, tuttavia, qualsiasi modifica alla raccolta produce risultati non definiti per tutti i thread che accedono alla raccolta, inclusi i thread del lettore." Puoi seguire i suggerimenti su quella pagina MSDN per rendere i tuoi accessi thread-safe.

Un problema che probabilmente dovresti eseguire con il tuo codice corrente è se StopRecording viene chiamato mentre qualche altro thread si trova in DoSomething. Poiché la creazione di un nuovo elenco da uno esistente richiede l'enumerazione su di esso, è probabile che si esegua il vecchio problema "La raccolta è stata modificata, operazione di enumerazione non eseguita".

La linea di fondo: pratica la filettatura sicura!

Answer 4

È necessario bloccare l'elenco se più thread si aggiungono ad esso.

Alcune osservazioni ...

Forse c'è un motivo per non farlo, ma vorrei suggerire rendendo la classe statica e quindi di tutti i suoi membri statici. Non c'è un vero motivo, almeno da quello che hai mostrato, per richiedere ai client di MyClass di chiamare il metodo GetInstance() solo così possono chiamare un metodo di istanza, DoSomething() in questo caso.

Non vedo cosa impedisca a qualcuno di chiamare il metodo StartRecording() più volte. Potresti prendere in considerazione l'idea di inserire un assegno in modo tale che se sta già registrando non crei una nuova lista, estraendo il tappeto dai piedi di tutti.

Infine, quando si blocca la lista, non farlo in questo modo:

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

ridurre al minimo la quantità di tempo trascorso all'interno della serratura spostando la nuova creazione di oggetti al di fuori della serratura.

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

EDIT

Lei ha detto che DoSomething() non può essere statico, ma scommetto che può. Puoi comunque utilizzare un oggetto di MyClass all'interno di DoSomething() per qualsiasi cosa relativa all'istanza che devi fare. Ma dal punto di vista dell'usabilità della programmazione, non richiedere agli utenti di MyClass di chiamare prima GetInstance(). Considerate questo:

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(); 
    } 
} 

In questo modo, gli utenti di MyClass può andare da

MyClass.GetInstance().DoSomething(); 

a

MyClass.DoSomething();