Markus, Thank you for refactoring.
Looks good for me! -Dmitry On 2014-01-30 17:52, Markus Gronlund wrote: > Thanks guys. > > Yes, the reason I added the return in 37 is to avoid anyone adding anything > after the else clause moving forward. However, as Dmitry pointed out, an > inversion of the version check will make this clearer - thanks. > > In addition, just realized I hadn't pulled in the latest changes for the last > webrev (for the diff), there were actually recent updates to this file as of: > > changeset: 9197:902aa2b3265c > user: simonis > date: Mon Jan 20 17:16:05 2014 +0100 > summary: 8031581: PPC64: Addons and fixes for AIX to pass the jdk > regression tests > > > So here's an updated webrev for your convenience: > > http://cr.openjdk.java.net/~mgronlun/8032518/webrev02/ > > Dmitry: > > With the pull, the locations of "free, throw, return" reduces to only two so > I don't think refactoring here will add much. Thanks for pointing it out > though. > > Cheers > Markus > > > -----Original Message----- > From: David Holmes > Sent: den 30 januari 2014 13:06 > To: Markus Gronlund; serviceability-dev@openjdk.java.net > serviceability-dev@openjdk.java.net; hotspot-runtime-dev > Subject: Re: RFR(XXS): 8032518: fatal error has been detected by the Java > Runtime Environment (access violation) > > This is good to me. The return at line 37 was unnecessary but it does help > reinforce the pattern that you must return after JNU_ThrowXXX > > David > > On 30/01/2014 9:11 PM, Markus Gronlund wrote: >> Greetings, >> >> Kindly asking for reviews for this very small fix: >> >> Bug: https://bugs.openjdk.java.net/browse/JDK-8032518 >> >> Webrev: http://cr.openjdk.java.net/~mgronlun/8032518/webrev01/ >> >> Background: >> >> Still a bit puzzled about the manifestations of the crashes when >> inspecting the .mdmp files, which seems to be dereferencing a (debug) >> ResourceObj allocation[0] cookie address (~allocation address) at >> point of crashing. In addition, if the issue is an effect of not >> handling OOM correctly, I would expect to see a _pending_exception off >> the problematic thread, but there seems to be none. Also unknown why >> this seems to occur more on Windows x64 than any other platform. >> >> Testing: >> >> I have iterated the testcase nsk/stress/jck60/jck60014 locally - >> without suggested fixes I get about 10 crashes in about 300 runs. With >> fixes I am yet to see any crashes, currently ~600 iterations. >> >> I suggest to putback this first (since it should be fixed anyhow), to >> see the effect, before any more time is spent on tracing this down. >> >> Thanks >> >> Markus >> -- Dmitry Samersoff Oracle Java development team, Saint Petersburg, Russia * I would love to change the world, but they won't give me the sources.
