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.

Sign up for GitHub

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

Jump to bottom

A number of toString() methods on classes that implement ByteBufHolder can throw IllegalReferenceCountException #4327

Closed
jroper opened this issue Oct 7, 2015 · 2 comments
Closed

A number of toString() methods on classes that implement ByteBufHolder can throw IllegalReferenceCountException #4327

jroper opened this issue Oct 7, 2015 · 2 comments
Assignees
@normanmaurer
Labels
defect
Milestone
4.0.33.Final

Comments

@jroper
Copy link
Contributor

jroper commented Oct 7, 2015

Two examples that I've found include WebSocketFrame.toString() and DefaultByteBufHolder.toString(). toString() implementations should always try to avoid throwing exceptions, since they're used for things like creating exception messages and arbitrary logging of inputs/outputs. Consequently, if toString() does throw an exception, this will often hide another real problem, a typical example would be:

TextWebSocketFrame frame = ...
try {
  String text = frame.text();
  ReferenceCountUtil.release(frame);
  doSomeProcessing(text);
} catch (Exception e) {
  log.error("Error processing frame: " + frame, e);
}

In this example, if doSomeProcessing throws an exception, the exception will be caught, but not logged, instead, when frame.toString() is invoked, that will throw an IllegalReferenceCountException.

I don't think there's any need to guard access to ByteBuf.toString(), since it likewise shouldn't throw an exception for the same reasons as above, so ByteBufHolder toString() implementations should just invoke that directly without going through any reference count checks.
@normanmaurer
Copy link
Member

@jroper sounds like a bug... let me fix it.

@normanmaurer normanmaurer self-assigned this Oct 7, 2015
@normanmaurer normanmaurer added this to the 4.0.33.Final milestone Oct 7, 2015
normanmaurer added a commit that referenced this issue Oct 10, 2015
@normanmaurer
[#4327] Ensure toString() will not throw IllegalReferenceCountException
2fb18be 
Motivation:

As toString() is often used while logging we need to ensure this produces no exception.

Modifications:

Ensure we never throw an IllegalReferenceCountException.

Result:

Be able to log without produce exceptions.
normanmaurer added a commit that referenced this issue Oct 10, 2015
@normanmaurer
[#4327] Ensure toString() will not throw IllegalReferenceCountException
99b4aec 
Motivation:

As toString() is often used while logging we need to ensure this produces no exception.

Modifications:

Ensure we never throw an IllegalReferenceCountException.

Result:

Be able to log without produce exceptions.
normanmaurer added a commit that referenced this issue Oct 10, 2015
@normanmaurer
[#4327] Ensure toString() will not throw IllegalReferenceCountException
99b11c9 
Motivation:

As toString() is often used while logging we need to ensure this produces no exception.

Modifications:

Ensure we never throw an IllegalReferenceCountException.

Result:

Be able to log without produce exceptions.
normanmaurer added a commit that referenced this issue Oct 10, 2015
@normanmaurer
[#4327] Ensure toString() will not throw IllegalReferenceCountException
9faf4c9 
Motivation:

As toString() is often used while logging we need to ensure this produces no exception.

Modifications:

Ensure we never throw an IllegalReferenceCountException.

Result:

Be able to log without produce exceptions.
@normanmaurer
Copy link
Member

Fixed by #4341

pulllock pushed a commit to pulllock/netty that referenced this issue Oct 19, 2023
@normanmaurer
[netty#4327] Ensure toString() will not throw IllegalReferenceCountEx…
9932f27 
…ception

Motivation:

As toString() is often used while logging we need to ensure this produces no exception.

Modifications:

Ensure we never throw an IllegalReferenceCountException.

Result:

Be able to log without produce exceptions.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@normanmaurer normanmaurer

Labels
defect
Projects
None yet
Milestone
4.0.33.Final
Development

No branches or pull requests
2 participants
@jroper @normanmaurer