2010-11-19 13 views
28

Necesito bloquear una sección de código por cadena. Por supuesto, el siguiente código es terriblemente inseguro:Bloqueando por cadena. ¿Esto es seguro/sensato?

lock("http://someurl") 
{ 
    //bla 
} 

Así que he estado cocinando una alternativa. Normalmente no soy de los que publican grandes cantidades de código aquí, pero cuando se trata de programación concurrente, estoy un poco aprensivo acerca de hacer mi propio esquema de sincronización, así que estoy enviando mi código para preguntar si está bien hacerlo. de esta manera o si hay un enfoque más directo.

public class StringLock 
{ 
    private readonly Dictionary<string, LockObject> keyLocks = new Dictionary<string, LockObject>(); 
    private readonly object keyLocksLock = new object(); 

    public void LockOperation(string url, Action action) 
    { 
     LockObject obj; 
     lock (keyLocksLock) 
     { 
      if (!keyLocks.TryGetValue(url, 
             out obj)) 
      { 
       keyLocks[url] = obj = new LockObject(); 
      } 
      obj.Withdraw(); 
     } 
     Monitor.Enter(obj); 
     try 
     { 
      action(); 
     } 
     finally 
     { 
      lock (keyLocksLock) 
      { 
       if (obj.Return()) 
       { 
        keyLocks.Remove(url); 
       } 
       Monitor.Exit(obj); 
      } 
     } 
    } 

    private class LockObject 
    { 
     private int leaseCount; 

     public void Withdraw() 
     { 
      Interlocked.Increment(ref leaseCount); 
     } 

     public bool Return() 
     { 
      return Interlocked.Decrement(ref leaseCount) == 0; 
     } 
    } 
} 

lo usaría como esto:

StringLock.LockOperation("http://someurl",()=>{ 
    //bla 
}); 

bueno para ir, o pesadez?

EDITAR

Para la posteridad, aquí es mi código de trabajo. Gracias por todas las sugerencias:

public class StringLock 
{ 
    private readonly Dictionary<string, LockObject> keyLocks = new Dictionary<string, LockObject>(); 
    private readonly object keyLocksLock = new object(); 

    public IDisposable AcquireLock(string key) 
    { 
     LockObject obj; 
     lock (keyLocksLock) 
     { 
      if (!keyLocks.TryGetValue(key, 
             out obj)) 
      { 
       keyLocks[key] = obj = new LockObject(key); 
      } 
      obj.Withdraw(); 
     } 
     Monitor.Enter(obj); 
     return new DisposableToken(this, 
            obj); 
    } 

    private void ReturnLock(DisposableToken disposableLock) 
    { 
     var obj = disposableLock.LockObject; 
     lock (keyLocksLock) 
     { 
      if (obj.Return()) 
      { 
       keyLocks.Remove(obj.Key); 
      } 
      Monitor.Exit(obj); 
     } 
    } 

    private class DisposableToken : IDisposable 
    { 
     private readonly LockObject lockObject; 
     private readonly StringLock stringLock; 
     private bool disposed; 

     public DisposableToken(StringLock stringLock, LockObject lockObject) 
     { 
      this.stringLock = stringLock; 
      this.lockObject = lockObject; 
     } 

     public LockObject LockObject 
     { 
      get 
      { 
       return lockObject; 
      } 
     } 

     public void Dispose() 
     { 
      Dispose(true); 
      GC.SuppressFinalize(this); 
     } 

     ~DisposableToken() 
     { 
      Dispose(false); 
     } 

     private void Dispose(bool disposing) 
     { 
      if (disposing && !disposed) 
      { 
       stringLock.ReturnLock(this); 
       disposed = true; 
      } 
     } 
    } 

    private class LockObject 
    { 
     private readonly string key; 
     private int leaseCount; 

     public LockObject(string key) 
     { 
      this.key = key; 
     } 

     public string Key 
     { 
      get 
      { 
       return key; 
      } 
     } 

     public void Withdraw() 
     { 
      Interlocked.Increment(ref leaseCount); 
     } 

     public bool Return() 
     { 
      return Interlocked.Decrement(ref leaseCount) == 0; 
     } 
    } 
} 

utiliza como sigue:

var stringLock=new StringLock(); 
//... 
using(stringLock.AcquireLock(someKey)) 
{ 
    //bla 
} 
+2

se ve más de la ingeniería para mí, pero es difícil de decir sin saber cuál es el problema que se supone que resolver. ¿Es solo para evitar bloquear una cadena? – LukeH

+0

@LukeH, estoy tratando de escribir un caché de imágenes donde es probable que varios clientes (más de un 100) soliciten la imagen simultáneamente. El primer hit solicitará la imagen desde otro servidor, y me gustaría que todas las demás solicitudes a la caché se bloqueen hasta que se complete esta operación para que puedan recuperar la versión en caché. En mi opinión, esto requerirá bloqueo de esta naturaleza, pero si hay alguna alternativa, soy todo oídos. – spender

+0

@Ani, sí, estoy al tanto de esto (como se describe en mi publicación) y si considero que este enfoque es un aficionado, reforzaré las defensas. – spender

Respuesta

10

Otra opción es obtener el HashCode de cada URL, luego dividirlo por un número primo y usarlo como un índice en una matriz de bloqueos. Esto limitará el número de bloqueos que necesita mientras le permite controlar la probabilidad de un "bloqueo falso" al elegir la cantidad de bloqueos que debe usar.

Sin embargo, lo anterior solo vale la pena si es demasiado costoso solo tiene un candado por url activo.

+2

La gran ventaja de este código es que es bastante simple de implementar, ya que a diferencia del código basado en el diccionario, no necesita eliminar objetos de bloqueo para evitar una pérdida de memoria. Este es el único código en este tema que me gustaría implementar correctamente sin pensar demasiado. Y, por supuesto, tener más de un bloqueo a la vez en un hilo se pone realmente feo. – CodesInChaos

+0

Gracias amigo! Sé que esto es como 4 años más tarde, pero me ayudaste a encontrar una gran solución para un problema que estoy teniendo. :) – Haney

+0

@Ian Ringrose ¿me pueden dar una muestra del código? –

14

de bloqueo por una instancia cadena arbitraria sería una mala idea, porque Monitor.Lock bloquea la instancia. Si tenía dos instancias de cadena diferentes con el mismo contenido, serían dos bloqueos independientes, que no desea. Así que tienes razón en preocuparte por cadenas arbitrarias.

Sin embargo, .NET ya tiene un mecanismo incorporado para devolver la "instancia canónica" del contenido de una cadena determinada: String.Intern. Si le pasa dos instancias de cadena diferentes con el mismo contenido, obtendrá la misma instancia de resultado las dos veces.

lock (string.Intern(url)) { 
    ... 
} 

Esto es más simple; hay menos código para que pruebes, porque estarías confiando en lo que ya está en el Framework (que, presumiblemente, ya funciona). Hace

+1

Monitor.Enter no bloquea la instancia. La instancia es solo un token utilizado para implementar el protocolo de sincronización. –

+0

Esto es * precisamente * lo que el OP intenta evitar. Si otro hilo estaba ejecutando código de un módulo diferente que decidió bloquear la misma cadena interna (por coincidencia), habría una contienda completamente innecesaria. El OP está tratando de proporcionar al usuario la facilidad de bloqueo de cadenas, pero sin que el grupo interno de .NET se interponga en el camino. – Ani

+3

Y la memoria interna de pérdidas – CodesInChaos

2

Alrededor de 2 años tuve que poner en práctica lo mismo:

Estoy intentando escribir un caché de imágenes donde múltiples clientes (más de un 100 o así) es probable que solicite la imagen simultaneamente. El primer hit solicitará la imagen de otro servidor, y me gustaría que todas las demás solicitudes en el caché se bloqueen hasta que esta operación sea completa para que puedan recuperar la versión en caché .

Terminé con el código haciendo lo mismo que el suyo (el diccionario de LockObjects). Bueno, el tuyo parece estar mejor encapsulado.

Por lo tanto, creo que tiene una buena solución. A tan sólo 2 comentarios:

  1. Si necesita aún mejor peformance que puede resultar útil para utilizar algún tipo de ReadWriteLock, ya que tiene 100 lectores y sólo el 1 escritor conseguir la imagen desde otro servidor.

  2. No estoy seguro de lo que sucede en caso de cambio de hilo justo antes de Monitor.Enter (obj); en su LockOperation(). Digamos que el primer hilo que desea la imagen construye un nuevo bloqueo y luego el interruptor de hilo justo antes de que entre en la sección crítica. Entonces podría suceder que el segundo hilo entre en la sección crítica antes que el primero. Bien podría ser que esto no es un problema real.

+0

Eso significa que mi segundo punto es relevante si confía en que el primer hilo que ingresa en la sección crítica es el mismo que creó el bloqueo. Por ejemplo, si se trata de este primer hilo que activa la búsqueda de la imagen desde otro servidor. – Adesit

2

Usted ha creado un envoltorio bastante compleja en torno a una declaración de bloqueo simple. ¿No sería mejor crear un diccionario de URL y crear un objeto de bloqueo para todos y cada uno? Simplemente podrías hacer.

objLink = GetUrl("Url"); //Returns static reference to url/lock combination 
lock(objLink.LockObject){ 
    //Code goes here 
} 

Incluso se puede simplificar esta bloqueando el objeto objLink Wich directamente podría ser la instancia cadena getURL ("URL"). (Sin embargo, debe bloquear la lista estática de cadenas)

Código original si es muy propenso a errores. Si el código:

if (obj.Return()) 
{ 
keyLocks.Remove(url); 
} 

Si el original por último código produce una excepción uno se queda con un estado no válido cuadro LockObject.

+0

Las únicas excepciones que debería arrojar este código son las excepciones asincrónicas como StackOverflow, OutOfMemory y el aborto de subprocesos, en cuyo caso debe descargar el dominio de la aplicación de todos modos, para que no importe mucho. – CodesInChaos

-1

En la mayoría de los casos, cuando cree que necesita bloqueos, no es así. En su lugar, intente utilizar una estructura de datos segura para subprocesos (por ejemplo, Queue) que maneje el bloqueo por usted.

Por ejemplo, en python:

class RequestManager(Thread): 
    def __init__(self): 
     super().__init__() 
     self.requests = Queue() 
     self.cache = dict() 
    def run(self):   
     while True: 
      url, q = self.requests.get() 
      if url not in self.cache: 
       self.cache[url] = urlopen(url).read()[:100] 
      q.put(self.cache[url]) 

    # get() runs on MyThread's thread, not RequestManager's 
    def get(self, url): 
     q = Queue(1) 
     self.requests.put((url, q)) 
     return q.get() 

class MyThread(Thread): 
    def __init__(self): 
     super().__init__() 
    def run(self): 
     while True: 
      sleep(random()) 
      url = ['http://www.google.com/', 'http://www.yahoo.com', 'http://www.microsoft.com'][randrange(0, 3)] 
      img = rm.get(url) 
      print(img) 
+0

OP pidió una solución C# – JJS

-1

ahora tengo el mismo problema. Decidí usar solución simple: para evitar estancamientos crear una nueva cadena con prefijo único fijo y utilizarlo como un objeto de bloqueo:

lock(string.Intern("uniqueprefix" + url)) 
{ 
// 
} 
+3

Eso es un poco menos terrible que no asegurarse de que está internado, pero sigue siendo una muy mala idea. No deberías estar bloqueando los datos que están disponibles públicamente; debe bloquear los datos que solo son accesibles a los bloques de código que deberían poder bloquearlo. – Servy

5

Aquí está una sencilla , solución muy elegante y correcta para .NET 4 Uso ConcurrentDictionary adaptado de this question.

public static class StringLocker 
{ 
    private static readonly ConcurrentDictionary<string, object> _locks = new ConcurrentDictionary<string, object>(); 

    public static void DoAction(string s, Action action) 
    { 
     lock(_locks.GetOrAdd(s, new object())) 
     { 
      action(); 
     } 
    } 
} 

Puede utilizar este modo:

StringLocker.DoAction("http://someurl",() => 
{ 
    ... 
}); 
+3

Esta implementación nunca reclamará bloqueos no utilizados, y terminará consumiendo mucha memoria en un proceso de larga ejecución. – CaseyB

+0

@CaseyB, el bloqueo se reclamará al final del bloque de código 'lock'. Creo que te refieres a que los * objetos de bloqueo * no utilizados permanecerán en el diccionario '_locks'. Esto es cierto, pero el uso de la memoria será proporcional al número de URL ... y si hace algunos cálculos rápidos verá que el número de URL debe ser ** extremadamente ** grande para que la memoria sea un problema. –

+0

@ZaidMasud: Esa es una idea válida, pero tal vez CaseyB tiene un punto ... podría resolverse envolviendo 'action();' dentro de un try-finally y eliminando la entrada de cadena de '_locks' en la cláusula finally – digEmAll

0

que añade una solución en Bardock.Utils paquete inspirado por @ Eugene-Beresovsky answer.

Uso:

private static LockeableObjectFactory<string> _lockeableStringFactory = 
    new LockeableObjectFactory<string>(); 

string key = ...; 

lock (_lockeableStringFactory.Get(key)) 
{ 
    ... 
} 

Solution code:

namespace Bardock.Utils.Sync 
{ 
    /// <summary> 
    /// Creates objects based on instances of TSeed that can be used to acquire an exclusive lock. 
    /// Instanciate one factory for every use case you might have. 
    /// Inspired by Eugene Beresovsky's solution: https://stackoverflow.com/a/19375402 
    /// </summary> 
    /// <typeparam name="TSeed">Type of the object you want lock on</typeparam> 
    public class LockeableObjectFactory<TSeed> 
    { 
     private readonly ConcurrentDictionary<TSeed, object> _lockeableObjects = new ConcurrentDictionary<TSeed, object>(); 

     /// <summary> 
     /// Creates or uses an existing object instance by specified seed 
     /// </summary> 
     /// <param name="seed"> 
     /// The object used to generate a new lockeable object. 
     /// The default EqualityComparer<TSeed> is used to determine if two seeds are equal. 
     /// The same object instance is returned for equal seeds, otherwise a new object is created. 
     /// </param> 
     public object Get(TSeed seed) 
     { 
      return _lockeableObjects.GetOrAdd(seed, valueFactory: x => new object()); 
     } 
    } 
} 
0

Así es como he implementado este esquema de bloqueo:

public class KeyLocker<TKey> 
{ 
    private class KeyLock 
    { 
     public int Count; 
    } 

    private readonly Dictionary<TKey, KeyLock> keyLocks = new Dictionary<TKey, KeyLock>(); 

    public T ExecuteSynchronized<T>(TKey key, Func<TKey, T> function) 
    { 
     KeyLock keyLock = null; 
     try 
     {    
      lock (keyLocks) 
      { 
       if (!keyLocks.TryGetValue(key, out keyLock)) 
       { 
        keyLock = new KeyLock(); 
        keyLocks.Add(key, keyLock); 
       } 
       keyLock.Count++; 
      } 
      lock (keyLock) 
      { 
       return function(key); 
      } 
     } 
     finally 
     {   
      lock (keyLocks) 
      { 
       if (keyLock != null && --keyLock.Count == 0) keyLocks.Remove(key); 
      } 
     } 
    } 

    public void ExecuteSynchronized(TKey key, Action<TKey> action) 
    { 
     this.ExecuteSynchronized<bool>(key, k => 
     { 
      action(k); 
      return true; 
     }); 
    } 
} 

y usados ​​como esto:

private locker = new KeyLocker<string>(); 
...... 

void UseLocker() 
{ 
    locker.ExecuteSynchronized("some string",() => DoSomething()); 
}