Re: RFR of JDK-8187376: test issue in java/lang/invoke/VarHandles/VarHandleBaseTest.java

2017-09-12 Thread huaming . li
On 13/09/2017 3:09 AM, Paul Sandoz wrote: On 10 Sep 2017, at 20:20, huaming...@oracle.com wrote: Would you please review the below patch? bug: https://bugs.openjdk.java.net/browse/JDK-8187376 webrev: http://cr.openjdk.java.net/~mli/8187376/webrev.00/ Looks good. I dunno what planet i was

Re: [10] Review Request: 8187442 Xcheck:jni produces various "WARNING in native method" in launcher

2017-09-12 Thread David Holmes
On 13/09/2017 9:42 AM, Sergey Bylokhov wrote: On 9/12/17 16:03, David Holmes wrote: call. That is fine, but still leaves the problem that you are skipping the DeleteLocalRef call. The leak was there before: if we will get an exception at "1512 SetByteArrayRegion()" then we never call this

Re: [10] Review Request: 8187442 Xcheck:jni produces various "WARNING in native method" in launcher

2017-09-12 Thread Sergey Bylokhov
On 9/12/17 16:03, David Holmes wrote: call. That is fine, but still leaves the problem that you are skipping the DeleteLocalRef call. The leak was there before: if we will get an exception at "1512 SetByteArrayRegion()" then we never call this DeleteLocalRef(). I assumed it was done

Re: [10] Review Request: 8187442 Xcheck:jni produces various "WARNING in native method" in launcher

2017-09-12 Thread David Holmes
On 13/09/2017 7:58 AM, Sergey Bylokhov wrote: On 9/12/17 14:28, David Holmes wrote: But in case of CallStaticObjectMethod() we cannot check for NULL which can be a valid result, so I added a check for exception and return 0. My point is that the check is completely redundant. If an exception

Re: [10] Review Request: 8187442 Xcheck:jni produces various "WARNING in native method" in launcher

2017-09-12 Thread Sergey Bylokhov
On 9/12/17 14:28, David Holmes wrote: But in case of CallStaticObjectMethod() we cannot check for NULL which can be a valid result, so I added a check for exception and return 0. My point is that the check is completely redundant. If an exception occurred then the return value is already

Re: [10] Review Request: 8187442 Xcheck:jni produces various "WARNING in native method" in launcher

2017-09-12 Thread David Holmes
Hi Sergey, On 13/09/2017 7:22 AM, Sergey Bylokhov wrote: Hi, David. Thank you for review, here are my thoughts. The patch uses the same pattern as NULL_CHECK0. If an exception is occurred then NULL_CHECK0 in one situation and CHECK_EXCEPTION_RETURN_VALUE in another situation will return 0 to

Re: [10] Review Request: 8187442 Xcheck:jni produces various "WARNING in native method" in launcher

2017-09-12 Thread Sergey Bylokhov
Hi, David. Thank you for review, here are my thoughts. The patch uses the same pattern as NULL_CHECK0. If an exception is occurred then NULL_CHECK0 in one situation and CHECK_EXCEPTION_RETURN_VALUE in another situation will return 0 to the upper code which will process this value(there is the

Re: [10] Review Request: 8187442 Xcheck:jni produces various "WARNING in native method" in launcher

2017-09-12 Thread David Holmes
Hi Sergey, On 13/09/2017 5:18 AM, Sergey Bylokhov wrote: Hello, Please review the fix for jdk10/jdk10. Bug: https://bugs.openjdk.java.net/browse/JDK-8187442 Webrev can be found at: http://cr.openjdk.java.net/~serb/8187442/webrev.00 This doesn't look right to me: str =

[10] Review Request: 8187442 Xcheck:jni produces various "WARNING in native method" in launcher

2017-09-12 Thread Sergey Bylokhov
Hello, Please review the fix for jdk10/jdk10. Bug: https://bugs.openjdk.java.net/browse/JDK-8187442 Webrev can be found at: http://cr.openjdk.java.net/~serb/8187442/webrev.00 The simple application with empty main method produce some "warnings" when Xcheck:jni is used. Because of that it is

Re: RFR of JDK-8187376: test issue in java/lang/invoke/VarHandles/VarHandleBaseTest.java

2017-09-12 Thread Paul Sandoz
> On 10 Sep 2017, at 20:20, huaming...@oracle.com wrote: > > Would you please review the below patch? > > bug: https://bugs.openjdk.java.net/browse/JDK-8187376 > > webrev: http://cr.openjdk.java.net/~mli/8187376/webrev.00/ > Looks good. I dunno what planet i was on when i wrote that code.

Re: [10] RFR(M) 8182701: Modify JVMCI to allow Graal Compiler to expose platform MBean

2017-09-12 Thread mandy chung
On 9/12/17 10:44 AM, Jaroslav Tulach wrote: Dear reviewers, after several reconsiderations I have webrev #4 ready for your review. Can you please take a look at http://cr.openjdk.java.net/~jtulach/8182701/webrev.04/ and let me know if it is in a reasonable shape? Thanks a lot. -jt Yes

Re: [10] RFR(M) 8182701: Modify JVMCI to allow Graal Compiler to expose platform MBean

2017-09-12 Thread Jaroslav Tulach
Dear reviewers, after several reconsiderations I have webrev #4 ready for your review. Can you please take a look at http://cr.openjdk.java.net/~jtulach/8182701/webrev.04/ and let me know if it is in a reasonable shape? Thanks a lot. -jt

Re: RFR[10]:8159526 Deprivilege jdk.httpserver

2017-09-12 Thread mandy chung
This patch looks fine.  It's fine with me to follow up the second phase to identify the permissions needed rather than granting AllPermissions. Mandy On 9/12/17 1:06 AM, vyom tewari wrote: Hi, Please review the below code change. BugId: https://bugs.openjdk.java.net/browse/JDK-8159526

Re: RFR[10]:8159526 Deprivilege jdk.httpserver

2017-09-12 Thread Sean Mullan
On 9/12/17 4:06 AM, vyom tewari wrote: Hi, Please review the below code change. BugId: https://bugs.openjdk.java.net/browse/JDK-8159526 Webrev-1: http://cr.openjdk.java.net/~vtewari/8159526/jdk/webrev/index.html Can you put the entry for jdk.httpserver after jdk.dynalink so you maintain

Re: RFR[10]:8159526 Deprivilege jdk.httpserver

2017-09-12 Thread Chris Hegarty
> On 12 Sep 2017, at 09:46, vyom tewari wrote: > > On Tuesday 12 September 2017 02:12 PM, Alan Bateman wrote: >> On 12/09/2017 09:06, vyom tewari wrote: >>> Hi, >>> >>> Please review the below code change. >>> >>> BugId: https://bugs.openjdk.java.net/browse/JDK-8159526

Re: RFR[10]:8159526 Deprivilege jdk.httpserver

2017-09-12 Thread Michael McMahon
Looks good Vyom. - Michael On 12/09/2017, 09:46, vyom tewari wrote: On Tuesday 12 September 2017 02:12 PM, Alan Bateman wrote: On 12/09/2017 09:06, vyom tewari wrote: Hi, Please review the below code change. BugId: https://bugs.openjdk.java.net/browse/JDK-8159526 Webrev-1:

Re: RFR[10]:8159526 Deprivilege jdk.httpserver

2017-09-12 Thread vyom tewari
On Tuesday 12 September 2017 02:12 PM, Alan Bateman wrote: On 12/09/2017 09:06, vyom tewari wrote: Hi, Please review the below code change. BugId: https://bugs.openjdk.java.net/browse/JDK-8159526 Webrev-1: http://cr.openjdk.java.net/~vtewari/8159526/jdk/webrev/index.html Webrev-2:

Re: RFR[10]:8159526 Deprivilege jdk.httpserver

2017-09-12 Thread Alan Bateman
On 12/09/2017 09:06, vyom tewari wrote: Hi, Please review the below code change. BugId: https://bugs.openjdk.java.net/browse/JDK-8159526 Webrev-1: http://cr.openjdk.java.net/~vtewari/8159526/jdk/webrev/index.html Webrev-2: http://cr.openjdk.java.net/~vtewari/8159526/root/webrev/index.html

RFR[10]:8159526 Deprivilege jdk.httpserver

2017-09-12 Thread vyom tewari
Hi, Please review the below code change. BugId: https://bugs.openjdk.java.net/browse/JDK-8159526 Webrev-1: http://cr.openjdk.java.net/~vtewari/8159526/jdk/webrev/index.html Webrev-2: http://cr.openjdk.java.net/~vtewari/8159526/root/webrev/index.html Code change will De-privilege