2012-05-22 5 views
7

Sto costruendo il mio progetto con l'analisi del codice set "Microsoft regole minime" e mi dà CA2000 su questo metodo:Perché questo metodo di analisi del codice causa l'errore CA2000: chiamare Dispose()

private Timer InitializeTimer(double intervalInSeconds) 
{ 
    Timer timer = null; 

    try 
    { 
     timer = new Timer { Interval = intervalInSeconds * 1000, Enabled = true }; 
     timer.Elapsed += timer_Elapsed; 
     timer.Start(); 
    } 
    catch 
    { 
     if (timer != null) 
     { 
      timer.Dispose(); 
     } 
    } 
    return timer; 
} 

Questo metodo crea solo un nuovo System.Timers.Timer da un intervallo in secondi. Ho tre timer in funzione (uno per ogni secondo, uno ogni minuto e uno ogni mezz'ora). Forse è meglio avere un timer e controllare il gestore di eventi scaduti se è passato un minuto o mezz'ora, ma non lo so, questo è più facile in questo momento, è un codice ereditato e non voglio rompere tutto ancora.

Questo metodo mi dà l'infame

Warning 21 CA2000 : Microsoft.Reliability : In method 'TimerManager.InitializeTimer(double)', call System.IDisposable.Dispose on object '<>g__initLocal0' before all references to it are out of scope. 

Ora sto chiamando Dispose il fermo pensato che questo sarebbe sufficiente? Sto anche disponendo tutti i timer nella propria implementazione idispostabile della classe.

Cosa mi manca qui?

+4

Estrarre la sintassi di inizializzazione degli oggetti - che è quello che sta innescando l'allarme (in quanto l'oggetto esiste, costruito, ma non assegnato a 'timer' mentre l'inizializzatore è in esecuzione. Sto cercando di trovare la domanda dupe che si occupa di questo. –

+1

possibile duplicato di [Inizializzatori oggetto in using-block genera avviso di analisi del codice CA2000] (http://stackoverflow.com/questions/3514902/object-initializers-in-using-block-genera-code-analysis-warning-ca2000) –

+3

Beh, non buttare via la sintassi dell'inizializzatore per uno strumento stupido. L'avviso è fasullo, c'è nulla da smaltire finché non viene chiamato Start(). Ingoiando le eccezioni e restituendo un oggetto disposto, ora * che * sarebbe qualcosa di cui lamentarsi. –

risposta

2

Si chiama solo Dispose in caso di un'eccezione (che non si dovrebbe mai gestire con un blocco catch-all BTW, ma questa è un'altra storia). In caso di eccezioni, non si dispone dell'oggetto Timer.

O aggiungere un blocco finally e spostare lo Dispose lì oppure utilizzare un blocco using.

+0

Poiché il timer viene restituito su "non-eccezione", non dovrebbe essere corretto? O meglio, perché dovrebbe * sempre * essere disposto? –

+0

Ok, ma voglio che il timer sopravviva oltre il metodo, poiché è assegnato a un campo della mia classe. Sto eliminando i miei campi nell'implementazione IDisposable. Il mio tentativo di cattura è stato un tentativo di sopprimere l'avvertimento. – Davio

+0

@pst Sì, dovrebbe essere OK, ma CodeAnalysis potrebbe non rilevarlo. Potresti sopprimere quel particolare incidente allora. –

1

L'avviso indica che si sta creando un oggetto monouso e non lo si smaltisce in tutti i casi. Se lo smaltisci correttamente con altri metodi, puoi tranquillamente sopprimere questo avviso (puoi farlo usando lo SuppressMessageAttribute).

+0

Questo è quello che sto facendo in altri casi (Unity's LifeTimeManager), ma volevo controllare prima di farlo qui. – Davio

+0

Sì, ho ricevuto questo avviso abbastanza spesso con oggetti a esecuzione prolungata, come i timer. È solo lì per renderti consapevole che devi disporre l'oggetto da qualche altra parte. –

1

Va bene, ho modificato in questo modo:

private Timer InitializeTimer(double intervalInSeconds) 
    { 
     Timer tempTimer = null; 
     Timer timer; 
     try 
     { 
      tempTimer = new Timer(); 
      tempTimer.Interval = intervalInSeconds * 1000; 
      tempTimer.Enabled = true; 
      tempTimer.Elapsed += timer_Elapsed; 
      tempTimer.Start(); 
      timer = tempTimer; 
      tempTimer = null; 
     } 
     finally 
     { 
      if (tempTimer != null) 
      { 
       tempTimer.Dispose(); 
      } 
     } 
     return timer; 
    } 

Questo è per la documentazione CA2000 e non dà un avvertimento. Avevo trascurato il fatto che la sintassi di inizializzatore dell'oggetto crea un oggetto temporaneo che non può essere smaltito.

Grazie ragazzi!

+0

Si potrebbe voler cambiare quello alla fine con una presa con rethrow. Altrimenti, disporrai il tuo timer prima di restituirlo dal metodo, anche se non viene lanciata alcuna eccezione. –

+0

@NicoleCalinoiu Dispone solo se temp non è nullo e temp non è null solo se non ha raggiunto l'ultima riga del blocco try, il che significa che è stata generata un'eccezione. Questo va in giro a dover catturare e rethow un'eccezione. – juharr

-1

penso che sia molto meglio usare "utilizzando" invece di "try/finally" reference

private Timer InitializeTimer(double intervalInSeconds) 
{ 
    Timer timer; 
    using (var tempTimer = new Timer()) 
    { 
     tempTimer.Interval = intervalInSeconds * 1000; 
     tempTimer.Enabled = true; 
     tempTimer.Elapsed += timer_Elapsed; 
     tempTimer.Start(); 
     timer = tempTimer; 
    } 
    return timer; 
}