RFR: 8225715: jhsdb jmap fails to write binary heap dump of a jshell process

classic Classic list List threaded Threaded
5 messages Options
Reply | Threaded
Open this post in threaded view
|

RFR: 8225715: jhsdb jmap fails to write binary heap dump of a jshell process

Fairoz Matte
Hi,

Please review a fix that potentially handle NPE and allow heap to dump from a
process were we get source file name as null.

Background:
For taking heap dump of JShell process, we get getSourceFileName() as null.
Current implementation doesn't have null check.
This fix will handle the null check and avoid breaking in call to writeObjectID(sym).
Regression test provided with patch will demonstrate the problem.

JBS bug - https://bugs.openjdk.java.net/browse/JDK-8225715
Webrev - http://cr.openjdk.java.net/~fmatte/8225715/webrev.00/

Testing: Mach5 tier1,tier2 tier3, hs-tier1, hs-tier2 and hs-tier3 tests successfully passed

Thanks,
Fairoz

Reply | Threaded
Open this post in threaded view
|

Re: RFR: 8225715: jhsdb jmap fails to write binary heap dump of a jshell process

Chris Plummer
Looks good.

Chris

On 6/30/19 9:31 PM, Fairoz Matte wrote:

> Hi,
>
> Please review a fix that potentially handle NPE and allow heap to dump from a
> process were we get source file name as null.
>
> Background:
> For taking heap dump of JShell process, we get getSourceFileName() as null.
> Current implementation doesn't have null check.
> This fix will handle the null check and avoid breaking in call to writeObjectID(sym).
> Regression test provided with patch will demonstrate the problem.
>
> JBS bug - https://bugs.openjdk.java.net/browse/JDK-8225715
> Webrev - http://cr.openjdk.java.net/~fmatte/8225715/webrev.00/
>
> Testing: Mach5 tier1,tier2 tier3, hs-tier1, hs-tier2 and hs-tier3 tests successfully passed
>
> Thanks,
> Fairoz
>

Reply | Threaded
Open this post in threaded view
|

RE: RFR: 8225715: jhsdb jmap fails to write binary heap dump of a jshell process

Fairoz Matte
Hi Chris,

Thanks for the review.

Thanks,
Fairoz

> -----Original Message-----
> From: Chris Plummer
> Sent: Monday, July 01, 2019 11:21 PM
> To: Fairoz Matte <[hidden email]>; serviceability-
> [hidden email]
> Subject: Re: RFR: 8225715: jhsdb jmap fails to write binary heap dump of a
> jshell process
>
> Looks good.
>
> Chris
>
> On 6/30/19 9:31 PM, Fairoz Matte wrote:
> > Hi,
> >
> > Please review a fix that potentially handle NPE and allow heap to dump
> > from a process were we get source file name as null.
> >
> > Background:
> > For taking heap dump of JShell process, we get getSourceFileName() as
> null.
> > Current implementation doesn't have null check.
> > This fix will handle the null check and avoid breaking in call to
> writeObjectID(sym).
> > Regression test provided with patch will demonstrate the problem.
> >
> > JBS bug - https://bugs.openjdk.java.net/browse/JDK-8225715
> > Webrev - http://cr.openjdk.java.net/~fmatte/8225715/webrev.00/
> >
> > Testing: Mach5 tier1,tier2 tier3, hs-tier1, hs-tier2 and hs-tier3
> > tests successfully passed
> >
> > Thanks,
> > Fairoz
> >
>
Reply | Threaded
Open this post in threaded view
|

Re: RFR: 8225715: jhsdb jmap fails to write binary heap dump of a jshell process

KEVIN WALLS
In reply to this post by Fairoz Matte
Hi Fairoz -

Looks good to me too.

The " -encoding utf8" in the test might be a leftover from something
else (maybe /timeout=240 also?), you could leave it out but no problem
either way.

Thanks!
Kevin


On 01/07/2019 05:31, Fairoz Matte wrote:

> Hi,
>
> Please review a fix that potentially handle NPE and allow heap to dump from a
> process were we get source file name as null.
>
> Background:
> For taking heap dump of JShell process, we get getSourceFileName() as null.
> Current implementation doesn't have null check.
> This fix will handle the null check and avoid breaking in call to writeObjectID(sym).
> Regression test provided with patch will demonstrate the problem.
>
> JBS bug - https://bugs.openjdk.java.net/browse/JDK-8225715
> Webrev - http://cr.openjdk.java.net/~fmatte/8225715/webrev.00/
>
> Testing: Mach5 tier1,tier2 tier3, hs-tier1, hs-tier2 and hs-tier3 tests successfully passed
>
> Thanks,
> Fairoz
>
Reply | Threaded
Open this post in threaded view
|

RE: RFR: 8225715: jhsdb jmap fails to write binary heap dump of a jshell process

Fairoz Matte
Hi Kevin,

> -----Original Message-----
> From: Kevin Walls
> Sent: Tuesday, July 02, 2019 2:18 PM
> To: Fairoz Matte <[hidden email]>; serviceability-
> [hidden email]
> Subject: Re: RFR: 8225715: jhsdb jmap fails to write binary heap dump of a
> jshell process
>
> Hi Fairoz -
>
> Looks good to me too.
>
> The " -encoding utf8" in the test might be a leftover from something else
Yes I will remove that.

> (maybe /timeout=240 also?), you could leave it out but no problem either
Timeout is required for dump file to be get created.

Thanks for the review.

Thanks,
Fairoz

> way.
>
> Thanks!
> Kevin
>
>
> On 01/07/2019 05:31, Fairoz Matte wrote:
> > Hi,
> >
> > Please review a fix that potentially handle NPE and allow heap to dump
> > from a process were we get source file name as null.
> >
> > Background:
> > For taking heap dump of JShell process, we get getSourceFileName() as
> null.
> > Current implementation doesn't have null check.
> > This fix will handle the null check and avoid breaking in call to
> writeObjectID(sym).
> > Regression test provided with patch will demonstrate the problem.
> >
> > JBS bug - https://bugs.openjdk.java.net/browse/JDK-8225715
> > Webrev - http://cr.openjdk.java.net/~fmatte/8225715/webrev.00/
> >
> > Testing: Mach5 tier1,tier2 tier3, hs-tier1, hs-tier2 and hs-tier3
> > tests successfully passed
> >
> > Thanks,
> > Fairoz
> >