RFR(xs): 8224487: outputStream should not be copyable

Kim Barrett kim.barrett at oracle.com
Tue May 21 15:56:53 UTC 2019


> On May 21, 2019, at 9:47 AM, Thomas Stüfe <thomas.stuefe at gmail.com> wrote:
> 
> Hi all,
> may I please have a review for this small fix. With JDK-8224193 (not yet
> pushed) I saw a number of crashes which were caused by code handing around
> copies of outputStream objects but copy semantics are not implemented.
> 
> This fix forbids copying for outputStream and its descendants and fixes the
> few places where that happened.
> 
> Bug: https://bugs.openjdk.java.net/browse/JDK-8224487
> cr:
> http://cr.openjdk.java.net/~stuefe/webrevs/8224487-make-streams-not-copyable/webrev.00/webrev/
> 
> Note: In event log coding, I removed the convenience function stringStream
> FormatStringLogMessage::stream(). This makes the (very few) call sites
> slightly more verbose but I have open for RFR a cleanup of the event log
> system (see JDK-8220762,
> https://mail.openjdk.java.net/pipermail/hotspot-runtime-dev/2019-May/034349.html)
> which will simplify and cleanup this coding anyway.
> 
> Thank you,
> 
> Thomas

Generally nice.  Just a couple of comments.

------------------------------------------------------------------------------
src/hotspot/share/utilities/ostream.hpp
  46    outputStream(const outputStream&);

Copy-assign should also be poisoned.  Hopefully that won't find too
many more problems.

------------------------------------------------------------------------------
src/hotspot/share/utilities/exceptions.hpp
 189   static void log_exception(Handle exception, stringStream tempst);
=>
 189   static void log_exception(Handle exception, const char* message);

Why not change the stringStream parameter to be passed by const-ref
rather than by value?  That would avoid changing all the callers.

------------------------------------------------------------------------------
src/hotspot/share/utilities/events.hpp
Removed from class FormatStringLogMessage
 140  public:
 141   // Wrap this buffer in a stringStream.
 142   stringStream stream() {
 143     return stringStream(this->_buf, this->size());
 144   }

FormatStringLogMessage seems kind of pointless now.  Maybe it should
be eliminated?  That could be a separate cleanup.

------------------------------------------------------------------------------



More information about the hotspot-dev mailing list