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)
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.
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
