Bataille avec le Unity et la règle CA2000

Version imprimable

Bonjour,

Je dois reprendre un projet avec Unity car mon manager veut que je corrige tous les avertissement remonté par l'analyseur de code.

A l'origine il y a ce bout de code qui provoque l'avertissement CA2000.
- Il s'agit d'une méthode static dans une classe static (si ça peut aider)
- Container est également est de type UnityContainer
- new HierarchicalLifetimeManager() déclenche l'avertissement
Code:

1 2 3 Container.RegisterType<ICustomRepository<Customer>, CustomRepository<Customer>>(new HierarchicalLifetimeManager()); Container.RegisterType<ICustomRepository<Provider>, CustomRepository<Provider>>(new HierarchicalLifetimeManager()); // Encore une dizaine de lignes sur le même moule
En cherchant un peu sur le net, j'ai découvert un POST de 2010 sur StackOverFlow (réponse qui a été validée et qui est donc normalement bonne) :
https://stackoverflow.com/questions/...rcontrolledlif

J'ai donc adapté mon code en créant la méthode suivante :
Code:

1 2 3 4 5 6 7 8 9 10 11 12 13 private static void RegisterType<TFrom, TTo>() where TTo : TFrom { var manager = new HierarchicalLifetimeManager(); try { Container.RegisterType<TFrom, TTo>(manager); } catch { manager.Dispose(); throw; } }
Et en l'appelant de la manière suivante :

Code:

RegisterType<ICustomRepository<Customer>, CustomRepository<Customer>>();

Au lieu d'avoir plusieurs avertissement CA2000, j'en ai plus qu'un sur ma nouvelle méthode mais toujours sur le new HierarchicalLifetimeManager

Quelqu'un a-t-il une idée pour corriger cet avertissement ?
Merci.

07/06/2018, 03h54
Noxen

Est-ce-que ça change quelque chose si tu déplaces l'appel au constructeur dans le try ?

J'avais déjà essayé de la manière suivante mais ça n'a rien changé :

Code:

1
2
3
4
5
6
7
8
9
10
11
12
13
14
private static void RegisterType<TFrom, TTo>() where TTo : TFrom
{
    HierarchicalLifetimeManager manager = null;
    try
    {
        manager = new HierarchicalLifetimeManager();
        Container.RegisterType<TFrom, TTo>(manager);
    }
    catch
    {
        manager.Dispose();
        throw;
    }
}

07/06/2018, 22h28
François DORIN
Une première possibilité serait de préciser explicitement d'ignorer l'avertissement en décorant la méthode avec un attribut
Code:

1 2 3 [SuppressMessage("Microsoft.Reliability", "CA2000:DisposeObjectsBeforeLosingScope", Justification = "Propriété transférée à Container")]
.

Sinon, peut être (je n'ai pas restesté) une seconde possibilité c'est de créer une méthode qui s'occupe d'instancier l'instance et la retourne. Donc simplement :
Code:

1 2 3 4 5 private static HierarchicalLifetimeManager CreateManager() { return new HierarchicalLifetimeManager(); }
car il me semble que l'avertissement CA2000 n'est généré que dans la méthode où l'objet est créé. Donc en déportant la création dans un autre objet, l'avertissement devrait disparaître.
08/06/2018, 11h23
Kyroon

Bonjour,

Cet avertissement survient car tu instancie un objet (HierarchicalLifetimeManager) dans ta méthode qui n'est potentiellement jamais disposé (sauf en passant le catch).
- Une solution pourrait être d'instancier ton objet non pas dans la méthode, mais dans la classe (dans le constructeur par exemple) et de le disposer dans le destructeur de la classe (si c'est possible suivant le contexte).
- Je ne connais pas spécialement bien Unity, mais j'ai quand même trouvé ceci : https://docs.microsoft.com/en-us/pre...82(v=pandp.20) qui devrait te permettre de laisser le container faire l'instanciation de l'objet que tu veux passer en paramètre (InjectionConstructor). Ça devrait devrait corriger ton avertissement, car ce n'est plus toi qui fait le new. A tester.

Citation:

Donc en déportant la création dans un autre objet, l'avertissement devrait disparaître.

C'est vrai, mais ça risque aussi de déporter l'avertissement si ce n'est pas l'objet qui fait la création, qui fait le dispose.

Bonne continuation.
08/06/2018, 19h25
DotNetMatt
Ca donne quoi avec un finally ?
Code:

1 2 3 4 5 6 7 8 9 10 11 12 try { // … } catch { throw; } finally { manager.Dispose(); }
Comme cela on s'assure que manager est toujours dispose, exception ou pas exception.
08/06/2018, 19h30
François DORIN

Citation:

Envoyé par DotNetMatt

Ca donne quoi avec un finally ?

Si j'ai bien compris, Container.RegisterType devient le propriétaire de cette instance. Mais il ne faut pas la disposer à la fin de la méthode, sinon elle devient inutilisable par la suite (ce qui n'est pas recherché).
08/06/2018, 20h17
DotNetMatt

Bien vu Francois, je m'etais focalise sur le message lui-meme, et non sur la fonctionnalite :aie: Donc le finally n'est pas une solution ici.
09/06/2018, 20h43
Noxen

IL y a un e-book de référence concernant Unity :
Microsoft Pattern & Practice : Unity Container

Dans les exemples de code proposés concernant le HierarchicalLifetimeManager tous font l'instanciation directement "inline" dans l'appel de fonction, ils n'utilisent pas de structure try/catch. J'aurais tendance à penser que cette structure n'est pas nécessaire en l'occurrence, une erreur ne pourrait se produire que dans le constructeur du HierarchicalLifetimeManager, cas qui me paraît improbable, ou dans la méthode Resolve, qui prend peut-être déjà en charge le Dispose. Quand à savoir si ça jouerai sur le rapport de warning je n'en ai aucune idée.
11/06/2018, 11h43
popo
Citation:
Envoyé par François DORIN

Une première possibilité serait de préciser explicitement d'ignorer l'avertissement en décorant la méthode avec un attribut
Sinon, peut être (je n'ai pas restesté) une seconde possibilité c'est de créer une méthode qui s'occupe d'instancier l'instance et la retourne. Donc simplement :

Code:

1 2 3 4 5 private static HierarchicalLifetimeManager CreateManager() { return new HierarchicalLifetimeManager(); }

car il me semble que l'avertissement CA2000 n'est généré que dans la méthode où l'objet est créé. Donc en déportant la création dans un autre objet, l'avertissement devrait disparaître.
ça ne me génère plus d'avertissement sur le new de cette manière.
Mais l'appel à CreateManager me déclenche le même avertissement.
Dommage.

Citation:

Envoyé par Noxen

IL y a un e-book de référence concernant Unity :
Microsoft Pattern & Practice : Unity Container

Dans les exemples de code proposés concernant le HierarchicalLifetimeManager tous font l'instanciation directement "inline" dans l'appel de fonction, ils n'utilisent pas de structure try/catch. J'aurais tendance à penser que cette structure n'est pas nécessaire en l'occurrence, une erreur ne pourrait se produire que dans le constructeur du HierarchicalLifetimeManager, cas qui me paraît improbable, ou dans la méthode Resolve, qui prend peut-être déjà en charge le Dispose. Quand à savoir si ça jouerai sur le rapport de warning je n'en ai aucune idée.
Citation:
Envoyé par François DORIN

Une première possibilité serait de préciser explicitement d'ignorer l'avertissement en décorant la méthode avec un attribut

Code:

1 2 3 [SuppressMessage("Microsoft.Reliability", "CA2000:DisposeObjectsBeforeLosingScope", Justification = "Propriété transférée à Container")]

.
ça ma poussé à regarder ce que UnityContainer faisait de cet objet et j'ai découvert ceci :
Dans la méthode RegisterType
Code:

1 2 if (registration.LifetimeManager is IDisposable manager) container._lifetimeContainer.Add(manager);
Dans le Dipose de UnityContainer

Code:

_lifetimeContainer.Dispose();

Dans le dispose de LifetimeContainer
Code:

1 2 3 4 5 6 7 8 9 10 11 foreach (var disposable in disposables) { try { disposable.Dispose(); } catch (Exception e) { exceptions.Add(e); } }
Dans la mesure où le container s'occupe d'appeler le Dispose, je vais simplement ignorer l'avertissement.
Merci à vous deux.