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 possible unsafe sharing of internal NIO buffer in CompositeByteBuf #9169

Merged
merged 1 commit into from May 22, 2019

Conversation

Projects
None yet
3 participants
@njhill
Copy link
Member

commented May 21, 2019

Motivation

A small thread-safety bug was introduced during the internal optimizations of ComponentByteBuf made a while back in #8437. When there is a single component which was added as a slice,
internalNioBuffer(int,int) will currently return the unwrapped slice's un-duplicated internal NIO buffer. This is not safe since it could be modified concurrently with other usage of that parent buffer.

Modifications

Delegate internalNioBuffer to nioBuffer in this case, which returns a duplicate. This matches what's done in derived buffers in general (for the same reason). Add unit test.

Result

Fixed possible thread-safety bug

Fix possible unsafe sharing of internal NIO buffer in CompositeByteBuf
Motivation

A small thread-safety bug was introduced during the internal
optimizations of ComponentByteBuf made a while back in #8437. When there
is a single component which was added as a slice,
internalNioBuffer(int,int) will currently return the unwrapped slice's
un-duplicated internal NIO buffer. This is not safe since it could be
modified concurrently with other usage of that parent buffer.

Modifications

Delegate internalNioBuffer to nioBuffer in this case, which returns a
duplicate. This matches what's done in derived buffers in general
(for the same reason). Add unit test.

Result

Fixed possible thread-safety bug
@netty-bot

This comment has been minimized.

Copy link

commented May 21, 2019

Can one of the admins verify this patch?

@normanmaurer

This comment has been minimized.

Copy link
Member

commented May 22, 2019

@netty-bot test this please

@normanmaurer normanmaurer added this to the 4.1.37.Final milestone May 22, 2019

@normanmaurer normanmaurer merged commit 507e0a0 into netty:4.1 May 22, 2019

3 checks passed

pull request validation (centos6-java11) Build finished.
Details
pull request validation (centos6-java12) Build finished.
Details
pull request validation (centos6-java8) Build finished.
Details
@normanmaurer

This comment has been minimized.

Copy link
Member

commented May 22, 2019

@njhill good catch... Thanks!

normanmaurer added a commit that referenced this pull request May 22, 2019

Fix possible unsafe sharing of internal NIO buffer in CompositeByteBuf (
#9169)

Motivation

A small thread-safety bug was introduced during the internal
optimizations of ComponentByteBuf made a while back in #8437. When there
is a single component which was added as a slice,
internalNioBuffer(int,int) will currently return the unwrapped slice's
un-duplicated internal NIO buffer. This is not safe since it could be
modified concurrently with other usage of that parent buffer.

Modifications

Delegate internalNioBuffer to nioBuffer in this case, which returns a
duplicate. This matches what's done in derived buffers in general
(for the same reason). Add unit test.

Result

Fixed possible thread-safety bug

@njhill njhill deleted the njhill:cbb-internalnio branch May 22, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.