RFR(S): 8149557: Resource mark breaks printing to string stream

[Lindenmaier, Goetz]

Hi Thomas,

thanks for looking at this.

> you should check the return value of os::strdup().
strdup returns null on oom, %s should be ‘null’, and
free handles null properly, too.  So I think this is fine.

If I remove the RM, I might break callers. E.g. TestLogTouchedMethods.java.
Also, in that test, all logged method names will accumulate in memory.

Best regards,
  Goetz.


From: Thomas Stüfe [mailto:thomas.stuefe at gmail.com]
Sent: Thursday, February 11, 2016 9:41 AM
To: Lindenmaier, Goetz <goetz.lindenmaier at sap.com>
Cc: hotspot-runtime-dev at openjdk.java.net
Subject: Re: RFR(S): 8149557: Resource mark breaks printing to string stream

Hi Goetz,

Thank you for fixing this. I see you are restless :-)

you should check the return value of os::strdup(). The change looks fine otherwise, though I wonder whether we could just omit the ResourceMark and using the one of the caller. At least it is inconsistent, because Symbol::print_utf8_on() does not have a ResourceMark either.

Kind regards, Thomas



On Thu, Feb 11, 2016 at 9:24 AM, Lindenmaier, Goetz <goetz.lindenmaier at sap.com<mailto:goetz.lindenmaier at sap.com>> wrote:
Hi

Please review this change. I please need a sponsor.
http://cr.openjdk.java.net/~goetz/wr16/8149557-ResMrk/webrev.01/

If a stringStream is passed to Symbol::print_symbol_on(),
assertion "stringStream is re-allocated with a different ResourceMark." can fire in stringStream::write(const char* s, size_t len).
This can be verified by running TestLogTouchedMethods.java after adding the patch below to the VM.

My fix copies the string to locally managed memory before popping the ResoreceMark.
A better solution would be to guarantee enough space on the stream
before doing the resource mark, but outputStream has no such
method as 'ensureCapacity' or the like.
Alternatively the ResourceMark could be skipped altogether.
But then the aux memory used remains on the ResourceArea too long.
Also, various ResourceMarks have to be added in other places.

Best regards,
  Goetz.


--- a/src/share/vm/runtime/java.cpp     Wed Jan 13 11:33:21 2016 +0100
+++ b/src/share/vm/runtime/java.cpp     Wed Feb 10 08:56:14 2016 +0100
@@ -340,7 +340,10 @@
   }

   if (LogTouchedMethods && PrintTouchedMethodsAtExit) {
-    Method::print_touched_methods(tty);
+    ResourceMark rm;
+    stringStream st(16);
+    Method::print_touched_methods(&st);
+    tty->print("%s", st.as_string());
   }

   if (PrintBiasedLockingStatistics) {