Skip to content

fix: Don't make MqttPublishMessage ensure payload accessibility, close #14507#1

Open
slandelle wants to merge 1 commit into
netty-contrib:mainfrom
slandelle:fix-14507
Open

fix: Don't make MqttPublishMessage ensure payload accessibility, close #14507#1
slandelle wants to merge 1 commit into
netty-contrib:mainfrom
slandelle:fix-14507

Conversation

@slandelle

Copy link
Copy Markdown

Motivations:

  • it's not the responsibility of MqttPublishMessage, which is a ByteBufHolder to ensure the accessibility of the ByteBuf. The ByteBuf checks it himself.
  • the current design is broken and there's no way to check if the content is accessible as refCnt crashes with
    IllegalReferenceCountException instead of returning 0.

Modification:

Remove ByteBufUtil.ensureAccessible from MqttPublishMessage#content. ByteBuf's methods will check themselves.

…e #14507

Motivations:

* it's not the responsibility of MqttPublishMessage, which is a
ByteBufHolder to ensure the accessibility of the ByteBuf. The ByteBuf
checks it himself.
* the current design is broken and there's no way to check if the
content is accessible as refCnt crashes with
IllegalReferenceCountException instead of returning 0.

Modification:

Remove ByteBufUtil.ensureAccessible from MqttPublishMessage#content.
ByteBuf's methods will check themselves.
@slandelle

Copy link
Copy Markdown
Author

Ping @normanmaurer

No CI here?

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.

1 participant