Idiome correct pour gérer plusieurs ressources chaînées dans le bloc try-with-resources?

168

La syntaxe Java 7 try-with-resources (également connue sous le nom de bloc ARM ( Automatic Resource Management )) est agréable, courte et simple lors de l'utilisation d'une seule AutoCloseableressource. Cependant, je ne sais pas quelle est l'idiome correct lorsque je dois déclarer plusieurs ressources qui dépendent les unes des autres, par exemple a FileWriteret aBufferedWriter qui l'enveloppent. Bien entendu, cette question concerne tous les cas où certaines AutoCloseableressources sont encapsulées, pas seulement ces deux classes spécifiques.

J'ai proposé les trois alternatives suivantes:

1)

L'idiome naïf que j'ai vu est de déclarer uniquement le wrapper de niveau supérieur dans la variable gérée par ARM:

static void printToFile1(String text, File file) {
    try (BufferedWriter bw = new BufferedWriter(new FileWriter(file))) {
        bw.write(text);
    } catch (IOException ex) {
        // handle ex
    }
}

C'est joli et court, mais c'est cassé. Comme le sous FileWriter- jacent n'est pas déclaré dans une variable, il ne sera jamais fermé directement dans le finallybloc généré . Il sera fermé uniquement par la closeméthode de l'emballage BufferedWriter. Le problème est que si une exception est levée depuis le bwconstructeur de s, closeelle ne sera pas appelée et donc le sous-jacent FileWriter ne sera pas fermé .

2)

static void printToFile2(String text, File file) {
    try (FileWriter fw = new FileWriter(file);
            BufferedWriter bw = new BufferedWriter(fw)) {
        bw.write(text);
    } catch (IOException ex) {
        // handle ex
    }
}

Ici, la ressource sous-jacente et la ressource d'encapsulation sont déclarées dans les variables gérées par ARM, donc les deux seront certainement fermées, mais le sous-jacent fw.close() sera appelé deux fois : non seulement directement, mais également via l'encapsulation bw.close().

Cela ne devrait pas être un problème pour ces deux classes spécifiques qui implémentent toutes les deux Closeable (qui est un sous-type de AutoCloseable), dont le contrat stipule que plusieurs appels àclose sont autorisés:

Ferme ce flux et libère toutes les ressources système qui lui sont associées. Si le flux est déjà fermé, l'invocation de cette méthode n'a aucun effet.

Cependant, dans un cas général, je peux avoir des ressources qui implémentent uniquement AutoCloseable(et non Closeable), ce qui ne garantit pas queclose puissent être appelées plusieurs fois:

Notez que contrairement à la méthode close de java.io.Closeable, cette méthode close n'a pas besoin d'être idempotente. En d'autres termes, appeler cette méthode close plus d'une fois peut avoir un effet secondaire visible, contrairement à Closeable.close qui doit n'avoir aucun effet s'il est appelé plus d'une fois. Cependant, les implémenteurs de cette interface sont fortement encouragés à rendre leurs méthodes proches idempotentes.

3)

static void printToFile3(String text, File file) {
    try (FileWriter fw = new FileWriter(file)) {
        BufferedWriter bw = new BufferedWriter(fw);
        bw.write(text);
    } catch (IOException ex) {
        // handle ex
    }
}

Cette version devrait être théoriquement correcte, car seul le fwreprésente une ressource réelle qui doit être nettoyée. Le bwlui-même ne contient aucune ressource, il ne délègue qu'au fw, il devrait donc suffire de fermer uniquement le sous-jacent fw.

D'un autre côté, la syntaxe est un peu irrégulière et aussi, Eclipse émet un avertissement, qui je crois est une fausse alerte, mais c'est quand même un avertissement auquel il faut faire face:

Fuite de ressources: 'bw' n'est jamais fermé


Alors, quelle approche adopter? Ou ai-je manqué un autre idiome qui est le bon ?

Natix
la source
4
Bien sûr, si le constructeur de FileWriter sous-jacent lève une exception, il ne s'ouvre même pas et tout va bien. Le premier exemple concerne ce qui se passe si le FileWriter est créé, mais que le constructeur de BufferedWriter lève une exception.
Natix
6
Il est intéressant de noter que BufferedWriter ne lèvera pas d'exception. Y at-il un exemple auquel vous pouvez penser où cette question n'est pas purement académique.
Peter Lawrey
10
@PeterLawrey Oui, vous avez raison de dire que le constructeur de BufferedWriter dans ce scénario ne lèvera probablement pas d'exception, mais comme je l'ai souligné, cette question concerne toutes les ressources de style décorateur. Mais par exemple public BufferedWriter(Writer out, int sz)peut lancer un fichier IllegalArgumentException. De plus, je peux étendre BufferedWriter avec une classe qui lèverait quelque chose de son constructeur ou créerait un wrapper personnalisé dont j'ai besoin.
Natix
5
Le BufferedWriterconstructeur peut facilement lever une exception. OutOfMemoryErrorest probablement le plus courant car il alloue une bonne partie de la mémoire pour le tampon (bien qu'il puisse indiquer que vous souhaitez redémarrer l'ensemble du processus). / Vous avez besoin de flushvotre BufferedWritersi vous ne fermez pas et que vous souhaitez conserver le contenu (généralement uniquement le cas sans exception). FileWriterramasse tout ce qui se trouve être le codage de fichier "par défaut" - il vaut mieux être explicite.
Tom Hawtin - tackline
10
@Natix Je souhaite que toutes les questions de SO soient aussi bien documentées et claires que celle-ci. J'aimerais pouvoir voter 100 fois.
Geek

Réponses:

75

Voici mon avis sur les alternatives:

1)

try (BufferedWriter bw = new BufferedWriter(new FileWriter(file))) {
    bw.write(text);
}

Pour moi, la meilleure chose à faire en Java depuis le C ++ traditionnel il y a 15 ans était que vous pouviez faire confiance à votre programme. Même si les choses sont dans la boue et vont mal, ce qu'elles font souvent, je veux que le reste du code soit sur le meilleur comportement et l'odeur des roses. En effet, le BufferedWriterpourrait jeter une exception ici. Manquer de mémoire ne serait pas inhabituel, par exemple. Pour les autres décorateurs, savez-vous lequel desjava.io classes wrapper lancent une exception vérifiée de leurs constructeurs? Je ne. La compréhension du code n'est pas très bonne si vous vous fiez à ce genre de connaissances obscures.

Il y a aussi la «destruction». S'il y a une condition d'erreur, vous ne voulez probablement pas vider les ordures dans un fichier qui doit être supprimé (code pour cela non affiché). Bien que, bien sûr, la suppression du fichier soit également une autre opération intéressante à effectuer en tant que gestion des erreurs.

En règle générale, vous voulez que les finallyblocs soient aussi courts et fiables que possible. L'ajout de flushs n'aide pas cet objectif. Pour de nombreuses versions, certaines des classes de mise en mémoire tampon dans le JDK avaient un bogue où une exception de l' flushintérieur closeprovoquée closesur l'objet décoré n'était pas appelée. Bien que cela ait été corrigé depuis un certain temps, attendez-vous à d'autres implémentations.

2)

try (
    FileWriter fw = new FileWriter(file);
    BufferedWriter bw = new BufferedWriter(fw)
) {
    bw.write(text);
}

Nous sommes toujours en train de rincer le bloc implicite finally (maintenant avec close - cela s'aggrave à mesure que vous ajoutez plus de décorateurs), mais la construction est sûre et nous devons finalement bloquer implicitement afin que même un échec flushn'empêche pas la libération de ressources.

3)

try (FileWriter fw = new FileWriter(file)) {
    BufferedWriter bw = new BufferedWriter(fw);
    bw.write(text);
}

Il y a un bug ici. Devrait être:

try (FileWriter fw = new FileWriter(file)) {
    BufferedWriter bw = new BufferedWriter(fw);
    bw.write(text);
    bw.flush();
}

Certains décorateurs mal implémentés sont en fait des ressources et devront être fermés de manière fiable. De plus, certains flux peuvent avoir besoin d'être fermés d'une manière particulière (peut-être font-ils de la compression et doivent-ils écrire des bits pour terminer, et ne peuvent pas simplement tout vider.

Verdict

Bien que 3 soit une solution techniquement supérieure, des raisons de développement logiciel font de 2 le meilleur choix. Cependant, try-with-resource est toujours une solution inadéquate et vous devez vous en tenir à l' idiome Execute Around , qui devrait avoir une syntaxe plus claire avec des fermetures dans Java SE 8.

Tom Hawtin - ligne de pêche
la source
4
Dans la version 3, comment savez-vous que bw n'a pas besoin d'appeler son close? Et même si vous pouvez en être sûr, n'est-ce pas aussi une «connaissance obscure» comme vous le mentionnez pour la version 1?
TimK
3
"Les raisons du développement logiciel font de 2 le meilleur choix " Pouvez-vous expliquer cette affirmation plus en détail?
Duncan Jones
8
Pouvez-vous donner un exemple de "Exécuter autour de l'idiome avec des fermetures"
Markus
2
Pouvez-vous expliquer ce qu'est «une syntaxe plus claire avec des fermetures dans Java SE 8»?
petertc
1
Voici un exemple de "Execute Around idiom": stackoverflow.com/a/342016/258772
mrts
20

Le premier style est celui suggéré par Oracle .BufferedWriterne lève pas d'exceptions vérifiées, donc si une exception est levée, le programme ne devrait pas s'en remettre, ce qui rend la récupération des ressources pratiquement inutile.

Surtout parce que cela pouvait se produire dans un thread, avec la mort du thread mais le programme continuait toujours - disons, il y avait une panne de mémoire temporaire qui n'était pas assez longue pour sérieusement affecter le reste du programme. C'est un cas plutôt compliqué, cependant, et si cela arrive assez souvent pour faire de la fuite de ressources un problème, l'essai avec des ressources est le moindre de vos problèmes.

Daniel C. Sobral
la source
2
C'est également l'approche recommandée dans la 3e édition de Effective Java.
shmosel
5

Option 4

Modifiez vos ressources pour qu'elles soient fermables et non automatiquement si vous le pouvez. Le fait que les constructeurs puissent être chaînés implique qu'il n'est pas rare de fermer la ressource deux fois. (C'était également vrai avant ARM.) Plus d'informations ci-dessous.

Option 5

N'utilisez pas ARM et code très soigneusement pour vous assurer que close () n'est pas appelé deux fois!

Option 6

N'utilisez pas ARM et ayez vos appels enfin close () dans un try / catch eux-mêmes.

Pourquoi je ne pense pas que ce problème soit propre à ARM

Dans tous ces exemples, les appels finally close () doivent être dans un bloc catch. Laissé pour la lisibilité.

Pas bon car fw peut être fermé deux fois. (ce qui convient à FileWriter mais pas dans votre exemple hypothétique):

FileWriter fw = null;
BufferedWriter bw = null;
try {
  fw = new FileWriter(file);
  bw = new BufferedWriter(fw);
  bw.write(text);
} finally {
  if ( fw != null ) fw.close();
  if ( bw != null ) bw.close();
}

Pas bon car fw n'est pas fermé en cas d'exception lors de la construction d'un BufferedWriter. (encore une fois, cela ne peut pas arriver, mais dans votre exemple hypothétique):

FileWriter fw = null;
BufferedWriter bw = null;
try {
  fw = new FileWriter(file);
  bw = new BufferedWriter(fw);
  bw.write(text);
} finally {
  if ( bw != null ) bw.close();
}
Jeanne Boyarsky
la source
3

Je voulais juste m'appuyer sur la suggestion de Jeanne Boyarsky de ne pas utiliser ARM mais de m'assurer que FileWriter est toujours fermé exactement une fois. Ne pensez pas qu'il y a des problèmes ici ...

FileWriter fw = null;
BufferedWriter bw = null;
try {
    fw = new FileWriter(file);
    bw = new BufferedWriter(fw);
    bw.write(text);
} finally {
    if (bw != null) bw.close();
    else if (fw != null) fw.close();
}

Je suppose que comme ARM n'est que du sucre syntaxique, nous ne pouvons pas toujours l'utiliser pour remplacer finalement des blocs. Tout comme nous ne pouvons pas toujours utiliser une boucle for-each pour faire quelque chose qui est possible avec les itérateurs.

AmyGamy
la source
5
Si votre tryet vos finallyblocs lancent des exceptions, cette construction perdra la première (et potentiellement la plus utile).
rxg
3

Pour être d'accord avec les commentaires précédents: le plus simple est (2) d'utiliser les Closeableressources et de les déclarer dans l'ordre dans la clause try-with-resources. Si vous ne l'avez que AutoCloseable, vous pouvez les envelopper dans une autre classe (imbriquée) qui vérifie juste et qui closen'est appelée qu'une seule fois (Facade Pattern), par exemple en ayant private bool isClosed;. En pratique, même Oracle enchaîne simplement (1) les constructeurs et ne gère pas correctement les exceptions à mi-chemin de la chaîne.

Vous pouvez également créer manuellement une ressource chaînée à l'aide d'une méthode de fabrique statique; cela encapsule la chaîne et gère le nettoyage s'il échoue en partie:

static BufferedWriter createBufferedWriterFromFile(File file)
  throws IOException {
  // If constructor throws an exception, no resource acquired, so no release required.
  FileWriter fileWriter = new FileWriter(file);
  try {
    return new BufferedWriter(fileWriter);  
  } catch (IOException newBufferedWriterException) {
    try {
      fileWriter.close();
    } catch (IOException closeException) {
      // Exceptions in cleanup code are secondary to exceptions in primary code (body of try),
      // as in try-with-resources.
      newBufferedWriterException.addSuppressed(closeException);
    }
    throw newBufferedWriterException;
  }
}

Vous pouvez ensuite l'utiliser comme ressource unique dans une clause try-with-resources:

try (BufferedWriter writer = createBufferedWriterFromFile(file)) {
  // Work with writer.
}

La complexité vient de la gestion de plusieurs exceptions; sinon, il s'agit simplement de "fermer les ressources que vous avez acquises jusqu'à présent". Une pratique courante semble être de commencer par initialiser la variable qui contient l'objet qui contient la ressource null(ici fileWriter), puis d'inclure une vérification nulle dans le nettoyage, mais cela semble inutile: si le constructeur échoue, il n'y a rien à nettoyer, nous pouvons donc laisser cette exception se propager, ce qui simplifie un peu le code.

Vous pourriez probablement le faire de manière générique:

static <T extends AutoCloseable, U extends AutoCloseable, V>
    T createChainedResource(V v) throws Exception {
  // If constructor throws an exception, no resource acquired, so no release required.
  U u = new U(v);
  try {
    return new T(u);  
  } catch (Exception newTException) {
    try {
      u.close();
    } catch (Exception closeException) {
      // Exceptions in cleanup code are secondary to exceptions in primary code (body of try),
      // as in try-with-resources.
      newTException.addSuppressed(closeException);
    }
    throw newTException;
  }
}

De même, vous pouvez chaîner trois ressources, etc.

D'un point de vue mathématique, vous pouvez même enchaîner trois fois en chaînant deux ressources à la fois, et ce serait associatif, ce qui signifie que vous obtiendriez le même objet en cas de succès (car les constructeurs sont associatifs), et les mêmes exceptions en cas d'échec dans l'un des constructeurs. En supposant que vous ayez ajouté un S à la chaîne ci-dessus (donc vous commencez par un V et vous terminez par un S , en appliquant tour à tour U , T et S ), vous obtenez la même chose soit si vous enchaînez d'abord S et T , puis U , correspondant à (ST) U , ou si vous avez d'abord enchaîné T et U , alorsS , correspondant à S (TU) . Cependant, il serait plus clair d'écrire simplement une chaîne triple explicite dans une seule fonction d'usine.

Nils von Barth
la source
Est-ce que je comprends correctement qu'il faut encore utiliser try-with-resource, comme dans try (BufferedWriter writer = <BufferedWriter, FileWriter>createChainedResource(file)) { /* work with writer */ }?
ErikE
@ErikE Oui, vous auriez toujours besoin d'utiliser try-with-resources, mais vous n'auriez besoin que d'une seule fonction pour la ressource chaînée: la fonction factory encapsule le chaînage. J'ai ajouté un exemple d'utilisation; Merci!
Nils von Barth
2

Étant donné que vos ressources sont imbriquées, vos clauses try-with doivent également être:

try (FileWriter fw=new FileWriter(file)) {
    try (BufferedWriter bw=new BufferedWriter(fw)) {
        bw.write(text);
    } catch (IOException ex) {
        // handle ex
    }
} catch (IOException ex) {
    // handle ex
}
poison
la source
5
Ceci est assez similaire à mon deuxième exemple. Si aucune exception ne se produit, le FileWriter closesera appelé deux fois.
Natix le
0

Je dirais de ne pas utiliser ARM et de continuer avec Closeable. Utilisez une méthode comme,

public void close(Closeable... closeables) {
    for (Closeable closeable: closeables) {
       try {
           closeable.close();
         } catch (IOException e) {
           // you can't much for this
          }
    }

}

Vous devriez également envisager d'appeler close of BufferedWritercar il ne s'agit pas simplement de déléguer la proximité de FileWriter, mais il effectue un nettoyage comme flushBuffer.

sakthisundar
la source
0

Ma solution est de faire un refactoring "méthode d'extraction", comme suit:

static AutoCloseable writeFileWriter(FileWriter fw, String txt) throws IOException{
    final BufferedWriter bw  = new BufferedWriter(fw);
    bw.write(txt);
    return new AutoCloseable(){

        @Override
        public void close() throws IOException {
            bw.flush();
        }

    };
}

printToFile peut être écrit soit

static void printToFile(String text, File file) {
    try (FileWriter fw = new FileWriter(file)) {
        AutoCloseable w = writeFileWriter(fw, text);
        w.close();
    } catch (Exception ex) {
        // handle ex
    }
}

ou

static void printToFile(String text, File file) {
    try (FileWriter fw = new FileWriter(file);
        AutoCloseable w = writeFileWriter(fw, text)){

    } catch (Exception ex) {
        // handle ex
    }
}

Pour les concepteurs de bibliothèques de classes, je leur proposerai d'étendre l' AutoClosableinterface avec une méthode supplémentaire pour supprimer la fermeture. Dans ce cas, nous pouvons alors contrôler manuellement le comportement de fermeture.

Pour les concepteurs de langage, la leçon est que l'ajout d'une nouvelle fonctionnalité peut signifier en ajouter beaucoup d'autres. Dans ce cas Java, la fonctionnalité ARM fonctionnera évidemment mieux avec un mécanisme de transfert de propriété des ressources.

METTRE À JOUR

À l'origine, le code ci-dessus nécessite @SuppressWarningcar l' BufferedWriterintérieur de la fonction nécessiteclose() .

Comme suggéré par un commentaire, si flush()nous devons être appelés avant de fermer l'écrivain, nous devons le faire avant toutreturn instruction (implicite ou explicite) à l'intérieur du bloc try. Il n'y a actuellement aucun moyen de garantir que l'appelant fasse cela, je pense, donc cela doit être documenté pour writeFileWriter.

MISE À JOUR À NOUVEAU

La mise à jour ci-dessus fait @SuppressWarning inutile car elle nécessite que la fonction retourne la ressource à l'appelant, elle-même n'a donc pas besoin d'être fermée. Malheureusement, cela nous ramène au début de la situation: l'avertissement est maintenant renvoyé du côté de l'appelant.

Donc, pour résoudre correctement cela, nous avons besoin d'un personnalisé AutoClosablequi, chaque fois qu'il se ferme, le soulignement BufferedWriterdoit être flush()édité. En fait, cela nous montre une autre façon de contourner l'avertissement, car le BufferWritern'est jamais fermé dans les deux cas.

Moteur de la Terre
la source
L'avertissement a son sens: pouvons-nous être sûrs ici que le bwva réellement écrire les données? Il est mis en mémoire tampon après tout, donc il suffit parfois d'écrire sur le disque (lorsque le tampon est plein et / ou activé flush()et close()méthodes). Je suppose que la flush()méthode devrait être appelée. Mais de toute façon, il n'est pas nécessaire d'utiliser le graveur en mémoire tampon lorsque nous l'écrivons immédiatement en un seul lot. Et si vous ne corrigez pas le code, vous pourriez vous retrouver avec des données écrites dans le fichier dans un ordre incorrect, voire même pas du tout écrites dans le fichier.
Petr Janeček
S'il flush()est nécessaire d'être appelé, cela doit se produire chaque fois que l'appelant a décidé de fermer le FileWriter. Cela devrait donc se produire printToFilejuste avant tout returns dans le bloc try. Cela ne doit pas faire partie de writeFileWriteret donc l'avertissement ne concerne rien à l'intérieur de cette fonction, mais l'appelant de cette fonction. Si nous avons une annotation, @LiftWarningToCaller("wanrningXXX")cela aidera ce cas et similaire.
Earth Engine