Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: resource leak when JournalChannel is not fully initialized #4340

Merged
merged 1 commit into from
May 9, 2024

Conversation

shoothzj
Copy link
Member

@shoothzj shoothzj commented May 5, 2024

Motivation

There is a resource leak in the JournalChannel implementation of Apache Bookkeeper when the channel is closed before it is fully initialized (specifically, before the writing of headers begins). In such cases, the buffer channel (bc) is not yet instantiated, but the file channel (fc) might already be open.

Changes

Updated the close() method in JournalChannel to include a conditional check that closes the file channel (fc) if the buffer channel (bc) is null. This ensures that all resources are properly released, even if the JournalChannel is prematurely closed before full initialization.

wrie header method

private void writeHeader(Journal.BufferedChannelBuilder bcBuilder,
int writeBufferSize) throws IOException {
int headerSize = (V4 == formatVersion) ? VERSION_HEADER_SIZE : HEADER_SIZE;
ByteBuffer bb = ByteBuffer.allocate(headerSize);
ZeroBuffer.put(bb);
bb.clear();
bb.put(magicWord);
bb.putInt(formatVersion);
bb.clear();
fc.write(bb);
bc = bcBuilder.create(fc, writeBufferSize);
forceWrite(true);
nextPrealloc = this.preAllocSize;
fc.write(zeros, nextPrealloc - journalAlignSize);
}

Signed-off-by: ZhangJian He <shoothzj@gmail.com>
@shoothzj
Copy link
Member Author

shoothzj commented May 9, 2024

Copy link
Contributor

@eolivelli eolivelli left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@eolivelli eolivelli merged commit 82a7dd3 into apache:master May 9, 2024
21 checks passed
@shoothzj shoothzj deleted the journal-channel-close branch May 9, 2024 10:48
@hangc0276 hangc0276 added this to the 4.18.0 milestone May 25, 2024
shoothzj added a commit that referenced this pull request May 25, 2024
shoothzj added a commit that referenced this pull request May 25, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants