RFR: 8251850: Simplify ResourceMark constructors using delegation

[David Holmes]

On 18/08/2020 8:38 pm, Thomas Stüfe wrote:
> On Tue, Aug 18, 2020 at 11:59 AM David Holmes <david.holmes at oracle.com 
> <mailto:david.holmes at oracle.com>> wrote:
> 
>     On 18/08/2020 7:40 pm, Thomas Stüfe wrote:
>      > On Tue, Aug 18, 2020 at 9:52 AM Kim Barrett
>     <kim.barrett at oracle.com <mailto:kim.barrett at oracle.com>
>      > <mailto:kim.barrett at oracle.com <mailto:kim.barrett at oracle.com>>>
>     wrote:
>      >
>      >      > On Aug 17, 2020, at 10:44 PM, David Holmes
>      >     <david.holmes at oracle.com <mailto:david.holmes at oracle.com>
>     <mailto:david.holmes at oracle.com <mailto:david.holmes at oracle.com>>>
>     wrote:
>      >      >
>      >      > Hi Kim,
>      >      >
>      >      > Not a full review - sorry. Have you tested what happens if a
>      >     resource leak is introduced? The _warned variable was used,
>     IIUC, to
>      >     avoid hitting recursive errors during error reporting.
>      >
>      >     OH!  The recursive case hadn’t occurred to me!
>      >
>      >     Poorly named and poorly commented variable from before the
>     beginning
>      >     of the mercurial age,
>      >     so hard to track down the changeset that introduced it to see if
>      >     that shed some light.  Good
>      >     thing we have some institutional memory :)
>      >
>      >     I will do something about that, and then try to figure out how to
>      >     trigger it.
>      >
>      >
>      > You could rig up something using -XX:ErrorHandlerTest
>      > (VMError::controlled_crash) and allocate from the same ResourceArea
>      > again inside the error handler. There are tests for similar
>     things, e.g.
>      > SafeFetch, see hotspot/jtreg/runtime/ErrorReporting.
>      >
>      > But I am not sure this is worthwhile. IMHO it would be better to
>     fix the
>      > places where ResourceAreas are used in error reporting instead of
>     muting
>      > this warning for recursive calls. Because ideally we do not want
>      > allocations to happen at all during error reporting.
> 
>     I think that is impractical given all the information we try to
>     print as
>     part of the error report. I expect a lot of it - e.g register and
>     memory
>     contents - to require ResourceMarks (e.g. to print class names).
> 
>     David
>     -----
> 
> 
> All those cases feed from the current Thread's ResourceArea, no? So we 
> could in theory just place a single ResourceMark at the beginning of 
> VMError::report() to avoid all those errors (only if we have a current 
> Thread, of course)?

But if the current thread has encountered a missing RM and aborts, the 
error code will be trying to use a RM in a thread where the RMs are 
already broken. Will that work or will it just abort again? And there 
may still be nested RMs in the code called from the error code.

This is getting a long way away from just fixing up some constructor issues!

And with that I'm off on vacation - sorry. :)

Cheers,
David
-----


> Thomas