[RFR] 8196969: JTreg Failure: serviceability/sa/ClhsdbJstack.java causes NPE

[Severin Gehwolf]

On Wed, 2019-10-02 at 08:35 -0700, Chris Plummer wrote:
> On 10/2/19 1:57 AM, Severin Gehwolf wrote:
> > On Wed, 2019-10-02 at 10:39 +0200, Severin Gehwolf wrote:
> > > On Tue, 2019-10-01 at 12:51 -0700, Chris Plummer wrote:
> > > > Hi Severin,
> > > > 
> > > > Sorry, this is not an area that I have any expertise in. However, I did
> > > > confirm that it fixes the NPE I was seeing with JShellHeapDumpTest.java,
> > > > which brings up a question. You said this happens with -Xcomp, but I was
> > > > never using -Xcomp. Might it also be triggered without -Xcomp?
> > > Yes, as soon as C1/C2 kick in you might be seeing the issue. In fact, I
> > > had one reproducer which triggered the issue quite reliably which
> > > didn't use -Xcomp explicitly. Nevertheless, certain methods got JIT
> > > compiled soon and the issue surfaced. You should be able to verify by
> > > adding -XX:+PrintCompilation to the host program and/or comparing it to
> > > -Xint runs.
> > > 
> > > TLDR; -Xcomp helps reproducing the issue on smaller test cases, but
> > > isn't essential in the general case.
> > On second thought, I haven't investigated whether running the SA on a
> > heap dump triggers the issue as well. There might be another code path
> > not covered with this patch. If there still is, it might be a similar,
> > but slightly different bug.
> So  you are saying your fix here is for when producing the heap dump, 
> but parsing the heap dump might also have a similar issue? I think we 
> are not seeing any issue with the parsing. My new 
> HeapDumpTestWithActiveProcess.java test used to reproduce the NPE when 
> dumping the heap, and your fix has resolved that issue. The test then 
> goes on to dump all the stacks in the heap dump, so I think if there was 
> a problem we would have seen it.

Glad to hear that.

Process question: Do I need a second review for SA changes?

Thanks,
Severin