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

[Thomas Stüfe]

Hi Goetz,

this looks fine. Thank you for the fix!

Thomas

On Mon, Feb 29, 2016 at 2:09 PM, Lindenmaier, Goetz <
goetz.lindenmaier at sap.com> wrote:

> Hi David, Thomas,
>
> you are right, I need to check for NULL. I had the linux
> case in mind.
> New webrev:
> http://cr.openjdk.java.net/~goetz/wr16/8149557-ResMrk/webrev.02/
>
> Best regards,
>   Goetz.
>
>
> > -----Original Message-----
> > From: David Holmes [mailto:david.holmes at oracle.com]
> > Sent: Montag, 22. Februar 2016 01:55
> > To: Lindenmaier, Goetz <goetz.lindenmaier at sap.com>; 'Thomas Stüfe'
> > <thomas.stuefe at gmail.com>
> > Cc: hotspot-runtime-dev at openjdk.java.net
> > Subject: Re: RFR(S): 8149557: Resource mark breaks printing to string
> stream
> >
> > On 19/02/2016 11:05 PM, Lindenmaier, Goetz wrote:
> > > 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
> >
> > What are you assuming takes care of %s to 'null' conversion? gcc printf
> > may do it but AFAIK it is undefined at the C library level.
> >
> > David
> > -----
> >
> > > 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) {
> > >
>