.
Thanks,
David
-
On 20/07/2019 2:20 am, Mandy Chung wrote:
This was a follow up to the observation during the code review
of JDK-82193798.
Webrev:
http://cr.openjdk.java.net/~mchung/jdk14/8219774/webrev.01/
In the current implementation, Modifier:: provides a hook
to initialize the static
(Coming back to this patch and ready to push this change later today)
Here is the updated webrev:
http://cr.openjdk.java.net/~mchung/jdk14/8173978/webrev.02/
This includes the change that Peter suggested accepting m2 == null.
Mandy
On 7/5/19 12:29 PM, Peter Levart wrote:
Hi Mandy,
Yes,
On 7/20/19 8:08 AM, Alan Bateman wrote:
On 19/07/2019 17:20, Mandy Chung wrote:
This was a follow up to the observation during the code review
of JDK-82193798.
Webrev:
http://cr.openjdk.java.net/~mchung/jdk14/8219774/webrev.01/
This cleanup looks good to me. A minor nit
Hi Leo,
Thanks for adding the copyright and license header. The patch looks
okay assuming you will separate the legal notices from the existing
comment block in JFC properties files.
Mandy
On 7/22/19 9:23 AM, li.ji...@oracle.com wrote:
Hi all,
Please review this change.
We found the
On 7/19/19 10:13 AM, Severin Gehwolf wrote:
Hi Mandy,
On Fri, 2019-07-19 at 09:58 -0700, Mandy Chung wrote:
Hi Severin,
On 7/19/19 9:55 AM, Severin Gehwolf wrote:
There might be other tests with policy files where this is not the case.
My issue is with finding those tests :-/ If we know
Hi Severin,
On 7/19/19 9:55 AM, Severin Gehwolf wrote:
There might be other tests with policy files where this is not the case.
My issue is with finding those tests :-/ If we know the set of *all*
tests affected by the breakage we could do approach 2. Approach 1 (or
3) seems safer.
Severin -
On 7/19/19 7:14 AM, Alan Bateman wrote:
On 19/07/2019 15:00, Severin Gehwolf wrote:
:
Sure. By adding a method also accepting a default value it would work
as well. If that's preferred, I can change it:
diff --git a/test/lib/jdk/test/lib/Platform.java
This was a follow up to the observation during the code review
of JDK-82193798.
Webrev:
http://cr.openjdk.java.net/~mchung/jdk14/8219774/webrev.01/
In the current implementation, Modifier:: provides a hook
to initialize the static ReflectionFactory::langReflectAccess field
lazily. This was
JDK-8219774 is relevant to this patch (this was discussed in the code
review for JDK-8219378: NPE in ReflectionFactory.newMethodAccessor
when langReflectAccess not initialized).
This cleans up the initialization of LangReflectAccess:
http://cr.openjdk.java.net/~mchung/jdk14/8219774/webrev.00/
On 7/17/19 3:25 AM, Claes Redestad wrote:
Hi Peter,
On 2019-07-17 10:29, Peter Levart wrote:
Hi Claes,
Am I reading correct that package-private
ClassLoader.loadLibrary(Class fromClass, String name, boolean
isAbsolute) wouldn't need to consult SecurityManager wasn't it for
accessing
On 7/16/19 6:48 AM, Claes Redestad wrote:
Hi,
refactored to use a BootLoader::loadLibrary API that is only visible
within the java.base module:
http://cr.openjdk.java.net/~redestad/8227587/open.03/
Looks good.
Nit: in JavaLangAccess
321 void loadLibrary(Class klass, String
and wrap the call with doPriv; otherwise, just call the shared
secret loadLibrary method.
Then we can investigate in the future to refactor the native library
loading implementation to jdk.internal.loader.
what do you think?
Mandy
On 7/12/19 8:25 AM, Mandy Chung wrote:
Claes,
Thanks for exploring
Claes,
Thanks for exploring this. I would like to see if we can avoid introducing
an internal @CS method (keep @CSM only for public APIs will help security
analysis).
There are other alternatives to improve the footprint performance.
One idea is java.base and java.desktop each has its own
On 7/5/19 8:24 AM, Peter Levart wrote:
Hi Mandy,
On 7/2/19 7:20 PM, Mandy Chung wrote:
Webrev updated:
http://cr.openjdk.java.net/~mchung/jdk14/8173978/webrev.01/
I just skimmed across code and I think there's a little optimization
possible in VerifyAccess:
:
...instead of seting
On 7/2/19 9:54 PM, Kasper Nielsen wrote:
On Tue, 2 Jul 2019 at 18:50, Mandy Chung wrote:
I'm not getting how getCallerClass is used and related to access check.
Can you elaborate?
Caller sensitive methods are viral, in the sense that if you invoke a caller
sensitive method in the JDK
+1
Mandy
On 7/4/19 2:13 AM, Thomas Stüfe wrote:
Greetings,
please review this trivial fix which switches
off jdk/java/lang/reflect/exeCallerAccessTest on AIX.
JBS: https://bugs.openjdk.java.net/browse/JDK-8227252
cr:
On 7/2/19 6:57 PM, Ralph Goers wrote:
Thanks Mandy,
It seems I commented on the thread mentioned in the issue you linked
to. Unfortunately, it doesn’t look like any work has been done on the
issue.in the last 18 months.
I did start to explore some options and never have time to spend
On 7/2/19 11:10 PM, David Holmes wrote:
Hi Mandy,
Thanks for taking a look.
On 3/07/2019 8:57 am, Mandy Chung wrote:
On 7/2/19 3:44 PM, David Holmes wrote:
webrev: http://cr.openjdk.java.net/~dholmes/8227055/webrev/
bug: https://bugs.openjdk.java.net/browse/JDK-8227055
The launcher help
On 7/2/19 10:30 PM, Thomas Stüfe wrote:
Hi Mandy,
On Wed, Jul 3, 2019, 00:20 Mandy Chung <mailto:mandy.ch...@oracle.com>> wrote:
Hi Thomas,
This is AIX-only. It would be cleaner to move AIX-specific
launcher
to a new file like main_aix.c. Have you considered
, 2019, at 10:48 AM, Mandy Chung <mailto:mandy.ch...@oracle.com>> wrote:
MethodHandles::lookup is optimized (@ForceInline) and so it may not
represent apple-to-apple comparison.StackWalker::getCallerClass
does have overhead compared to Reflection::getCallerClass and
need to get the micro
On 7/2/19 3:44 PM, David Holmes wrote:
webrev: http://cr.openjdk.java.net/~dholmes/8227055/webrev/
bug: https://bugs.openjdk.java.net/browse/JDK-8227055
The launcher help text needs some minor updates/corrections
- -verbose needs more info
- -Xdebug needs to say it does nothing
Should this
Hi Thomas,
This is AIX-only. It would be cleaner to move AIX-specific launcher
to a new file like main_aix.c. Have you considered that?
I don't know how to specify additional .c file in
make/test/JtregNativeJdk.gmk though.
Mandy
On 7/1/19 10:23 PM, Thomas Stüfe wrote:
Second,
MethodHandles::lookup is optimized (@ForceInline) and so it may not
represent apple-to-apple comparison.StackWalker::getCallerClass
does have overhead compared to Reflection::getCallerClass and
need to get the microbenchmark in the jdk repo and rerun the numbers [1].
I'm not getting how
On 7/2/19 6:59 AM, Alan Bateman wrote:
This is really good work and fixes several issues left over from JDK
9. The compatibility issues (mostly small/advanced) are unfortunate
but necessary and I hope it won't impact too many things. It will need
a detail release note once the CSR is done
BTW: is the right header? I thought was reserved for the
class name. John Gibbon will know more ;-)
It should be . Updated. This was written prior to that change.
thanks
Mandy
best regards,
-- daniel
On 02/07/2019 03:47, Mandy Chung wrote:
On 7/1/19 12:51 PM, Mandy Chung wrote:
This is a
On 7/1/19 12:51 PM, Mandy Chung wrote:
This is an enhancement to |`Lookup::in`| and
|`MethodHandles::privateLookupIn`| API
for cross module teleporting. A `Lookup` object will record the previous
lookup class from which this |Lookup| object was teleported such that
the access check will use
This is an enhancement to |`Lookup::in`| and
|`MethodHandles::privateLookupIn`| API
for cross module teleporting. A `Lookup` object will record the previous
lookup class from which this |Lookup| object was teleported such that
the access check will use both the previous lookup class and the
Hi Vicente,
This looks fine.
Nit: the new test line 71 and 80 have an extra space "Error (" that can
be taken out.
Mandy
On 6/24/19 8:33 PM, Vicente Romero wrote:
Please review fix for [1], at [2]. The implementation of method
MethodTypeDesc.resolveConstantDes is not in sync with its
in which I had doubt about using default
policy. I kept them on problem list.
Other tests, as I understand, manipulate permissions for test classes.
They don't extend system classes so default policy should not affect
test class permissions.
Thanks,
Vladimir
On 6/19/19 11:23 PM, Mandy Chung
could also be fixed in these tests by inspecting the CodeSource
of the ProtectionDomain. Or better yet, they should just use the jtreg
java.security.policy option and a policy file and avoid the need to
create a Policy implementation.
Thanks,
Sean
On 6/20/19 11:04 AM, Mandy Chung wrote:
The Polic
On 6/20/19 10:35 AM, Alan Bateman wrote:
On 20/06/2019 17:50, yumin qi wrote:
Hi, Alan and Ioi
Thanks. Forget to add core-libs-dev for the review.
If add a public API, surely it should be discussed in detail the
design, implementation and effects. One question, will adding a
public API
,
If this were java.base, we would use doPrivilege to ignore the policy
and place specific limits.
Encumbering the default policy with conditions needed by a trusted
subsystem seems
like distributing what should be a local implementation issue.
$.02, Roger
On 6/20/19 2:23 AM, Mandy Chung wrote:
Hi
Hi Vladimir,
Indeed these are test issues that the tests will need to grant permissions
to jdk.internal.vm.compiler as the default policy grants.
Thanks for going extra miles to fix the tests.
My suggestion may be a bit general. What I intend to say the custom
security policy should extend
Looks good. Thanks for fixing it.
Mandy
On 6/12/19 1:35 PM, Joe Darcy wrote:
Hello,
Please review the small patch below to address
JDK-8225675: Outdated citation of JLS in java.lang.ref.Reference
(I'll reflow the paragraph before pushing; wanted to make the nature
of the diff clearer
Reviewed, both the patch and CSR.
Mandy
On 6/5/19 4:07 PM, mark.reinh...@oracle.com wrote:
Bug: https://bugs.openjdk.java.net/browse/JDK-8197927
CSR: https://bugs.openjdk.java.net/browse/JDK-8225381
Revise the specification of the `java.lang.System::getProperties` method
to allow the values
rev:
http://cr.openjdk.java.net/~mikael/webrevs/8225305/webrev.01/open/webrev/
Cheers,
Mikael
On Jun 4, 2019, at 3:18 PM, Mandy Chung <mailto:mandy.ch...@oracle.com>> wrote:
I agree with Igor. The best is to keep running these tests except
the AOT, perhaps + fastdebug, run only.
Mandy
I agree with Igor. The best is to keep running these tests except
the AOT, perhaps + fastdebug, run only.
Mandy
On 6/4/19 2:06 PM, Igor Ignatyev wrote:
Hi Mikael,
as it looks like 8222445 isn't going to be fixed for a long time (as it's
"targeted" to tbd), and the defect seems to affect
Alan, Sundar,
Thanks for the review.
I further clean up the test and rename jdk.test.Main2 class to a new
test2 module. No change to the fix.
http://cr.openjdk.java.net/~mchung/jdk13/webrevs/8221368/webrev.01/
Mandy
On 6/4/19 12:10 AM, Alan Bateman wrote:
On 04/06/2019 05:00, Mandy Chung
:02 pm, Mandy Chung wrote:
test/jdk/java/lang/reflect/PublicMethods/PublicMethodsTest.java time out
in certain configuration e.g. fastdebug -Xcomp. Setting the empty class
path significantly improves the execution time as it eliminates opening
and scanning of the JAR files on the class path. Alan
The launcher prints out the error message when the main class fails to
initialize but java.launcher.module.error5 resource that intends to
print the Caused-by exception message when running in a named module
but prints an incorrect parameter.
Very simple fix - fix the resource param indices.
test/jdk/java/lang/reflect/PublicMethods/PublicMethodsTest.java time out
in certain configuration e.g. fastdebug -Xcomp. Setting the empty class
path significantly improves the execution time as it eliminates opening
and scanning of the JAR files on the class path. Alan has also experimented
it
Looks fine.
Mandy
On 5/31/19 10:37 AM, Jonathan Gibbons wrote:
Please review another round of fixes for HTML issues, this time in
java.naming.
As with the management APIs, there were some inconsistencies in the
ranks for the headings, which have been addressed. The log with the
simplified
+1
Mandy
On 5/30/19 6:07 PM, Jonathan Gibbons wrote:
Please review a simple docs fix for jdk.zipfs module-info.java.
The ranks of the headings are updated to close up the gaps, and a
couple of superfluous are removed.
Webrev link below, but the patch is small enough to read inline if you
On 5/30/19 8:55 AM, Peter Levart wrote:
Yes Roger, this sounds better to me.
Maybe even easier:
"There is no guarantee that this effort will recycle any particular
number of unused objects, reclaim any particular amount of space, or
complete before the method returns (or ever)."
+1 and
On 5/30/19 3:25 AM, Roger Riggs wrote:
Hi,
ok, thanks for the comments.
Any other comments on:
"* Runs the garbage collector in the Java Virtual Machine.
*
* Calling this method suggests that the Java Virtual Machine
* expend effort toward recycling unused objects in order to
* make the
On 4/13/19 3:29 AM, Ivan Gerasimov wrote:
Hello!
This is request to review the combined backport for the four listed
bugs (the later three are the test fixes).
The first fix had to be slightly modified:
1) The spec change was removed from AccessibleObject.java,
2) The method
The patch looks good (the return statement is missing). Thanks for
fixing it.
Mandy
P.S. Sorry for the belated review. Good to see this pushed. I'm OOO.
On 4/8/19 9:23 PM, Aleksey Shipilev wrote:
On 4/8/19 3:08 PM, David Holmes wrote:
+1
Thanks, I am going to push this under triviality
/04/2019 9:37 am, Mandy Chung wrote:
A simple test fix. The test causes the build failure.
Thanks
Mandy
diff --git
a/test/jdk/java/lang/reflect/exeCallerAccessTest/exeCallerAccessTest.c
b/test/jdk/java/lang/reflect/exeCallerAccessTest/exeCallerAccessTest.c
---
a/test/jdk/java/lang/reflect
A simple test fix. The test causes the build failure.
Thanks
Mandy
diff --git
a/test/jdk/java/lang/reflect/exeCallerAccessTest/exeCallerAccessTest.c
b/test/jdk/java/lang/reflect/exeCallerAccessTest/exeCallerAccessTest.c
---
On 4/1/19 11:40 PM, Amy Lu wrote:
Please review the patch to fix a fake passed test:
jdk/internal/ref/Cleaner/ExitOnThrow.java
It expects the target test program exists with '1', and throws an
exception. Due to the target test program be wrongly forked (missed
--add-exports ), it exists
On 4/1/19 5:22 PM, Lance Andersen wrote:
A follow-up on this.
I ran this test 300+ times without failure on the internal Mach 5
machines including the one that it failed on (which was only 4 times
since January). This one system would run the test in approximately
6-7 minutes on average
On 3/28/19 1:39 PM, Alan Bateman wrote:
On 28/03/2019 16:43, Mandy Chung wrote:
:
Updated webrev:
http://cr.openjdk.java.net/~mchung/jdk13/webrevs/8221530/webrev.01
I think this looks okay. One minor nit is that
newIllegalAccessException doesn't throw IAE.
Thanks Alan.
CSR: https
On 3/27/19 7:18 AM, Lindenmaier, Goetz wrote:
, is this example very clear to them that it won't be supported?
You probably meant that for
n().getNull().m()
it is not printed that getNull() was called on the result of n()?
Yes, I caught my error after I sent my example.
This is a
On 3/28/19 7:48 AM, Peter Levart wrote:
Hi,
On 3/28/19 9:40 AM, Alan Bateman wrote:
On 27/03/2019 23:17, Mandy Chung wrote:
:
The proposed fix is to perform proper access check. When there is no
caller frame, it only allows to access to public members of a public
type
On 3/28/19 12:19 AM, Alan Bateman wrote:
On 28/03/2019 00:23, Mandy Chung wrote:
On 3/27/19 4:56 PM, Lance Andersen wrote:
Hi Mandy,
On Mar 27, 2019, at 7:23 PM, Mandy Chung <mailto:mandy.ch...@oracle.com>> wrote:
Hi Lance,
Do you understand what takes so long for this te
On 3/28/19 8:46 AM, Peter Levart wrote:
On 3/28/19 4:08 PM, Alan Bateman wrote:
On 28/03/2019 14:48, Peter Levart wrote:
:
In addition, if access from null caller is granted and it is
performed to a member in a "concealed" package, there's no warning
displayed
The proposed check is
On 3/28/19 1:40 AM, Alan Bateman wrote:
On 27/03/2019 23:17, Mandy Chung wrote:
:
The proposed fix is to perform proper access check. When there is no
caller frame, it only allows to access to public members of a public
type
in an unconditional exported API package.
The approach seems
On 3/27/19 4:56 PM, Lance Andersen wrote:
Hi Mandy,
On Mar 27, 2019, at 7:23 PM, Mandy Chung <mailto:mandy.ch...@oracle.com>> wrote:
Hi Lance,
Do you understand what takes so long for this test to run?
Well it is executing a lot of jar commands. I did not see anything
that s
Hi Lance,
Do you understand what takes so long for this test to run?
Is it running fastdebug and -Xcomp or other flag?
Mandy
On 3/27/19 1:55 PM, Lance Andersen wrote:
Hi all ,
Please review this fix for https://bugs.openjdk.java.net/browse/JDK-8216539
which increases the timeout value for
This is to fix a regression introduced in JDK 12 by JDK-8206240.
This impacts only native application that creates JVM via JNI
and also invokes Field::getField (or other reflection API) via
JNI that triggers reflection access check against the caller class.
The regression happens only when there
This is a new version of the patch:
http://cr.openjdk.java.net/~mchung/jdk13/webrevs/8220282/webrev.01
I made further clean up to this new test.
It extends MethodHandlesTest.HasFields class to include the test
cases for instance final fields. HasFields is used to test
On 3/26/19 10:24 AM, Adam Farley8 wrote:
Hiya Mandy,
I've updated the webrev with your proposal, plus a few tweaks for
formatting.
http://cr.openjdk.java.net/~afarley/8216558.2/webrev
Let me know what you think.
This looks okay. I took the liberty to take out the comment line
On 3/26/19 4:57 AM, Lindenmaier, Goetz wrote:
and also the cases when it requires to look at its caller frame.
Never. Only the method that was executed when the exception is
thrown is needed. I only mention the backtrace because it happens to
store the top method and the corresponding
On 3/25/19 11:36 AM, Adam Farley8 wrote:
Addendum: URL for new webrev:
http://cr.openjdk.java.net/~afarley/8216558.1/webrev/
Thanks for sending a versioned webrev.
> What is a USA test?
UNREFLECT_SETTER_ACCESSIBLE.
I was trying to be brief, and I lost readability.
Will re-upload with
Hi Goetz,
On 3/15/19 3:55 AM, Lindenmaier, Goetz wrote:
I followed your advice and created a JEP:
https://bugs.openjdk.java.net/browse/JDK-8220715
This is a good start. I include my comments as a reader who does not
read TrackingStackCreator and other C++ code.
In the "Basic algorithm to
Hi Adam,
On 3/22/19 8:40 AM, Joe Darcy wrote:
Please update distinct versions of a webrev (e.g. distinguished with
.1, .2 directory names) rather than overwriting a single one. This
make it easier for those coming to the review thread later to see the
evolution of the changes over time.
217 //If this is a USA test, then only the fraction of the expected
failures will occur; those which are both static and final. 218 if (fl
!= FieldLookup.MH_UNREFLECT_SETTER_ACCESSIBLE &&
actualFieldNames.stream().anyMatch(s->!(s.contains("static")&&(s.contains("final")
What is a USA test?
On 3/20/19 9:03 PM, Dan Smith wrote:
http://cr.openjdk.java.net/~dlsmith/8174222/webrev.00/
AbstractValidatingLambdaMetafactory.java
+ throw new LambdaConversionException("implementation is not direct or
cannot be cracked"); It may help to print implementation method handle:
throw new
On 3/20/19 1:54 AM, Lindenmaier, Goetz wrote:
Should I move the JEP to status 'submitted'?
Per the Process states section [1]
Draft --- In circulation by the author for initial review and
consensus-building
I certainly don't want to be the bottleneck. Others can help do the
initial
Hi Brent,
The change looks fine.
Can you file a JBS issue to follow up a place to document this
JDK-specific property?
thanks
Mandy
On 3/20/19 2:01 PM, Brent Christian wrote:
Hi,
JDK-8216401[1][2] added code to improve JAR spec conformance (WRT
relative URLs in the Class-Path attribute),
I looked through the webrev and this looks fine to me.
Mandy
On 3/20/19 2:50 PM, Jonathan Gibbons wrote:
Please review a medium-sized but conceptually simple patch to fix most
of the headings in the generated documentation for java.base, as
described in [1]. This does not address the headings
Hi Adam,
I imported your patch but there is one test failure:
test/jdk/java/lang/invoke/VarHandles/accessibility/TestFieldLookupAccessibility.java
This test needs update of this change. Can you please send an updated
patch and run all test/jdk/java/lang/invoke tests.
Mandy
On 3/6/19 8:28
Hi Goetz,
Sorry I have been busy on other time-critical things. I will
get to it hopefully next week.
Mandy
On 3/15/19 3:55 AM, Lindenmaier, Goetz wrote:
Hi everybody, Mark,
I followed your advice and created a JEP:
https://bugs.openjdk.java.net/browse/JDK-8220715
Please point me to things
On 3/14/19 10:41 AM, Brent Christian wrote:
On 3/13/19 6:08 PM, Martin Buchholz wrote:
Why not Reference.reachabilityFence ?
You mean the mechanism for this precise situation. Yeah, OK.
http://cr.openjdk.java.net/~bchristi/8220088/webrev.01/
The use of Reference.reachabilityFence is
.. Sorry, I missed the "reply all" on my first reply.
-Original Message-
From: Maurizio Cimadamore
Sent: Freitag, 15. März 2019 12:33
To: Lindenmaier, Goetz ; Mandy Chung
; Roger Riggs
Cc: Java Core Libs ; hotspot-runtime-
d...@openjdk.java.net
Subject: Re: RFR(L): 8218
Hi Goetz,
Roger, Coleen, Maurizio and I talked about this proposed feature.
We all think that improving NPE message is a useful enhancement for
the platform and helps developers to tell what causes NPE.
This is not a small enhancement. Diving into a large code review
would not be the best way
On 3/13/19 1:29 PM, Kim Barrett wrote:
On Mar 13, 2019, at 4:07 PM, Kim Barrett wrote:
Please review this change to the JNI specification. The specified
behavior of Weak Global References, and in particular their
relationship to Java PhantomReference, is being updated to account
for a
to be created for
other releases if this fix gets backported?
Which release are you thinking to backport to?
IMO I don't think this fix is critical for existing releases
and this fix has behavioral change.
Mandy
Best Regards
Adam Farley
IBM Runtimes
Mandy Chung wrote on 07/03/2019 23:17:15
The updated patch looks good.
Mandy
On 3/11/19 9:29 AM, Joe Darcy wrote:
Hello,
Always surprising how much discussion an (apparently) simple refactoring
can generate!
Thanks to Tagir for spotting this issue.
For completeness, the two-argument forms of requireNonNull which takes a
On 3/8/19 12:35 PM, Martin Buchholz wrote:
On Fri, Mar 8, 2019 at 3:57 AM Tagir Valeev wrote:
Hello!
diff -r 274361bd6915 src/java.base/share/classes/java/lang/Throwable.java
--- a/src/java.base/share/classes/java/lang/Throwable.javaThu Mar 07
10:22:19 2019 +0100
+++
Looks good to me.
Mandy
On 3/8/19 3:08 AM, Joe Darcy wrote:
Hello,
The code in java.lang.Throwable has various explicit null checks that
could be rewritten to use Objects.requireNonNull.
Please review the patch below which implements this refactoring.
Thanks,
-Joe
diff -r 274361bd6915
On 3/7/19 3:17 PM, Mandy Chung wrote:
Lastly, I'm sorry to see you weren't happy with the quality of my test
code. Your changes seem much larger in scale, and quite different to
my changes. Could you explain the benefits of your approach, vs simply
adding non-static final fields to my code
On 3/7/19 9:44 AM, Adam Farley8 wrote:
Hi Mandy,
Since you have created a new work item to cover the test work, do let
me know if you want the test code removed from the webrev for the
original issue.
You push what you have. Your fix should come with a regression test.
Please make sure
This adds the test cases of setter on final fields to ensure no write
access to final fields with the exception on unreflectSetter on Field
of an instance final field whose accessible flag is set once
JDK-8216558 is resolved.
Webrev at:
Hi Adam,
On 3/6/19 8:28 AM, Adam Farley8 wrote:
Hi Mandy,
The webrev has been updated with the new test:
http://cr.openjdk.java.net/~afarley/8216558/webrev/
Looks okay although I think the test adds isFinal check for the new test
case is meant to be say static final field.
Note that I
ards
Adam Farley
IBM Runtimes
Mandy Chung wrote on 21/02/2019 17:37:54:
From: Mandy Chung
To: Adam Farley8
Cc: core-libs-dev
Date: 21/02/2019 17:41
Subject: Re: RFR: JDK-8216558: Lookup.unreflectSetter(Field) fails
to throw IllegalAccessException for final fields
Hi Adam,
On 2/14/19 3:1
+1
Mandy
On 2/27/19 12:11 PM, Joe Darcy wrote:
Hello,
Please review the patch below to fix a small typo ("at" -> "as") in text
of Throwable introduced with some of the Project Coin changes.
Thanks,
-Joe
diff -r 2c50e900e8af src/java.base/share/classes/java/lang/Throwable.java
---
Hi Andrew,
Thanks for verifying the suggested patch. I created
https://bugs.openjdk.java.net/browse/JDK-8219774 to follow up this.
Mandy
On 2/26/19 8:46 AM, Andrew Leonard wrote:
Hi Mandy,
I think your last proposal sounds a good plan, push 8219378 to fix the
immediate issue and create a
nt
IBM Hursley
IBM United Kingdom Ltd
Phone internal: 245913, external: 01962 815913
internet email: andrew_m_leon...@uk.ibm.com
From: Mandy Chung
To: Andrew Leonard
Cc: core-libs-dev@openjdk.java.net, Roger Riggs
Date: 26/02/2019 00:16
Subject: Re: Fix proposal: J
On 2/25/19 5:12 AM, Andrew Leonard wrote:
Hi Mandy,
I must admit I don't completely follow the logic of the existing
Modifier init of langReflectAccess, the comment indicates a "protocol
between java.lang and java.lang.reflect".
That sets up the shared secret for ReflectionFactory to
Hi Andrew,
I think initializing LangReflectAccess in AccessibleObject
is a better fix. Besides moving clinit to AccessibleObject,
ReflectionFactory::langReflectAccess can simply assert
if langReflectAccess is non-null. Attached is the patch
that you can reference.
We should do more testing
On 2/22/19 8:37 AM, Vicente Romero wrote:
Hi Mandy,
Thanks for the review. I have uploaded a new iteration [1], please also
review the CSR at [2]
Vicente
[1] http://cr.openjdk.java.net/~vromero/8219480/webrev.01/
Looks good.
[2] https://bugs.openjdk.java.net/browse/JDK-8219587
On 2/22/19 9:50 AM, Andy Herrick wrote:
revised webrev t address issues brought up by Mandy:
[2] http://cr.openjdk.java.net/~herrick/8218055/webrev.03
I only looked at JLinkBundlerHelper.java and the removed
files that look okay. Nit: can you use "jlink" lower case
in the log/exception
Hi Andrew,
Thanks for the stack trace of the issue triggering this.
It seems to me that Modifier:: isn't the right place
to setLangReflectAccess shared secret. It might have assumed
that Modifier should have been initialized when Field/Method
or other AccessibleObject is instantiated which
On 2/22/19 6:17 AM, Andy Herrick wrote:
On 2/21/2019 8:54 PM, Mandy Chung wrote:
I only skimmed on the patch. A couple of comments:
73 () -> new RuntimeException("link tool not found"));
yes jlink should always exist in the JDK that jpackage is run from - I
On 2/21/19 4:49 PM, Andy Herrick wrote:
Please review the jpackage fix for bug [1] at [2].
This is a fix for the JDK-8200758-branch branch of the open sandbox
repository (jpackage).
This enhancement removes the use of
jdk.tools.jlink.internal.packager.AppRuntimeImageBuilder in favor of
On 2/20/19 3:10 PM, Vicente Romero wrote:
Please review the simple patch to fix [1] at [2]. The patch is simply
adding a comment to the API, (javadoc) to sync it with the implementation.
Thanks,
Vicente
[1] https://bugs.openjdk.java.net/browse/JDK-8219480
[2]
Hi Adam,
On 2/14/19 3:16 AM, Adam Farley8 wrote:
Hi Mandy,
Apologies for the delay.
Same here as I returned from vacation yesterday.
Could you review this cdiff as a proposal for the jtreg test?
Made sense to modify the existing test set for MethodHandle rather than
add a new one.
Yes
On 2/20/19 10:57 AM, Daniel Fuchs wrote:
Right - here is an updated webrev:
http://cr.openjdk.java.net/~dfuchs/webrev_8216363/webrev.02/
Looks good.
Mandy
Hi Daniel,
I return today from vacation.
On 2/15/19 10:46 AM, Daniel Fuchs wrote:
Hi Mandy,
Here is the new webrev:
http://cr.openjdk.java.net/~dfuchs/webrev_8216363/webrev.01/
Looks good.
I suggest to make the javadoc clear that isLoggable accepts null
@param record a {@code
1001 - 1100 of 3165 matches
Mail list logo
