¿Está bien usar una cadena como objeto de locking?

Necesito hacer una sección crítica en un área sobre la base de un conjunto finito de cadenas. Quiero que el locking se comparta para la misma instancia de cadena (algo similar al enfoque String.Intern ).

Estoy considerando la siguiente implementación:

public class Foo { private readonly string _s; private static readonly HashSet _locks = new HashSet(); public Foo(string s) { _s = s; _locks.Add(s); } public void LockMethod() { lock(_locks.Single(l => l == _s)) { ... } } } 

¿Hay algún problema con este enfoque? ¿Está bien bloquear un objeto de cadena de esta manera, y hay algún problema de seguridad de subprocesos al usar HashSet ?

¿Es mejor, por ejemplo, crear un Dictionary que cree un nuevo objeto de locking para cada instancia de cadena?


Implementación final

En base a las sugerencias, fui con la siguiente implementación:

 public class Foo { private readonly string _s; private static readonly ConcurrentDictionary _locks = new ConcurrentDictionary(); public Foo(string s) { _s = s; } public void LockMethod() { lock(_locks.GetOrAdd(_s, s => new object())) { ... } } } 

No se recomienda bloquear cadenas, la razón principal es que (debido a la interferencia de cadenas) algún otro código podría bloquearse en la misma instancia de cadena sin que usted lo sepa. Creando un potencial para situaciones de estancamiento.

Ahora bien, este es probablemente un escenario descabellado en la mayoría de las situaciones concretas. Es más una regla general para las bibliotecas.

Pero, por otro lado, ¿cuál es el beneficio percibido de las cadenas?

Entonces, punto por punto:

¿Hay algún problema con este enfoque?

Sí, pero principalmente teórico.

¿Está bien bloquear un objeto de cadena de esta manera, y hay algún problema de seguridad de subprocesos al usar el HashSet?

HashSet<> no está involucrado en la seguridad de subprocesos siempre que los subprocesos solo se lean simultáneamente.

¿Es mejor, por ejemplo, crear un diccionario que cree un nuevo objeto de locking para cada instancia de cadena?

Sí. Sólo para estar en el lado seguro. En un sistema grande, el objective principal para evitar el punto muerto es mantener los objetos de locking lo más local y privado posible. Solo una cantidad limitada de código debería poder acceder a ellos.

Yo diría que es una muy mala idea, personalmente. Eso no es para lo que son las cadenas.

(Personalmente, me desagrada el hecho de que cada objeto tenga un monitor en primer lugar, pero esa es una preocupación ligeramente diferente).

Si desea un objeto que represente un locking que pueda compartirse entre diferentes instancias, ¿por qué no crear un tipo específico para eso? Puede darle al candado un nombre con suficiente facilidad para fines de diagnóstico, pero el locking no es realmente el propósito de una cuerda. Algo como esto:

 public sealed class Lock { private readonly string name; public string Name { get { return name; } } public Lock(string name) { if (name == null) { throw new ArgumentNullException("name"); } this.name = name; } } 

Dada la forma en que las cadenas a veces se internan y otras veces no (de una manera que a veces puede ser difícil de discernir mediante una simple inspección), podría terminar fácilmente con lockings accidentalmente compartidos en los que no tenía la intención de hacerlo.

El locking de cadenas puede ser problemático, porque las cadenas internas son esencialmente globales.

Las cadenas internas son por proceso, por lo que incluso se comparten entre diferentes AppDomains. Lo mismo ocurre con los objetos de tipo (por lo que no se bloquea en typeof (x)) tampoco.

Tuve un problema similar no hace mucho cuando buscaba una buena manera de bloquear una sección de código basada en un valor de cadena. Esto es lo que tenemos implementado en este momento, que resuelve el problema de las cadenas internas y tiene la granularidad que queremos.

La idea principal es mantener un ConcurrentDictionary estático de objetos de sincronización con una clave de cadena. Cuando un hilo ingresa al método, inmediatamente establece un locking e intenta agregar el objeto de sincronización al diccionario concurrente. Si podemos agregar al diccionario concurrente, significa que ningún otro subproceso tiene un locking basado en nuestra clave de cadena y podemos continuar nuestro trabajo. De lo contrario, usaremos el objeto de sincronización del diccionario simultáneo para establecer un segundo locking, que esperará a que el subproceso en ejecución termine de procesarse. Cuando se libera el segundo locking, podemos intentar agregar el objeto de sincronización del hilo actual al diccionario nuevamente.

Una palabra de advertencia: los hilos no están en cola, por lo que si hay varios hilos con la misma clave de cadena compitiendo simultáneamente por un candado, no hay garantías sobre el orden en el que se procesarán.

Siéntete libre de criticar si crees que he pasado por alto algo.

 public class Foo { private static ConcurrentDictionary _lockDictionary = new ConcurrentDictionary(); public void DoSomethingThreadCriticalByString(string lockString) { object thisThreadSyncObject = new object(); lock (thisThreadSyncObject) { try { for (; ; ) { object runningThreadSyncObject = _lockDictionary.GetOrAdd(lockString, thisThreadSyncObject); if (runningThreadSyncObject == thisThreadSyncObject) break; lock (runningThreadSyncObject) { // Wait for the currently processing thread to finish and try inserting into the dictionary again. } } // Do your work here. } finally { // Remove the key from the lock dictionary object dummy; _lockDictionary.TryRemove(lockString, out dummy); } } } }