Skip to content

SnappyOutputStream.close() is not idempotent #107

@JoshRosen

Description

@JoshRosen

SnappyOutputStream's close() method is not idempotent, which can lead to stream corruption issues when CachedBufferAllocator is used (the default behavior).

Here's the current source for close():

    public void close() throws IOException {
        try {
            flush();
            out.close();
        } finally {
            inputBufferAllocator.release(inputBuffer);
            outputBufferAllocator.release(outputBuffer);
        }
    }

If we call close() twice, then we'll free the buffers twice. After the first close() call, the stream's buffers will be released back to the BachedBufferAllocator's buffer pool for re-use. If we create a new SnappyOutputStream, this stream will re-use the buffers from the BachedBufferAllocator. If we now call close() on the original stream a second time, we'll end up placing the same buffers into the BachedBufferAllocator's buffer pool even though another non-closed SnappyOutputStream is now using them. This can lead to scenarios where two open SnappyOutputStreams share the same buffers, leading to stream corruption issues.

This should be pretty easy to fix; I'm working on a unit test + patch now.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions