Skip to content

Conversation

miraculix0815
Copy link

Hello Sylvain,

I think it should be totally fine to close a ByteChannel more than once. The implementation of Files.readAllBytes(Path) is

    public static byte[] readAllBytes(Path path) throws IOException {
        try (SeekableByteChannel sbc = Files.newByteChannel(path);
             InputStream in = Channels.newInputStream(sbc)) {
            long size = sbc.size();
            if (size > (long)MAX_BUFFER_SIZE)
                throw new OutOfMemoryError("Required array size too large");

            return read(in, (int)size);
        }
    }

It leads to a double close. One for the InputStream and the other for the SeekableByteChannel. Please pull this patch to fix the Bug.

regards, jan

@SylvainJuge
Copy link
Owner

Yes, you are right,
being able to close more than once can be convenient and may not lead to
bugs like "forget close at least once".

However, for consistency & readeability could you add curly braces for the
two "if" blocks.

Regards,
Sylvain.

On Fri, Jan 22, 2016 at 11:41 AM, Jan Schlößin [email protected]
wrote:

Hello Sylvain,

I think it should be totally fine to close a ByteChannel more than once.
The implementation of Files.readAllBytes(Path) is

public static byte[] readAllBytes(Path path) throws IOException {
    try (SeekableByteChannel sbc = Files.newByteChannel(path);
         InputStream in = Channels.newInputStream(sbc)) {
        long size = sbc.size();
        if (size > (long)MAX_BUFFER_SIZE)
            throw new OutOfMemoryError("Required array size too large");

        return read(in, (int)size);
    }
}

It leads to a double close. One for the InputStream and the other for the
SeekableByteChannel. Please pull this patch to fix the Bug.

regards, jan

You can view, comment on, or merge this pull request online at:

#10
Commit Summary

  • MemoryByteChannel can be closed more than once

File Changes

Patch Links:


Reply to this email directly or view it on GitHub
#10.

@miraculix0815
Copy link
Author

Of course, I changed the bracket style.

In addition to that I inverted the test semantic for double close instead of simply deleting it.

Regards, Jan

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants