dschneider-pivotal commented on a change in pull request #6943:
URL: https://github.com/apache/geode/pull/6943#discussion_r725195184
##########
File path:
geode-core/src/main/java/org/apache/geode/distributed/internal/ReplyMessage.java
##########
@@ -274,10 +274,11 @@ public void toData(DataOutput out,
SerializationContext context) throws IOException {
super.toData(out, context);
- final HeapDataOutputStream hdos =
- new HeapDataOutputStream(context.getSerializationVersion());
+ HeapDataOutputStream hdos = null;
boolean failedSerialization = false;
- if (this.returnValueIsException || this.returnValue != null) {
+ final boolean hasReturnValue = this.returnValueIsException ||
this.returnValue != null;
+ if (hasReturnValue) {
+ hdos = new HeapDataOutputStream(context.getSerializationVersion());
Review comment:
something else to consider in this method is should you even call
hdos.close? If you look at it all it is doing is freeing up some heap resources
and reseting the BufferDataOutputStream which actually mean you could continue
to use it (which seems a bit off since close usually ends the life of an
object). If you had a HDOS that was stored in something that was going to live
longer than close could help free some memory up early. But in this case HDOS
becomes garbage at the end of this method. The best practice in that case is to
just let the garbage collector free it up and to not class close. But I'd be
okay with you leaving the close call in but if it is going to cause a major
refactor you might want to instead not call it.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
