Re: RFR: JDK-8204994: SA might fail to attach to process with "Windbg Error: WaitForEvent failed"

2020-05-28 Thread serguei.spit...@oracle.com
Hi Alex, It'd be nice to reduce noise from such intermittent issues and also get rid of such bugs in the future. My gut feeling is that we just significantly reduced a probability of this failure in something like an order of magnitude but it will still happens once in a month or a half of

Re: RFR: JDK-8225056 VM support for sealed classes

2020-05-28 Thread David Holmes
Hi Harold, Sorry Mandy's comment raised a couple of issues ... On 29/05/2020 7:12 am, Mandy Chung wrote: Hi Harold, On 5/27/20 1:35 PM, Harold Seigel wrote: Incremental webrev: http://cr.openjdk.java.net/~hseigel/sealedClasses.8225056.incr.2/ full webrev:

Re: RFR: JDK-8204994: SA might fail to attach to process with "Windbg Error: WaitForEvent failed"

2020-05-28 Thread Alex Menkov
Hi Chris, On 05/28/2020 15:22, Chris Plummer wrote: Hi Alex, Overall looks good to me. I'll be real happy if I never see "WaitForEvent Failed!" again in our CI runs. Just one nit: I hope so. 402   // see JDK-8204994: sometimes WaitForEvent fails with E_ACCESSDENIED, 403   // but

Re: RFR: JDK-8204994: SA might fail to attach to process with "Windbg Error: WaitForEvent failed"

2020-05-28 Thread Alex Menkov
Hi Serguei, With my testing 2nd call always succeeded, but I was able to reproduce the case only 3 times with my test runs. I can implement the loop, but number of retries is anyway an arbitrary value. --alex On 05/28/2020 15:44, serguei.spit...@oracle.com wrote: Hi Alex, It looks good in

Re: RFR: JDK-8225056 VM support for sealed classes

2020-05-28 Thread Mandy Chung
Thanks for confirming it. Mandy On 5/28/20 3:31 PM, Harold Seigel wrote: Hi Mandy, The entries in the PermittedSubclasses attribute are constant pool ConstantClass_info entries.  These names get validated by the VM in this code in ClassFileParser::parse_constant_pool():   for (index

Re: RFR(XS): 8222005: ClassRedefinition crashes with: guarantee(false) failed: OLD and/or OBSOLETE method(s) found

2020-05-28 Thread serguei.spit...@oracle.com
Hi Coleen, The updated webrev version is:   http://cr.openjdk.java.net/~sspitsyn/webrevs/2020/jvmti-redef.3/ It has your suggestions addressed:  - remove log_is_enabled conditions  - move ResourceMark's out of loops Thanks,

Re: RFR: 8244993: Revert changes to OutputAnalyzer stderrShouldBeEmptyIgnoreVMWarnings() that allow version strings

2020-05-28 Thread Daniil Titov
Hi Chris and David, Thank you for reviewing this change. --Best regards, Daniil On 5/26/20, 4:33 PM, "Chris Plummer" wrote: Hi Daniil, Looks good. thanks, Chris On 5/26/20 10:46 AM, Daniil Titov wrote: > Hi Chris and David, > > Please review a new version

Re: RFR: JDK-8204994: SA might fail to attach to process with "Windbg Error: WaitForEvent failed"

2020-05-28 Thread serguei.spit...@oracle.com
Hi Alex, It looks good in general. +static HRESULT WaitForEvent(IDebugControl *ptrIDebugControl) { + // see JDK-8204994: sometimes WaitForEvent fails with E_ACCESSDENIED, + // but succeeded on 2nd call. + HRESULT hr =

Re: RFR: JDK-8225056 VM support for sealed classes

2020-05-28 Thread Harold Seigel
Hi Mandy, The entries in the PermittedSubclasses attribute are constant pool ConstantClass_info entries.  These names get validated by the VM in this code in ClassFileParser::parse_constant_pool():   for (index = 1; index < length; index++) {     const jbyte tag =

Re: RFR: JDK-8204994: SA might fail to attach to process with "Windbg Error: WaitForEvent failed"

2020-05-28 Thread Chris Plummer
Hi Alex, Overall looks good to me. I'll be real happy if I never see "WaitForEvent Failed!" again in our CI runs. Just one nit: 402   // see JDK-8204994: sometimes WaitForEvent fails with E_ACCESSDENIED, 403   // but succeeded on 2nd call. I think "succeeds" would be better than "succeeded".

Re: RFR(XS): 8222005: ClassRedefinition crashes with: guarantee(false) failed: OLD and/or OBSOLETE method(s) found

2020-05-28 Thread serguei.spit...@oracle.com
Hi Coleen, Thank you a lot for reviewing this! On 5/28/20 12:48, coleen.phillim...@oracle.com wrote: Hi Serguei, Sorry for the delay reviewing this again. On 5/18/20 3:30 AM, serguei.spit...@oracle.com wrote:

Re: RFR: JDK-8225056 VM support for sealed classes

2020-05-28 Thread Mandy Chung
Hi Harold, On 5/27/20 1:35 PM, Harold Seigel wrote: Incremental webrev: http://cr.openjdk.java.net/~hseigel/sealedClasses.8225056.incr.2/ full webrev: http://cr.openjdk.java.net/~hseigel/sealedClasses.8225056.2/webrev/ Class.java 4406 ReflectionData rd = reflectionData(); 4407

Re: RFR(XS): 8222005: ClassRedefinition crashes with: guarantee(false) failed: OLD and/or OBSOLETE method(s) found

2020-05-28 Thread coleen . phillimore
Hi Serguei, Sorry for the delay reviewing this again. On 5/18/20 3:30 AM, serguei.spit...@oracle.com wrote: Hi Coleen and potential reviewers, Now, the webrev: http://cr.openjdk.java.net/~sspitsyn/webrevs/2020/jvmti-redef.2/ has a complete fix for all three failure modes related to the

Re: RFR(XS): 8245126 Kitchensink fails with: assert(!method->is_old()) failed: Should not be installing old methods

2020-05-28 Thread Dean Long
Sure, you could just have cache_jvmti_state() return a boolean to bail out immediately for is_old. dl On 5/28/20 7:23 AM, serguei.spit...@oracle.com wrote: Hi Dean, Thank you for looking at this! Okay. Let me check what cab be done in this direction. There is no point to cache is_old. The

Re: RFR(S) 8238585: Use handshake for JvmtiEventControllerPrivate::enter_interp_only_mode() and don't make compiled methods on stack not_entrant

2020-05-28 Thread Vladimir Kozlov
Vladimir Ivanov is on break currently. It looks good to me. Thanks, Vladimir K On 5/26/20 7:31 AM, Reingruber, Richard wrote: Hi Vladimir, Webrev: http://cr.openjdk.java.net/~rrich/webrevs/8238585/webrev.0/ Not an expert in JVMTI code base, so can't comment on the actual changes.

Re: RFR(XS): 8245126 Kitchensink fails with: assert(!method->is_old()) failed: Should not be installing old methods

2020-05-28 Thread serguei.spit...@oracle.com
Hi Dean, Thank you for looking at this! Okay. Let me check what cab be done in this direction. There is no point to cache is_old. The compilation has to bail out if it is discovered to be true. Thanks, Serguei On

Re: RFR(XS): 8245126 Kitchensink fails with: assert(!method->is_old()) failed: Should not be installing old methods

2020-05-28 Thread Dean Long
This seems OK as long as the memory barriers in the thread state transitions prevent the C++ compiler from doing something like reading is_old before reading redefinition_count.  I would feel better if both JVMCI and C1/C2 cached is_old and redefinition_count at the same time (making sure to

Re: RFR: JDK-8225056 VM support for sealed classes

2020-05-28 Thread David Holmes
Hi Harold, On 28/05/2020 6:35 am, Harold Seigel wrote: Hi David, Please review this updated webrev: Incremental webrev: http://cr.openjdk.java.net/~hseigel/sealedClasses.8225056.incr.2/ Changes look good - thanks! One minor comment: src/hotspot/share/prims/jvm.cpp 2159 if (length !=