Warning 'Resource leak'

Bonjour,

Le code suivant

Code:

Scanner reponse = new Scanner(System.in);

déclenche un warning (ou du moins, je suppose que c'est un warning : une ampoule avec un triangle, sous Eclipse) :

Code:

Resource leak: 'reponse' is never closed

C'est grave docteur ? Que dois-je faire pour corriger ça ?

Christophe

C'est bien un warning.

Ce n'est pas bien grave, puisque ce n'est pas toi qui a ouvert System.in. Il n'y a donc pas de raison que ce soit toi qui le ferme. Il se fermera à la fin du programme, comme cela arrive toujours qu'on mette un Scanner dessus ou pas.

Tu as ce warning parce qu'en général, quand on utilise une ressource fermable pour construire un Scanner, il faut toujours fermer le Scanner (ou la ressource directement, mais c'est pas très bon pour l'encapsulation.)

Bonjour,

Est-ce que si tu fais ceci tu as toujours le warning ?

Code:

1
2
3
4
5
6
7
8
9
Scanner reponse = null;
try{
reponse = new Scanner(System.in);
// ...
}finally{
    if(reponse != null){
        reponse.close();
    }
}

Romain.

Hum. C'est :

Code:

1
2
3
4
5
6
Scanner reponse = new Scanner(System.in);
try{
  // ...
}finally{
  reponse.close();
}

ou en Java 1.7 :

Code:

1
2
3
try(Scanner reponse = new Scanner(System.in)){
  // ...
}

Salut,

+1 avec thelvin pour la meilleure façon de libérer une ressource.

Mais dans ce cas précis on est dans un des rare cas où l'on ne doit pas libérer la ressource.
En effet si on ferme System.in on ne pourra plus l'utiliser, et je doute que cela soit voulu...

Du coup dans ce cas précis, on peut utiliser une annotation pour ignorer le warning :
Code:

1 2 3 // Il ne faut pas fermer System.in ! @SuppressWarnings("resource") Scanner scanner = new Scanner(System.in);
a++

Il serait encore pire de le réutiliser, puisqu'on vient de le mettre dans un Scanner qui va buffuriser tout ce qu'il veut.

D'un autre côté, on a pas de raison non plus de vouloir le fermer, bien sûr, puisqu'on l'a pas ouvert.

Conclusion : en réalité on s'en fiche.

Pourquoi la réponse de Thelvin est meilleur que celle de rg77140 ? (Je sais, c'est pas mon topic, mais je m'incruste ^^)

Citation:

Envoyé par Sharcoux

Je sais, c'est pas mon topic, mais je m'incruste ^^

T'es le bienvenu !
:-)

(et la réponse à ta question m’intéresse grandement.)

Simplement parce que mon code est plus simple, plus court, plus clair, et démontre d'une meilleure compréhension des mécanisme d'allocation et libération de ressources avec try.

Celui de rg77140 fonctionne parfaitement.

Bonjour,

Comme j'ai codé directement sur le forum j'ai pas vu que "new Scanner()" ne pouvait pas lancer d'exception. Dans ce cas là effectivement mon code est lourd et sans intérêt, celui de thelvin est plus clair et plus simple.

J'ai fait ça par habitude car il y a certaines ressources qui peuvent thrower des exceptions lors de leur instanciation. Je vais prendre l'exemple d'un FileReader qui peut thrower une FileNotFoundException. Dans ce cas là on est obligé de faire comme je l'ai montré auparavant.
Code:

1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 26 27 28 Reader reponse = null; try { reponse = new FileReader(new File("reponse.txt")); // lecture de la réponse } catch (FileNotFoundException e) { // gestion exception } catch (IOException e) { // gestion exception } finally { if (reponse != null) { try { reponse.close(); } catch (IOException e) { } } }
On ne peut pas instancier le FileReader en dehors du try car l’instanciation peut thrower une FileNotFoundException (ou il faut faire deux try catch, mais je préfère cette approche qui me semble plus standard de ce que j'en ai vu)... Et dans le finally on est obligé de vérifier si la réponse n'est pas nulle car si New FileReader() a lancé une FileNotFoundException, reponse est null.

On améliore la lisibilité par une classe utilitaire en général. Voir la librairie commons-io et sa classe IOUtils.

Ce qui donne :
Code:

1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 Reader reponse = null; try { reponse = new FileReader(new File("reponse.txt")); // lecture de la réponse } catch (FileNotFoundException e) { // gestion exception } catch (IOException e) { // gestion exception } finally { IOUtils.closeQuietly(reponse); }
Romain.

Citation:

Envoyé par rg77140

J'ai fait ça par habitude car il y a certaines ressources qui peuvent thrower des exceptions lors de leur instanciation. Je vais prendre l'exemple d'un FileReader qui peut thrower une FileNotFoundException. Dans ce cas là on est obligé de faire comme je l'ai montré auparavant.

Non pas forcément.

En utilisant ton pattern, tu es obligé de faire un cas particulier pour l'exception du close(), ce qui pourrait poser problème.
Avec un FileReader en général aucun soucis là dessus, mais si tu fais du réseau cela peut être problématique.

En effet dans certains rare cas le close() peut générer une exception mais tu l'ignores complètement.
Si le traitement ne se déroule pas correctement, ton programme ne le sais pas et il continue donc normalement, mais les données pourraient ne pas être correctement sauvegardé !!!

Bref ce n'est pas propre d'ignorer une exception.

Citation:

Envoyé par rg77140

On ne peut pas instancier le FileReader en dehors du try car l’instanciation peut thrower une FileNotFoundException (ou il faut faire deux try catch, mais je préfère cette approche qui me semble plus standard de ce que j'en ai vu)...

Si tu instancies la ressource avant le try/finnaly, elle ne sera pas fermée en cas d'exception puisqu'on ne passera pas dans le finally (on n'est même pas rentré dans le try).
Et si tu veux traiter les exceptions il faut bien utiliser bien deux blocs "try" :

Un bloc try/finally pour chacune des ressources.
Un bloc try/cacth global pour traiter TOUTES les exceptions.

Exemple :
Code:

1 2 3 4 5 6 7 8 9 10 11 12 13 14 try { final Reader reponse = new FileReader(new File("reponse.txt")); try { // TRAITEMENTS } finally { reponse.close(); } } catch (FileNotFoundException e) { // gestion exception } catch (IOException e) { // gestion exception }
Et si tu as plusieurs ressources, tu dois multiplier les try/finally :
Code:

1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 try { final OutputStream output = new FileOutputStream("output.txt"); try { final InputStream input = new FileInputStream("input.txt"); try { // TRAITEMENTS } finally { input.close(); } } finally { output.close(); } } catch (FileNotFoundException e) { // gestion exception } catch (IOException e) { // gestion exception }
Pour info la try-with-ressource de Java 7 utilise en interne cette solution avec des try/finally, couplé avec un try/catch global si on en a utilisé un.

a++

Personnellement je ne vois que rarement l'intérêt d'un try/catch qui englobe ceux qui font le finally. Gérer les Exceptions c'est bien joli, mais l'Exception elle est arrivée, il y a une erreur, il y a un problème, et ça va pas disparaître d'un coup de baguette magique. On ne peut pas "gérer" l'Exception, ou si on peut, c'est pas notre boulot de le savoir. Pas ici, pas dans la méthode qui ouvre et ferme les ressources.
En général L'Exception doit être remontée à l'appelant. Lui saura quoi faire. S'il faut décorer l'Exception pour la rendre plus explicite, ou la laisser oublier avec un log ou un réessai, etc, ce sera le boulot de l'appelant.

@thelvin : oui en règle général je préfère laisser remonter l'exception au niveau supérieur.
Mais si on doit la traiter le mieux est de faire un try/catch qui englobe le tout...

Par exemple perso j'aime bien encapsuler les exceptions dans des "unchecked-exceptions" (et utiliser un exception-handler par défaut pour les traiter).

a++

Qu'est ce que tu appelles un exception-handler ? Une classe de très haut niveau qui catch toutes tes "unchecked-exceptions" et qui les décorent dans une exception technique?

Romain.

Citation:

Envoyé par rg77140

Qu'est ce que tu appelles un exception-handler ? Une classe de très haut niveau qui catch toutes tes "unchecked-exceptions" et qui les décorent dans une exception technique?

Oui tout à fait.

En Java standard on peut utiliser Thread.setUncaughtExceptionHandler() pour cela.

Ou sinon plus simplement un try/catch englobant plusieurs traitements...

a++