xenoterracide
xenoterracide

Reputation: 16865

Do I need to safely handle resource closing of things delegating to OutputStreamWriter

Eclipse is suggesting there is a resource leak here, I'm unsure because the output stream writing is getting closed.

try ( OutputStreamWriter fileWriter = new OutputStreamWriter( new FileOutputStream( file ), encoder ) )
    {
        fileWriter.append( '\uFEFF' ); // byte order mark
        CSVWriter csvWriter = new CSVWriter( fileWriter ); //  au.com.bytecode.opencsv.CSVWriter
        csvWriter.writeNext( headers );
        streamSupplier.forEachOrdered( row -> {
            if ( row.length != headers.length )
            {
                String exception
                    = String.format( "row length %d should not be different from header length %d",
                                     row.length,
                                     headers.length );
                throw new IllegalStateException( exception );
            }
            csvWriter.writeNext( row );
            processedHandler.accept( processed.nextInt() );
        } );
        csvWriter.flush();
        csvWriter.close();
        return file;
    }
    catch ( IOException e )
    {
        logError( e );
        throw new RuntimeException( e );
    }

is there a resource leak here? is FileOutputStream successfully closed? does it matter if CSVWriter isn't since OutputStreamWriter will be? What is right right way to do this?

Upvotes: 1

Views: 1394

Answers (2)

Vijay
Vijay

Reputation: 552

Since you are using 2 resources OutputStreamWriter and CSVWriter, you should declare/open them within the try-with-resources. In your code, CSVWriter is not part of the try-with-resources declaration block and hence eclipse is highlighting it as a resource leak.

BTW, Couple of ways you can avoid resource leaks:

  • if you are using java version < 1.7, always close the resources in the finally block. something like below:

    Writer writer = null; try { writer = <<Your Writer>>; //Use your writer } catch(IOException e) { //handle any exception }finally { if(writer != null) writer.close(); }

  • If you are using Java Version 1.7+, use the try with resources, like below:

    try(Writer writer = <<Your Writer>>) { //Use your writer } catch(IOException e) { //handle any exception }

Upvotes: -1

Jose Martinez
Jose Martinez

Reputation: 12022

Technically you both are right. You happen to know that the resource that the CsvWriter is closing is in fact a resource that you are sure of will be closed. But Eclipse does not know how CSVWriter functions. It just knows that it is a Closable and has not been closed. To make your code more future proof and stop any alerts like the one you are seeing, you can use the CSVWriter in the try with resources.

try ( CSVWriter csvWriter = new CSVWriter(new OutputStreamWriter( new FileOutputStream( file ), encoder )) )

FYI: You can put both Closables in the try with resources, see example below.

        try (  OutputStreamWriter ow = new OutputStreamWriter( new FileOutputStream( "")); BufferedWriter bw = new BufferedWriter(ow)) {

    } 

Upvotes: 5

Related Questions