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
