Hi,
While this is in memory: I'd like to request review for an 8u backport.
The changeset imports cleanly into jdk8u/hs-dev/hotspot
Thanks
Kevin
On 04/03/2015 15:17, Staffan Larsen wrote:
Looks good!
Thanks,
/Staffan
On 4 mar 2015, at 15:14, Kevin Walls <kevin.wa...@oracle.com> wrote:
Sure, I updated the webrev:
http://cr.openjdk.java.net/~kevinw/8073688/webrev.01/
Thanks
Kevin
On 04/03/2015 14:05, Staffan Larsen wrote:
On 4 mar 2015, at 14:07, Kevin Walls <kevin.wa...@oracle.com> wrote:
Hi Staffan --
staticness: I could have argued that either way, it doesn't need to be static.
A static count of how many HotSpotTypeDataBase duplicate type errors we have
seen, or one count per HotSpotTypeDataBase? Any doubt I will make it per
HotSpotTypeDatabase. As I sometimes load multiple cores in kjdb, or load them
multiple times, I should be encouraging less staticness in the SA overall: so
yes let me remove that static! (updated webrev in same location)
Thanks.
Well spotted on the structs loop: 8-)
readVMTypes, readVMntConstants, readVMLongConstants may increase, or call
something could increase, the counter.
readVMStructs does not.
These are four methods that are so similar you'd imagine they should get merged
into one method one day. It seemed wrong to me to make the four methods yet
more different, by taking out the check on duplicateDefCount for one of them.
Also, maybe readVMStructs should be checking for duplicates anyway, and maybe
one day it will. I'm guessing it doesn't because it was harder in that method
compared to the others. 8-)
So I put it there for regularity, but if you think it's misleading I'll take it
out.
Yes, please take it out.
Thanks,
/Staffan
In practice, as long as we have the duplicate check in readVMTypes(), we have
the protection we need as this is called first. It is highly unlikely that
would succeed and another of the methods would fail, the other checks are just
making the code consistent.
Thanks
Kevin
On 04/03/2015 12:29, Staffan Larsen wrote:
On 4 mar 2015, at 12:29, Kevin Walls <kevin.wa...@oracle.com> wrote:
Thanks Dmitry -
I will explain the counter's presence where we introduce it:
// Counter to ensure read loops terminate:
private static final int MAX_DUPLICATE_DEFINITIONS = 100;
private static int duplicateDefCount = 0;
Should duplicateDefCount really be static?
For the use of duplicateDefCount on line 477 (readVMStructs()) I do not see
where it is increased inside the loop? Can you clarify?
/Staffan
Thanks
Kevin
On 03/03/2015 20:10, Dmitry Samersoff wrote:
Kevin,
The fix looks good for me, but please add a comment explaining the
problem we are solving here.
-Dmitry
On 2015-03-03 16:15, Kevin Walls wrote:
Hi,
This is a review request for a way to make the SA tools protect
themselves from infinite loops during initialisation.
Attaching jmap (for example) to a JVM can fail, infinitely writing an
error - and filling a disk if being logged to a file. This reproduces
on a Solaris package based install, but not with other distribution
bundles. In those packages, there's a link JDK/jre/lib/sparc/libjvm ->
client/libjvm.so. If a server/libjvm.so is loaded and running, we see
libverify.so pull in client/libjvm.so, as it finds the symlink in its
$ORIGIN, in preference to finding the already loaded libjvm.so.
Symbol lookup in the SA is fooled by having multiple libjvm.so loaded.
There are various things wrong with that. Protection against zero
strides through the type arrays and a maximum count for duplicated types
will protect us from a few possible problems.
I'll also work a bug for the "install" repo where we create that
symlink, but the tools need to protect themselves from this kind of
problem.
Testing was manual.
bug
https://bugs.openjdk.java.net/browse/JDK-8073688
webrev
http://cr.openjdk.java.net/~kevinw/8073688/webrev.00/
Thanks
Kevin
