¿Es abusivo usar IDisposable y "using" como un medio para obtener un "comportamiento de alcance" para la seguridad de excepción?

112

Algo que solía usar en C ++ era permitir que una clase Amanejara una condición de entrada y salida de estado para otra clase B, a través del Aconstructor y el destructor, para asegurarme de que si algo en ese alcance arrojaba una excepción, entonces B tendría un estado conocido cuando el se salió del alcance. Esto no es RAII puro en lo que respecta al acrónimo, pero es un patrón establecido de todos modos.

En C #, a menudo quiero hacer

class FrobbleManager
{
    ...

    private void FiddleTheFrobble()
    {
        this.Frobble.Unlock();
        Foo();                  // Can throw
        this.Frobble.Fiddle();  // Can throw
        Bar();                  // Can throw
        this.Frobble.Lock();
    }
}

Que debe hacerse así

private void FiddleTheFrobble()
{
    this.Frobble.Unlock();

    try
    {            
        Foo();                  // Can throw
        this.Frobble.Fiddle();  // Can throw
        Bar();                  // Can throw
    }
    finally
    {
        this.Frobble.Lock();
    }
}

si quiero garantizar el Frobbleestado cuando FiddleTheFrobbleregrese. El código sería mejor con

private void FiddleTheFrobble()
{
    using (var janitor = new FrobbleJanitor(this.Frobble))
    {            
        Foo();                  // Can throw
        this.Frobble.Fiddle();  // Can throw
        Bar();                  // Can throw
    }
}

donde FrobbleJanitorse ve más o menos como

class FrobbleJanitor : IDisposable
{
    private Frobble frobble;

    public FrobbleJanitor(Frobble frobble)
    {
        this.frobble = frobble;
        this.frobble.Unlock();
    }

    public void Dispose()
    {
        this.frobble.Lock();
    }
}

Y así es como quiero hacerlo. Ahora la realidad se pone al día, ya que lo que quiero usar requiere que FrobbleJanitorse use con using . Podría considerar esto como un problema de revisión de código, pero algo me está molestando.

Pregunta: ¿Se consideraría lo anterior un uso abusivo de usingy IDisposable?

Johann Gerell
fuente
23
+1 solo para los nombres de clases y métodos :-). Bueno, para ser justos, y la pregunta real.
Joey
2
@Johannes: Sí, puedo ver por qué, la mayoría de la gente simplemente toca su violín, pero debo protestar contra eso. ;-) Y, por cierto, +1 por la similitud de tu nombre.
Johann Gerell
¿Quizás pueda usar el alcance lock (this) {..} para lograr el mismo resultado en este ejemplo?
oɔɯǝɹ
1
@ oɔɯǝɹ: usa lock () para evitar el acceso simultáneo a algo de diferentes hilos. Nada que ver con el escenario que describo.
Johann Gerell
1
IScopeable sin finalizador en la próxima versión de C # :)
Jon Raynor

Respuestas:

33

No lo creo, necesariamente. IDisposable técnicamente está destinado a ser utilizado para cosas que tienen recursos no administrados, pero la directiva using es solo una forma ordenada de implementar un patrón común de try .. finally { dispose }.

Un purista diría "sí, es abusivo", y en el sentido purista lo es; pero la mayoría de nosotros no codificamos desde una perspectiva purista, sino desde una semi-artística. En mi opinión, usar el constructo 'usar' de esta manera es bastante artístico.

Probablemente debería colocar otra interfaz encima de IDisposable para empujarla un poco más lejos, explicando a otros desarrolladores por qué esa interfaz implica IDisposable.

Hay muchas otras alternativas para hacer esto pero, en última instancia, no puedo pensar en ninguna que sea tan ordenada como esta, ¡así que adelante!

Andras Zoltan
fuente
2
Creo que también es un uso artístico de "usar". Creo que la publicación de Samual Jack que enlaza con un comentario de Eric Gunnerson valida esta implementación de "usar". Puedo ver excelentes puntos de Andras y Eric sobre esto.
IAbstract
1
Hablé con Eric Gunnerson sobre esto hace un tiempo y, según él, era la intención del equipo de diseño de C # que el uso denotara los alcances. De hecho, postuló que si hubieran diseñado el uso antes de la declaración de bloqueo, es posible que ni siquiera hubiera una declaración de bloqueo. Habría sido un bloque de uso con una llamada de Monitor o algo así. ACTUALIZACIÓN: Me acabo de dar cuenta de que la siguiente respuesta es de Eric Lippert, que también está en el equipo de idiomas. ;) ¿Quizás el propio equipo de C # no está completamente de acuerdo con esto? El contexto de mi discusión con Gunnerson fue la clase TimedLock
Haacked
2
@Haacked - Divertido, ¿no? su comentario prueba totalmente mi punto; que habías hablado con un chico del equipo y estaba totalmente de acuerdo; luego señalando a Eric Lippert a continuación, del mismo equipo no está de acuerdo. Desde mi punto de vista; C # proporciona una palabra clave que explota una interfaz y también produce un patrón de código atractivo que funciona en muchos escenarios. Si se supone que no debemos abusar de él, entonces C # debería encontrar otra forma de hacer cumplir eso. De cualquier manera, ¡los diseñadores probablemente necesiten poner sus patos en una fila aquí! Mientras tanto, using(Html.BeginForm())señor? no importa si lo hago señor! :)
Andras Zoltan
71

Considero que esto es un abuso de la declaración de uso. Soy consciente de que soy una minoría en este puesto.

Considero que esto es un abuso por tres razones.

Primero, porque espero que "usar" se use para usar un recurso y desecharlo cuando haya terminado con él . Cambiar el estado del programa no significa utilizar un recurso y volver a cambiarlo no significa eliminar nada. Por lo tanto, "usar" para mutar y restaurar el estado es un abuso; el código es engañoso para el lector casual.

En segundo lugar, porque espero que "using" se use por cortesía, no por necesidad . La razón por la que usa "using" para deshacerse de un archivo cuando haya terminado con él no es porque sea necesario hacerlo, sino porque es educado - alguien más podría estar esperando para usar ese archivo, entonces diciendo "listo ahora "es lo moralmente correcto. Espero poder refactorizar un "uso" para que el recurso usado se conserve durante más tiempo y se elimine más tarde, y que el único impacto de hacerlo sea incomodar levemente otros procesos . Un bloque "en uso" que tiene un impacto semántico en el estado del programa. es abusivo porque oculta una mutación importante y requerida del estado del programa en una construcción que parece que está ahí por conveniencia y cortesía, no por necesidad.

Y tercero, las acciones de su programa están determinadas por su estado; la necesidad de una manipulación cuidadosa del estado es precisamente la razón por la que estamos teniendo esta conversación en primer lugar. Consideremos cómo podríamos analizar su programa original.

Si tuviera que llevar esto a una revisión de código en mi oficina, la primera pregunta que haría es "¿es realmente correcto bloquear el frobble si se lanza una excepción?" Es descaradamente obvio por su programa que esta cosa vuelve a bloquear agresivamente el frobble sin importar lo que suceda. ¿Está bien? Se ha lanzado una excepción. El programa se encuentra en un estado desconocido. No sabemos si Foo, Fiddle o Bar arrojaron, por qué lanzaron o qué mutaciones realizaron a otro estado que no se limpiaron. ¿Puedes convencerme de que en esa terrible situación siempre es lo correcto volver a bloquear?

Quizás lo sea, quizás no lo sea. Mi punto es que con el código tal como se escribió originalmente, el revisor del código sabe hacer la pregunta . Con el código que usa "usar", no sé hacer la pregunta; Supongo que el bloque "using" asigna un recurso, lo usa por un tiempo y lo desecha cortésmente cuando está hecho, no que la llave de cierre del bloque "using" mute el estado de mi programa en una circunstancia excepcional cuando arbitrariamente muchos se han violado las condiciones de coherencia del estado del programa.

El uso del bloque "using" para tener un efecto semántico hace que este programa se fragmente:

}

extremadamente significativo. Cuando miro ese único aparato ortopédico, no pienso de inmediato que "el aparato ortopédico tiene efectos secundarios que tienen un impacto de gran alcance en el estado global de mi programa". Pero cuando abusa del "uso" de esta manera, de repente lo hace.

La segunda cosa que preguntaría si viera su código original es "¿qué sucede si se lanza una excepción después del desbloqueo pero antes de que se ingrese el intento?" Si está ejecutando un ensamblado no optimizado, es posible que el compilador haya insertado una instrucción de no operación antes del intento, y es posible que se produzca una excepción de aborto de subproceso en la no operación. Esto es raro, pero sucede en la vida real, particularmente en servidores web. En ese caso, el desbloqueo ocurre pero el bloqueo nunca ocurre, porque la excepción se lanzó antes del intento. Es muy posible que este código sea vulnerable a este problema y, en realidad, debería escribirse

bool needsLock = false;
try
{
    // must be carefully written so that needsLock is set
    // if and only if the unlock happened:

    this.Frobble.AtomicUnlock(ref needsLock);
    blah blah blah
}
finally
{
    if (needsLock) this.Frobble.Lock();
}

Una vez más, tal vez lo haga, tal vez no, pero sé hacer la pregunta . Con la versión "using", es susceptible al mismo problema: se podría lanzar una excepción de aborto de hilo después de que Frobble esté bloqueado pero antes de que se ingrese la región protegida por prueba asociada con el using. Pero con la versión de "uso", supongo que se trata de un "¿y qué?" situación. Es una lástima que eso suceda, pero supongo que el "uso" es solo para ser educado, no para cambiar el estado del programa de vital importancia. Supongo que si ocurre una terrible excepción de aborto de subprocesos exactamente en el momento equivocado, entonces, bueno, el recolector de basura limpiará ese recurso eventualmente ejecutando el finalizador.

Eric Lippert
fuente
18
Aunque respondería la pregunta de manera diferente, creo que es una publicación bien argumentada. Sin embargo, discrepo de su afirmación de que usinges más una cuestión de cortesía que de corrección. Creo que las fugas de recursos son problemas de corrección, aunque a menudo son lo suficientemente menores como para no ser errores de alta prioridad. Por lo tanto, los usingbloques ya tienen un impacto semántico en el estado del programa, y ​​lo que evitan no es solo "inconvenientes" para otros procesos, sino posiblemente un entorno roto. Eso no quiere decir que el diferente tipo de impacto semántico que implica la pregunta también sea apropiado.
kvb
13
Las fugas de recursos son problemas de corrección, sí. Pero si no se utiliza un "uso" no se pierde el recurso; el recurso se limpiará eventualmente cuando se ejecute el finalizador. Es posible que la limpieza no sea tan agresiva y oportuna como le gustaría, pero sucederá.
Eric Lippert
7
Maravillosa publicación, aquí está mi granito de arena :) Sospecho que mucho "abuso" usingse debe a que es un tejedor orientado al aspecto de un hombre pobre en C #. Lo que el desarrollador realmente quiere es una forma de garantizar que algún código común se ejecute al final de un bloque sin tener que duplicar ese código. C # no es compatible con AOP hoy (no obstante, MarshalByRefObject y PostSharp). Sin embargo, la transformación del compilador de la declaración using hace posible lograr AOP simple al volver a proponer la semántica de la disposición de recursos determinista. Si bien no es una buena práctica, a veces es atractivo
dejarlo
11
Si bien en general estoy de acuerdo con su posición, hay algunos casos en los que el beneficio de la reorientación usinges demasiado atractivo para renunciar. El mejor ejemplo en el que puedo pensar es rastrear y reportar tiempos de código: using( TimingTrace.Start("MyMethod") ) { /* code */ } nuevamente, esto es AOP: Start()captura la hora de inicio antes de que comience el bloque, Dispose()captura la hora de finalización y registra la actividad. No cambia el estado del programa y es independiente de las excepciones. También es atractivo porque evita bastante plomería y su uso frecuente como patrón mitiga la confusa semántica.
LBushkin
6
Es curioso, siempre pensé en usarlo como un medio para apoyar a RAII. Me parece razonablemente intuitivo considerar un candado como un tipo de recurso.
Brian
27

Si solo desea un código limpio y con ámbito, también puede usar lambdas, á la

myFribble.SafeExecute(() =>
    {
        myFribble.DangerDanger();
        myFribble.LiveOnTheEdge();
    });

donde el .SafeExecute(Action fribbleAction)método envuelve el bloque try- catch- finally.

herzmeister
fuente
En ese ejemplo, se perdió el estado de entrada. Además, asegúrese de completar try / finalmente, pero no llama a nada finalmente, por lo que si algo en la lambda arroja, no tiene idea de en qué estado se encuentra.
Johann Gerell
1
¡Ah! Ok, supongo que quieres decir que SafeExecutees un Fribblemétodo que llama Unlock()y Lock()en el empaquetado try / finalmente. Mis disculpas entonces. Inmediatamente lo vi SafeExecutecomo un método de extensión genérico y, por lo tanto, mencioné la falta de estado de entrada y salida. Culpa mía.
Johann Gerell
1
Tenga mucho cuidado con este enfoque. ¡Podría haber algunas extensiones de vida peligrosas, sutiles y no deseadas en el caso de los lugareños capturados!
Jason
4
Puaj. ¿Por qué esconder intentar / atrapar / finalmente? Hace que su código se oculte para que otros lo puedan leer.
Frank Schwieterman
2
@Yukky Frank: No fue mi idea "ocultar" nada, fue la solicitud del interrogador. :-P ... Dicho esto, la solicitud es por fin una cuestión de "No se repita". Es posible que tenga muchos métodos que requieran el mismo "marco" estándar de adquirir / liberar algo limpiamente y no desee imponer eso a la persona que llama (piense en la encapsulación). También se podrían nombrar métodos como .SafeExecute (...) más claramente para comunicar lo suficientemente bien lo que hacen.
herzmeister
25

Eric Gunnerson, que estaba en el equipo de diseño del lenguaje C #, dio esta respuesta a prácticamente la misma pregunta:

Doug pregunta:

re: Una declaración de bloqueo con tiempo de espera ...

He hecho este truco antes para lidiar con patrones comunes en numerosos métodos. Por lo general, bloquear la adquisición , pero hay algunos otros . El problema es que siempre se siente como un truco ya que el objeto no es realmente desechable tanto como " call-back-at-the-end-of-a-scope-able ".

Doug,

Cuando decidimos [sic] la declaración using, decidimos nombrarla “using” en lugar de algo más específico para eliminar objetos para que pudiera usarse exactamente para este escenario.

Samuel Jack
fuente
4
Bueno, esa cita debería decir a qué se refería cuando dijo "exactamente este escenario", ya que dudo que se estuviera refiriendo a esta pregunta futura.
Frank Schwieterman
4
@Frank Schwieterman: Completé la cita. Al parecer, las personas del equipo de C # hicieron pensar la usingcaracterística fue que no se limite a disposición de recursos.
paercebal
11

Es una pendiente resbaladiza. IDisposable tiene un contrato, uno que está respaldado por un finalizador. Un finalizador es inútil en tu caso. No puede obligar al cliente a usar la declaración de uso, solo anímelo a hacerlo. Puedes forzarlo con un método como este:

void UseMeUnlocked(Action callback) {
  Unlock();
  try {
    callback();
  }
  finally {
    Lock();
  }
}

Pero eso tiende a ser un poco incómodo sin lamdas. Dicho esto, he usado IDisposable como lo hizo usted.

Sin embargo, hay un detalle en su publicación que lo acerca peligrosamente a un anti-patrón. Mencionaste que esos métodos pueden generar una excepción. Esto no es algo que la persona que llama pueda ignorar. Puede hacer tres cosas al respecto:

  • No haga nada, la excepción no es recuperable. El caso normal. Llamar a Desbloquear no importa.
  • Atrapa y maneja la excepción
  • Restaurar el estado en su código y dejar que la excepción pase por la cadena de llamadas.

Los dos últimos requieren que la persona que llama escriba explícitamente un bloque try. Ahora la declaración de uso se interpone. Bien puede inducir a un cliente a un coma que le haga creer que su clase se está ocupando del estado y que no es necesario realizar ningún trabajo adicional. Eso casi nunca es exacto.

Hans Passant
fuente
El caso forzado fue dado por "herzmeister der welten" arriba, creo, incluso si al OP no pareció gustarle el ejemplo.
Benjamin Podszun
Sí, interpreté el suyo SafeExecutecomo un método de extensión genérico. Ahora veo que probablemente estaba destinado a ser un Fribblemétodo que llama Unlock()y Lock()en el intento envuelto / finalmente. Culpa mía.
Johann Gerell
Ignorar una excepción no significa que no sea recuperable. Significa que se manejará arriba en la pila. Por ejemplo, las excepciones se pueden utilizar para salir de un hilo con elegancia. En este caso, debe liberar correctamente sus recursos, incluidos los bloqueos bloqueados.
paercebal
7

Un ejemplo del mundo real es BeginForm de ASP.net MVC. Básicamente puedes escribir:

Html.BeginForm(...);
Html.TextBox(...);
Html.EndForm();

o

using(Html.BeginForm(...)){
    Html.TextBox(...);
}

Html.EndForm llama a Dispose y Dispose solo genera la </form>etiqueta. Lo bueno de esto es que los corchetes {} crean un "alcance" visible que hace que sea más fácil ver qué hay dentro del formulario y qué no.

No lo usaría en exceso, pero esencialmente IDisposable es solo una forma de decir "TIENES que llamar a esta función cuando hayas terminado". MvcForm lo usa para asegurarse de que el formulario esté cerrado, Stream lo usa para asegurarse de que el flujo esté cerrado, puede usarlo para asegurarse de que el objeto esté desbloqueado.

Personalmente, solo lo usaría si las siguientes dos reglas son verdaderas, pero las establezco arbitrariamente:

  • Dispose debe ser una función que siempre debe ejecutarse, por lo que no debe haber ninguna condición excepto Null-Checks
  • Después de Dispose (), el objeto no debería ser reutilizable. Si quisiera un objeto reutilizable, prefiero darle métodos de abrir / cerrar en lugar de desecharlo. Entonces lanzo una InvalidOperationException cuando intento usar un objeto desechado.

Al final, se trata de expectativas. Si un objeto implementa IDisposable, supongo que necesita hacer una limpieza, así que lo llamo. Creo que suele ser mejor que tener una función de "apagado".

Dicho esto, no me gusta esta línea:

this.Frobble.Fiddle();

Como el FrobbleJanitor "es dueño" del Frobble ahora, me pregunto si no sería mejor llamar a Fiddle en el Frobble en el conserje.

Michael Stum
fuente
Acerca de tu "No me gusta esta línea", en realidad pensé brevemente en hacerlo de esa manera al formular la pregunta, pero no quería saturar la semántica de mi pregunta. Pero estoy de acuerdo contigo. Mas o menos. :)
Johann Gerell
1
Votado a favor por proporcionar un ejemplo de la propia Microsoft. Tenga en cuenta que hay una excepción específica que se lanzará en el caso de que mencione: ObjectDisposedException.
Antitoon
4

Tenemos mucho uso de este patrón en nuestra base de código, y lo he visto por todas partes antes; estoy seguro de que también debe haber sido discutido aquí. En general, no veo qué hay de malo en hacer esto, proporciona un patrón útil y no causa ningún daño real.

Simon Steele
fuente
4

Interviniendo en esto: estoy de acuerdo con la mayoría aquí en que esto es frágil, pero útil. Me gustaría señalarle la clase System.Transaction.TransactionScope , que hace algo como usted quiere hacer.

Generalmente me gusta la sintaxis, elimina mucho desorden de la carne real. Sin embargo, considere darle un buen nombre a la clase de ayuda, tal vez ... Alcance, como el ejemplo anterior. El nombre debería revelar que encapsula un fragmento de código. * Alcance, * Bloque o algo similar debería hacerlo.

Benjamin Podszun
fuente
1
El "Conserje" proviene de un antiguo artículo de C ++ de Andrei Alexandrescu sobre los protectores de alcance, incluso antes de que ScopeGuard llegara a Loki.
Johann Gerell
@Benjamin: Me gusta la clase TransactionScope, no había oído hablar de ella antes. Tal vez porque estoy en la tierra de .Net CF, donde no está incluido ... :-(
Johann Gerell
4

Nota: Mi punto de vista probablemente esté sesgado por mi experiencia en C ++, por lo que el valor de mi respuesta debe evaluarse contra ese posible sesgo ...

¿Qué dice la especificación del lenguaje C #?

Citando la especificación del lenguaje C # :

8.13 La declaración de uso

[...]

Un recurso es una clase o estructura que implementa System.IDisposable, que incluye un único método sin parámetros llamado Dispose. El código que usa un recurso puede llamar a Dispose para indicar que el recurso ya no es necesario. Si no se llama a Dispose, la eliminación automática eventualmente ocurre como consecuencia de la recolección de basura.

El código que está usando un recurso es, por supuesto, el código que comienza con la usingpalabra clave y va hasta el alcance adjunto al using.

Entonces supongo que esto está bien porque el bloqueo es un recurso.

Quizás la palabra clave usingfue mal elegida. Quizás debería haber sido llamado scoped.

Entonces, podemos considerar casi cualquier cosa como un recurso. Un identificador de archivo. Una conexión de red ... ¿Un hilo?

¿¿¿Un hilo???

¿Usar (o abusar) de la usingpalabra clave?

¿Sería brillante (ab) usar la usingpalabra clave para asegurarse de que el trabajo del hilo finalice antes de salir del alcance?

Herb Sutter parece pensar que es brillante , ya que ofrece un uso interesante del patrón IDispose para esperar a que termine el trabajo de un hilo:

http://www.drdobbs.com/go-parallel/article/showArticle.jhtml?articleID=225700095

Aquí está el código, copiado y pegado del artículo:

// C# example
using( Active a = new Active() ) {    // creates private thread
       
       a.SomeWork();                  // enqueues work
       
       a.MoreWork();                   // enqueues work
       
} // waits for work to complete and joins with private thread

Si bien no se proporciona el código C # para el objeto activo, está escrito que C # usa el patrón IDispose para el código cuya versión C ++ está contenida en el destructor. Y al mirar la versión C ++, vemos un destructor que espera a que termine el hilo interno antes de salir, como se muestra en este otro extracto del artículo:

~Active() {
    // etc.
    thd->join();
 }

Entonces, en lo que respecta a Herb, es brillante .

paercebal
fuente
3

Creo que la respuesta a su pregunta es no, esto no sería un abuso de IDisposable.

La forma en que entiendo la IDisposableinterfaz es que se supone que no debe trabajar con un objeto una vez que se ha eliminado (excepto que puede llamar a su Disposemétodo tantas veces como desee).

Dado que crea un nuevo FrobbleJanitor explícitamente cada vez que llega a la usingdeclaración, nunca usa el mismo FrobbeJanitorobjeto dos veces. Y dado que su propósito es administrar otro objeto, Disposeparece apropiado para la tarea de liberar este recurso ("administrado").

(Por cierto, el código de muestra estándar que demuestra la implementación adecuada de Disposecasi siempre sugiere que los recursos administrados también deben liberarse, no solo los recursos no administrados, como los identificadores del sistema de archivos).

Lo único que me preocupa personalmente es que está menos claro lo que sucede using (var janitor = new FrobbleJanitor())con un try..finallybloque más explícito donde las operaciones Lock& Unlockson directamente visibles. Pero el enfoque que se adopte probablemente se deba a una cuestión de preferencias personales.

stakx - ya no contribuye
fuente
1

No es abusivo. Los está utilizando para lo que fueron creados. Pero es posible que tenga que considerar uno tras otro según sus necesidades. Por ejemplo, si está optando por 'arte', entonces puede usar 'usar', pero si su código se ejecuta muchas veces, entonces, por razones de rendimiento, puede usar las construcciones 'probar' .. 'finalmente'. Porque 'usar' generalmente involucra creaciones de un objeto.

ata
fuente
1

Creo que lo hiciste bien. La sobrecarga de Dispose () sería un problema, ya que la misma clase tuvo que realizar una limpieza más tarde, y la vida útil de esa limpieza cambió para ser diferente al momento en que espera mantener un bloqueo. Pero dado que creó una clase separada (FrobbleJanitor) que es responsable solo de bloquear y desbloquear el Frobble, las cosas están lo suficientemente desacopladas como para que no llegue a ese problema.

Sin embargo, cambiaría el nombre de FrobbleJanitor, probablemente a algo como FrobbleLockSession.

Frank Schwieterman
fuente
Acerca del cambio de nombre: utilicé "Conserje" por la razón principal por la que traté de bloquear / desbloquear. Pensé que rimaba con las otras opciones de nombres. ;-) Si tuviera que usar este método como un patrón a lo largo de mi base de código, definitivamente incluiría algo más descriptivo genéricamente, como insinúas con "Sesión". Si estos hubieran sido los viejos días de C ++, probablemente usaría FrobbleUnlockScope.
Johann Gerell