Coleen,
It looks good.
Good catch by David H.
Thanks,
Serguei
On 5/16/16 09:28, Coleen Phillimore wrote:
David, Thank you for looking at this change.
On 5/15/16 9:51 PM, David Holmes wrote:
Hi Coleen,
On 13/05/2016 4:14 AM, Coleen Phillimore wrote:
8151066: assert(0 <= i && i < length()) failed: index out of bounds
Summary: lock classes for redefinition because constant pool merging
isn't thread safe, use method constant pool because constant pool
merging doesn't make equivalent cpCaches because of invokedynamic
I had to make these changes together. See bugs for more details.
open webrev at http://cr.openjdk.java.net/~coleenp/8155951/webrev
bug link https://bugs.openjdk.java.net/browse/JDK-8155951
bug link https://bugs.openjdk.java.net/browse/JDK-8151066
In general the locking approach seems okay - as long as all mutating
actions on the constantpool are being guarded with this locking.
But I'm concerned that you are using a bit in _misc_flags for this
mutable state. It isn't clear to me when the other bits in
_misc_flags might be set but I'm assuming only during initial class
parsing, or some other single-threaded use? Putting a mutable bit
into such a flag set is fragile and can only be done once.
I was uneasy about this too. Most of the _misc_flags are set during
initialization but there are a couple that are set while the vm is
running. I moved _is_being_redefined to a bool sized gap just about
the _misc_flags, which makes me more confident of the state of this
field when doing locking.
Retesting this small change with StressRedefine in a loop.
open webrev at http://cr.openjdk.java.net/~coleenp/8155951.02/webrev
Thanks,
Coleen
Thanks,
David
Tested with rbt nightly test and 24 hours with StressRedefine test.
Thanks,
Coleen
