É abusivo usar IDisposable e “using” como um meio para obter “comportamento com escopo” para segurança de exceção?

112

Algo que eu costumava usar em C ++ era permitir que uma classe Amanipulasse uma condição de entrada e saída de estado para outra classe B, por meio do Aconstrutor e do destruidor, para ter certeza de que se algo nesse escopo gerasse uma exceção, então B teria um estado conhecido quando o o escopo foi encerrado. Este não é um RAII puro no que diz respeito à sigla, mas é um padrão estabelecido mesmo assim.

Em C #, muitas vezes eu quero fazer

class FrobbleManager
{
    ...

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

O que precisa ser feito assim

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

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

se eu quiser garantir o Frobbleestado quando FiddleTheFrobblevoltar. O código seria melhor com

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

onde FrobbleJanitorolhares aproximadamente gostar

class FrobbleJanitor : IDisposable
{
    private Frobble frobble;

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

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

E é assim que eu quero fazer. Agora a realidade me alcança, pois o que eu quero usar exige que o FrobbleJanitorseja usado com using . Eu poderia considerar isso um problema de revisão de código, mas algo está me incomodando.

Pergunta: O acima exposto seria considerado uso abusivo de usinge IDisposable?

Johann Gerell
fonte
23
1 para os nomes de classe e método sozinho :-). Bem, para ser justo, e a questão real.
Joey de
2
@Johannes: Sim, posso ver por quê - a maioria das pessoas apenas brinca com seu violino, mas devo protestar contra isso. ;-) E, a propósito, +1 pela semelhança de seu nome.
Johann Gerell de
Talvez você possa usar o escopo lock (this) {..} para obter o mesmo resultado neste exemplo?
outubro
1
@ oɔɯǝɹ: Você usa lock () para prevenir o acesso simultâneo a algo de diferentes threads. Nada a ver com o cenário que descrevo.
Johann Gerell
1
IScopiável sem finalizador na próxima versão de C # :)
Jon Raynor

Respostas:

33

Acho que não, necessariamente. IDisposable tecnicamente é feito para ser usado para coisas que não têm recursos não gerenciados, mas, em seguida, utilizando a directiva é apenas uma maneira elegante de implementar um padrão comum de try .. finally { dispose }.

Um purista diria 'sim - é abusivo', e no sentido purista é; mas a maioria de nós não codifica de uma perspectiva purista, mas de uma semi-artística. Usar a construção 'usando' dessa maneira é bastante artístico, na minha opinião.

Você provavelmente deve colocar outra interface em cima do IDisposable para empurrá-lo um pouco mais longe, explicando a outros desenvolvedores por que essa interface implica em IDisposable.

Existem muitas outras alternativas para fazer isso, mas, no final das contas, não consigo pensar em nenhuma que seja tão legal quanto essa, então vá em frente!

Andras Zoltan
fonte
2
Eu acredito que é um uso artístico de "usar" também. Acho que a postagem de Samual Jack com um link para um comentário de Eric Gunnerson valida essa implementação de "usar". Posso ver pontos excelentes feitos por Andras e Eric sobre isso.
IAbstract
1
Falei com Eric Gunnerson sobre isso há algum tempo e, de acordo com ele, a intenção da equipe de design do C # era que o uso denotasse escopos. Na verdade, ele postulou que se eles tivessem projetado o uso antes da instrução de bloqueio, poderia nem haver uma instrução de bloqueio. Teria sido um bloco em uso com uma chamada de Monitor ou algo assim. ATUALIZAÇÃO: Acabei de perceber que a próxima resposta é de Eric Lippert, que também está na equipe de idiomas. ;) Talvez a própria equipe C # não esteja totalmente de acordo sobre isso? O contexto da minha discussão com Gunnerson foi a classe TimedLock
Haacked
2
@Haacked - Divertido não é; seu comentário prova totalmente meu ponto; que você tinha falado com um cara da equipe e ele era totalmente a favor; em seguida, apontando Eric Lippert abaixo, da mesma equipe discorda. Do meu ponto de vista; C # fornece uma palavra-chave que explora uma interface e também produz um padrão de código de boa aparência que funciona em muitos cenários. Se não devemos abusar disso, o C # deve encontrar outra maneira de aplicar isso. De qualquer maneira, os designers provavelmente precisarão colocar seus patos em uma linha aqui! Nesse ínterim: using(Html.BeginForm())senhor? não me importo se eu fizer, senhor! :)
Andras Zoltan
71

Eu considero isso um abuso da declaração de uso. Estou ciente de que sou minoria nesta posição.

Considero isso um abuso por três razões.

Primeiro, porque espero que "usando" seja usado para usar um recurso e descartá-lo quando terminar de usá- lo . Alterar o estado do programa não é usar um recurso e alterá-lo de volta não significa descartar nada. Portanto, "usar" para alterar e restaurar o estado é um abuso; o código é enganoso para o leitor casual.

Em segundo lugar, porque espero que "usar" seja usado por educação, não por necessidade . A razão de você usar "usar" para descartar um arquivo quando terminar de usá-lo não é porque é necessário fazer isso, mas porque é educado - outra pessoa pode estar esperando para usar esse arquivo, dizendo "pronto agora "é a coisa moralmente correta a fazer. Espero ser capaz de refatorar um "uso" de modo que o recurso usado seja mantido por mais tempo e descartado mais tarde, e que o único impacto disso seja incomodar um pouco outros processos . Um bloco "usando" que tem impacto semântico no estado do programa é abusivo porque esconde uma mutação importante e necessária do estado do programa em uma construção que parece estar lá por conveniência e educação, não por necessidade.

E terceiro, as ações de seu programa são determinadas por seu estado; a necessidade de manipulação cuidadosa do estado é exatamente o motivo pelo qual estamos tendo essa conversa em primeiro lugar. Vamos considerar como podemos analisar seu programa original.

Se você trouxesse isso para uma revisão de código em meu escritório, a primeira pergunta que eu faria é "é realmente correto bloquear o frobble se uma exceção for lançada?" É flagrantemente óbvio a partir do seu programa que essa coisa bloqueia agressivamente o frobble, não importa o que aconteça. Isso está certo? Uma exceção foi lançada. O programa está em um estado desconhecido. Não sabemos se Foo, Fiddle ou Bar jogou, por que jogou, ou quais mutações eles executaram em outro estado que não foram limpos. Você pode me convencer de que nessa situação terrível é sempre a coisa certa a fazer para travar novamente?

Talvez seja, talvez não. Meu ponto é que, com o código como foi originalmente escrito, o revisor sabe fazer a pergunta . Com o código que usa "using", não sei como fazer a pergunta; Eu suponho que o bloco "usando" aloca um recurso, usa-o por um tempo e o descarta educadamente quando é feito, não que a chave de fechamento do bloco "usando" mude o estado do meu programa em uma circunstância excepcional quando arbitrariamente muitos as condições de consistência do estado do programa foram violadas.

O uso do bloco "usando" para ter um efeito semântico torna este fragmento de programa:

}

extremamente significativo. Quando eu olho para aquela única chave de fechamento, não penso imediatamente "que a chave tem efeitos colaterais que têm impactos de longo alcance no estado global do meu programa". Mas quando você abusa do "uso" assim, de repente acontece.

A segunda coisa que eu perguntaria se visse seu código original é "o que acontece se uma exceção for lançada após o desbloqueio, mas antes que a tentativa seja inserida?" Se você estiver executando um assembly não otimizado, o compilador pode ter inserido uma instrução no-op antes de tentar, e é possível que uma exceção de interrupção de thread ocorra no no-op. Isso é raro, mas acontece na vida real, principalmente em servidores da web. Nesse caso, o desbloqueio acontece, mas o bloqueio nunca acontece, porque a exceção foi lançada antes da tentativa. É perfeitamente possível que este código seja vulnerável a este problema e deva realmente ser escrito

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

Novamente, talvez sim, talvez não, mas eu sei fazer a pergunta . Com a versão "usando", é suscetível ao mesmo problema: uma exceção de anulação de thread pode ser lançada após o Frobble ser bloqueado, mas antes que a região protegida por teste associada ao uso seja inserida. Mas com a versão "usando", presumo que seja um "e daí?" situação. É lamentável se isso acontecer, mas presumo que o "uso" existe apenas para ser educado, não para alterar o estado do programa de vital importância. Presumo que, se alguma exceção terrível de aborto de thread acontecer exatamente no momento errado, então, tudo bem, o coletor de lixo limpará esse recurso eventualmente executando o finalizador.

Eric Lippert
fonte
18
Embora eu responda à pergunta de forma diferente, acho que é uma postagem bem argumentada. No entanto, discordo da sua afirmação de que usingé uma questão de educação e não de correção. Acredito que vazamentos de recursos são problemas de correção, embora frequentemente sejam pequenos o suficiente para não serem bugs de alta prioridade. Assim, os usingblocos têm impacto semântico no estado do programa e o que eles evitam não é apenas "inconveniência" para outros processos, mas possivelmente um ambiente quebrado. Isso não quer dizer que o tipo diferente de impacto semântico implícito na pergunta também seja apropriado.
kvb de
13
Vazamentos de recursos são problemas de correção, sim. Mas deixar de usar um "uso" não vaza o recurso; o recurso será limpo eventualmente quando o finalizador for executado. A limpeza pode não ser tão agressiva e oportuna quanto você gostaria, mas acontecerá.
Eric Lippert de
7
Postagem maravilhosa, aqui estão meus dois centavos :) Suspeito que muito "abuso" de usingé porque é um tecelão orientado para os aspectos do pobre em C #. O que o desenvolvedor realmente quer é uma maneira de garantir que algum código comum seja executado no final de um bloco sem ter que duplicar esse código. C # não oferece suporte a AOP hoje (MarshalByRefObject e PostSharp não suportam). No entanto, a transformação do compilador da instrução using torna possível alcançar AOP simples, re-propondo a semântica de descarte de recursos determinísticos. Embora não seja uma boa prática, às vezes é atraente deixar passar .....
LBushkin
11
Embora eu geralmente concorde com sua posição, há alguns casos em que o benefício de um novo propósito usingé muito atraente para desistir. O melhor exemplo que posso pensar é rastrear e relatar tempos de código: using( TimingTrace.Start("MyMethod") ) { /* code */ } Novamente, isso é AOP - Start()captura a hora de início antes do bloco começar, Dispose()captura a hora de término e registra a atividade. Ele não muda o estado do programa e é independente de exceções. Também é atraente porque evita muitos problemas e seu uso frequente como um padrão mitiga a semântica confusa.
LBushkin
6
Engraçado, sempre pensei em usar como meio de apoiar o RAII. Parece-me razoavelmente intuitivo considerar uma fechadura um tipo de recurso.
Brian
27

Se você quer apenas um código limpo e com escopo definido, você também pode usar lambdas, á la

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

onde o .SafeExecute(Action fribbleAction)método envolve o bloco try- catch- finally.

herzmeister
fonte
Nesse exemplo, você perdeu o estado de entrada. Além disso, certifique-se de embrulhar try / finally, mas você não chama nada finalmente, então se algo no lambda jogar, você não tem ideia de em que estado você está.
Johann Gerell
1
Ah! Ok, eu acho que você quer dizer que SafeExecuteé um Fribblemétodo que chama Unlock()e Lock()no empacotado try / finally. Minhas desculpas então. Eu imediatamente vi SafeExecutecomo um método de extensão genérico e, portanto, mencionei a falta de estado de entrada e saída. Foi mal.
Johann Gerell de
1
Tenha muito cuidado com este apporach. Pode haver algumas extensões de vida perigosas sutis não intencionais no caso de locais capturados!
jason de
4
Yuk. Por que esconder try / catch / finally? Faz com que seu código seja oculto para ser lido por outras pessoas.
Frank Schwieterman de
2
@Yukky Frank: Não foi minha ideia "esconder" nada, foi o pedido do questionador. :-P ... Dito isto, o pedido é enfim uma questão de "Don't Repeat Yourself". Você pode ter muitos métodos que requerem o mesmo "quadro" clichê de aquisição / liberação limpa de algo e você não quer impor isso ao chamador (pense no encapsulamento). Também é possível nomear métodos como .SafeExecute (...) mais claramente para comunicar bem o que eles fazem.
herzmeister
25

Eric Gunnerson, que estava na equipe de design da linguagem C #, deu esta resposta para praticamente a mesma pergunta:

Doug pergunta:

re: uma instrução de bloqueio com tempo limite ...

Já fiz esse truque antes para lidar com padrões comuns em vários métodos. Usualmente , a aquisição de bloqueio , mas existem alguns outros . O problema é que sempre parece um hack, já que o objeto não é realmente descartável, mas sim " capaz de ligar de volta no final de um escopo ".

Doug,

Quando decidimos [sic] a instrução using, decidimos chamá-la de “using” em vez de algo mais específico para descartar objetos, de forma que pudesse ser usada exatamente para este cenário.

Samuel Jack
fonte
4
Bem, essa citação deve dizer a que ele estava se referindo quando disse "exatamente este cenário", pois duvido que ele estivesse se referindo a essa questão futura.
Frank Schwieterman de
4
@Frank Schwieterman: Concluí a citação. Aparentemente, o pessoal da equipe C # achava que o usingrecurso não se limitaria ao descarte de recursos.
paercebal
11

É uma ladeira escorregadia. IDisposable tem um contrato, que é apoiado por um finalizador. Um finalizador é inútil no seu caso. Você não pode forçar o cliente a usar a instrução using, apenas encorajá-lo a fazê-lo. Você pode forçá-lo com um método como este:

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

Mas isso tende a ficar um pouco estranho sem lamdas. Dito isso, usei IDisposable como você fez.

No entanto, há um detalhe em sua postagem que torna isso perigosamente próximo de um antipadrão. Você mencionou que esses métodos podem lançar uma exceção. Isso não é algo que o chamador pode ignorar. Ele pode fazer três coisas sobre isso:

  • Não faça nada, a exceção não é recuperável. O caso normal. Desbloquear chamada não importa.
  • Capture e trate a exceção
  • Restaure o estado em seu código e deixe a exceção passar pela cadeia de chamadas.

Os dois últimos requerem que o chamador escreva explicitamente um bloco try. Agora, a instrução using fica no caminho. Pode muito bem induzir um cliente a um coma que o faça acreditar que sua classe está cuidando do estado e nenhum trabalho adicional precisa ser feito. Isso quase nunca é preciso.

Hans Passant
fonte
O caso forçado foi apresentado por "herzmeister der welten" acima, eu acho - mesmo que o OP não pareça gostar do exemplo.
Benjamin Podszun de
Sim, interpretei o dele SafeExecutecomo um método de extensão genérico. Vejo agora que provavelmente se tratava de um Fribblemétodo que chama Unlock()e Lock()no empacotado try / finally. Foi mal.
Johann Gerell
Ignorar uma exceção não significa que ela não seja recuperável. Isso significa que será tratado acima na pilha. Por exemplo, exceções podem ser usadas para sair de um thread normalmente. Nesse caso, você deve liberar corretamente seus recursos, incluindo bloqueios bloqueados.
paercebal
7

Um exemplo do mundo real é o BeginForm do ASP.net MVC. Basicamente, você pode escrever:

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

ou

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

Html.EndForm chama Dispose, e Dispose apenas produz o </form> tag. A boa coisa sobre isso é que os {} colchetes criam um "escopo" visível que torna mais fácil ver o que está dentro do formulário e o que não está.

Eu não abusaria disso, mas essencialmente IDisposable é apenas uma maneira de dizer "Você TEM que chamar esta função quando terminar". MvcForm o usa para se certificar de que o formulário está fechado, o Stream usa para garantir que o fluxo está fechado, você pode usá-lo para ter certeza de que o objeto está desbloqueado.

Pessoalmente, só o usaria se as duas regras a seguir forem verdadeiras, mas elas são definidas arbitrariamente por mim:

  • Dispose deve ser uma função que sempre deve ser executada, então não deve haver nenhuma condição, exceto Null-Checks
  • Após Dispose (), o objeto não deve ser reutilizável. Se eu quisesse um objeto reutilizável, prefiro fornecer métodos de abrir / fechar em vez de descartá-lo. Portanto, lanço um InvalidOperationException ao tentar usar um objeto descartado.

No final, tudo se resume a expectativas. Se um objeto implementa IDisposable, presumo que ele precise fazer uma limpeza, então eu o chamo. Eu acho que geralmente é melhor ter uma função "Desligar".

Dito isto, não gosto desta linha:

this.Frobble.Fiddle();

Como o FrobbleJanitor "possui" o Frobble agora, eu me pergunto se não seria melhor chamar o Fiddle sobre o Frobble no zelador?

Michael Stum
fonte
Sobre o seu "Não gosto desta linha" - na verdade, pensei brevemente em fazer dessa forma ao formular a pergunta, mas não queria atrapalhar a semântica da minha pergunta. Mas eu meio que concordo com você. Mais ou menos. :)
Johann Gerell
1
Votado por fornecer um exemplo da própria Microsoft. Observe que há uma exceção específica a ser lançada no caso de você mencionar: ObjectDisposedException.
Antitoon de
4

Temos muito uso desse padrão em nossa base de código e já o vi em todos os lugares antes - tenho certeza de que deve ter sido discutido aqui também. Em geral, não vejo o que há de errado em fazer isso, fornece um padrão útil e não causa nenhum dano real.

Simon Steele
fonte
4

Refletindo sobre isso: concordo com a maioria aqui que isso é frágil, mas útil. Eu gostaria de apontar para a classe System.Transaction.TransactionScope , que faz algo como você deseja fazer.

Geralmente eu gosto da sintaxe, ela remove muita desordem da carne real. Por favor, considere dar um bom nome à classe auxiliar - talvez ... Escopo, como o exemplo acima. O nome deve revelar que encapsula um trecho de código. * Scope, * Block ou algo semelhante deve fazê-lo.

Benjamin Podszun
fonte
1
O "Janitor" vem de um antigo artigo C ++ de Andrei Alexandrescu sobre protetores de escopo, mesmo antes de ScopeGuard chegar a Loki.
Johann Gerell
@Benjamin: Eu gosto da classe TransactionScope, nunca tinha ouvido falar dela antes. Talvez porque eu esteja no terreno .Net CF, onde não está incluído ... :-(
Johann Gerell
4

Nota: Meu ponto de vista provavelmente é tendencioso devido ao meu histórico de C ++, então o valor da minha resposta deve ser avaliado em relação a esse possível viés ...

O que diz a especificação da linguagem C #?

Citando a especificação da linguagem C # :

8.13 A declaração de uso

[...]

Um recurso é uma classe ou estrutura que implementa System.IDisposable, que inclui um único método sem parâmetros denominado Dispose. O código que está usando um recurso pode chamar Dispose para indicar que o recurso não é mais necessário. Se Dispose não for chamado, o descarte automático eventualmente ocorrerá como consequência da coleta de lixo.

O código que está usando um recurso é, obviamente, o código que começa pela usingpalavra - chave e vai até o escopo anexado aousing .

Acho que está tudo bem porque a fechadura é um recurso.

Talvez a palavra-chave tenha usingsido mal escolhida. Talvez devesse ter sido chamadoscoped .

Então, podemos considerar quase tudo como um recurso. Um identificador de arquivo. Uma conexão de rede ... Um tópico?

Um tópico ???

Usando (ou abusando) do using palavra chave?

Seria brilhante (ab) usar a usingpalavra-chave para certificar-se de que o trabalho do thread foi encerrado antes de sair do escopo?

Herb Sutter parece pensar que é brilhante , pois oferece um uso interessante do padrão IDispose para esperar que o trabalho de um fio termine:

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

Aqui está o código, copiado e colado do artigo:

// 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

Embora o código C # para o objeto Active não seja fornecido, está escrito que o C # usa o padrão IDispose para o código cuja versão C ++ está contida no destruidor. E ao olhar para a versão C ++, vemos um destruidor que espera que o encadeamento interno termine antes de sair, conforme mostrado neste outro trecho do artigo:

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

Então, no que diz respeito ao Herb, é brilhante .

paercebal
fonte
3

Acredito que a resposta à sua pergunta é não, isso não seria um abuso de IDisposable .

Eu entendo a IDisposableinterface é que você não deve trabalhar com um objeto uma vez que ele foi descartado (exceto que você pode chamá-loDispose método quantas vezes quiser).

Como você cria um novo FrobbleJanitor explicitamente cada vez que acessa a usinginstrução, nunca usa o mesmo FrobbeJanitorobjeto duas vezes. E uma vez que sua finalidade é gerenciar outro objeto,Dispose parece adequado a tarefa de liberar este recurso ("gerenciado").

(Aliás, o código de amostra padrão que demonstra a implementação adequada de Disposequase sempre sugere que os recursos gerenciados também devem ser liberados, não apenas os recursos não gerenciados, como identificadores do sistema de arquivos.)

A única coisa com que eu pessoalmente me preocuparia é que é menos claro o que está acontecendo do using (var janitor = new FrobbleJanitor())que com um try..finallybloco mais explícito onde as operações Lock& Unlocksão diretamente visíveis. Mas a abordagem adotada provavelmente se resume a uma questão de preferências pessoais.

stakx - não contribui mais
fonte
1

Não é abusivo. Você os está usando para o que foram criados. Mas você pode ter que considerar um ao outro de acordo com suas necessidades. Por exemplo, se você está optando por 'arte', então você pode usar 'usando', mas se o seu trecho de código for executado muitas vezes, então por motivos de desempenho você pode usar as construções 'tentar' .. 'finalmente'. Porque 'usar' geralmente envolve a criação de um objeto.

ata
fonte
1

Eu acho que você fez certo. Sobrecarregar Dispose () seria um problema que a mesma classe teria que fazer posteriormente, e o tempo de vida dessa limpeza mudou para ser diferente do tempo em que você espera manter um bloqueio. Mas, como você criou uma classe separada (FrobbleJanitor) que é responsável apenas por bloquear e desbloquear o Frobble, as coisas estão desacopladas o suficiente para que você não encontre esse problema.

Eu mudaria o nome de FrobbleJanitor, provavelmente para algo como FrobbleLockSession.

Frank Schwieterman
fonte
Sobre renomear - usei "Janitor" como o principal motivo pelo qual tratei de Bloquear / Desbloquear. Achei que rimava com as outras opções de nome. ;-) Se eu fosse usar esse método como um padrão em toda a minha base de código, eu definitivamente incluiria algo mais genericamente descritivo, como você sugeriu em "Sessão". Se estivéssemos nos velhos tempos do C ++, provavelmente usaria o FrobbleUnlockScope.
Johann Gerell de