Re: Fix proposal: JDK-8219378 NPE in ReflectionFactory.newMethodAccessor when langReflectAccess not initialized
Thanks for merging Roger Andrew Leonard Java Runtimes Development IBM Hursley IBM United Kingdom Ltd Phone internal: 245913, external: 01962 815913 internet email: andrew_m_leon...@uk.ibm.com From: Roger Riggs To: Andrew Leonard , Mandy Chung Cc: core-libs-dev@openjdk.java.net Date: 26/02/2019 17:00 Subject:Re: Fix proposal: JDK-8219378 NPE in ReflectionFactory.newMethodAccessor when langReflectAccess not initialized Hi Andrew, I've got the changeset you sent earlier and have tested it as well. I'll push it today unless there any objections. Webrev: http://cr.openjdk.java.net/~rriggs/webrev-reflectionfactory-8219378-2/ Roger On 02/26/2019 11: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 new change to refactor the clinit location... As I am not a "Committer" (yet! working on it!) I can't use the Submit repo. I have built your patch and run a bunch of tests locally and it works great. I have also tested it successfully with J9. So I am happy with your change. If Roger & yourself are happy with 8219378, can we Submit-repo it for final test and get that one merged? Many thanks Andrew Andrew Leonard Java Runtimes Development 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: JDK-8219378 NPE in ReflectionFactory.newMethodAccessor when langReflectAccess not initialized 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 access non-public members in java.lang.reflect. > I can try moving the clinit > code from Modifier to AccessibleObject, but I question is there some > reason it is there? Would we be sure in moving it we are not missing > something? ReflectionFactory is the internal support for reflection. The methods that access LangReflectAccess shared secrets should have a Method, Field or Constructor in hand. The ReflectionFactory::newField and newMethod that don't take Field/Method parameter are unused (I suspect they were used by the VM native reflection implementation previously. That led me to suggest to move setLangReflectAccess to AccessibleObject. AccessibleObject is initialized very early during startup by the VM. My proposed fix would change the list of classes loaded during early startup but it would need to look at closely. Having a second thought, my proposed fix can be a follow-on clean up. I'm okay with your point fix that resolves JDK-8219378. I will file a JBS issue for the follow-on clean up. What do you think? thanks Mandy Unless stated otherwise above: IBM United Kingdom Limited - Registered in England and Wales with number 741598. Registered office: PO Box 41, North Harbour, Portsmouth, Hampshire PO6 3AU Unless stated otherwise above: IBM United Kingdom Limited - Registered in England and Wales with number 741598. Registered office: PO Box 41, North Harbour, Portsmouth, Hampshire PO6 3AU
Re: Fix proposal: JDK-8219378 NPE in ReflectionFactory.newMethodAccessor when langReflectAccess not initialized
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 new change to refactor the clinit location... As I am not a "Committer" (yet! working on it!) I can't use the Submit repo. I have built your patch and run a bunch of tests locally and it works great. I have also tested it successfully with J9. So I am happy with your change. If Roger & yourself are happy with 8219378, can we Submit-repo it for final test and get that one merged? Many thanks Andrew
Re: Fix proposal: JDK-8219378 NPE in ReflectionFactory.newMethodAccessor when langReflectAccess not initialized
On 2/26/19 8:56 AM, Roger Riggs wrote: Hi Andrew, I've got the changeset you sent earlier and have tested it as well. I'll push it today unless there any objections. Webrev: http://cr.openjdk.java.net/~rriggs/webrev-reflectionfactory-8219378-2/ +1 Mandy Roger On 02/26/2019 11: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 new change to refactor the clinit location... As I am not a "Committer" (yet! working on it!) I can't use the Submit repo. I have built your patch and run a bunch of tests locally and it works great. I have also tested it successfully with J9. So I am happy with your change. If Roger & yourself are happy with 8219378, can we Submit-repo it for final test and get that one merged? Many thanks Andrew Andrew Leonard Java Runtimes Development 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: JDK-8219378 NPE in ReflectionFactory.newMethodAccessor when langReflectAccess not initialized 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 access non-public members in java.lang.reflect. > I can try moving the clinit > code from Modifier to AccessibleObject, but I question is there some > reason it is there? Would we be sure in moving it we are not missing > something? ReflectionFactory is the internal support for reflection. The methods that access LangReflectAccess shared secrets should have a Method, Field or Constructor in hand. The ReflectionFactory::newField and newMethod that don't take Field/Method parameter are unused (I suspect they were used by the VM native reflection implementation previously. That led me to suggest to move setLangReflectAccess to AccessibleObject. AccessibleObject is initialized very early during startup by the VM. My proposed fix would change the list of classes loaded during early startup but it would need to look at closely. Having a second thought, my proposed fix can be a follow-on clean up. I'm okay with your point fix that resolves JDK-8219378. I will file a JBS issue for the follow-on clean up. What do you think? thanks Mandy Unless stated otherwise above: IBM United Kingdom Limited - Registered in England and Wales with number 741598. Registered office: PO Box 41, North Harbour, Portsmouth, Hampshire PO6 3AU
Re: Fix proposal: JDK-8219378 NPE in ReflectionFactory.newMethodAccessor when langReflectAccess not initialized
Hi Andrew, I've got the changeset you sent earlier and have tested it as well. I'll push it today unless there any objections. Webrev: http://cr.openjdk.java.net/~rriggs/webrev-reflectionfactory-8219378-2/ Roger On 02/26/2019 11: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 new change to refactor the clinit location... As I am not a "Committer" (yet! working on it!) I can't use the Submit repo. I have built your patch and run a bunch of tests locally and it works great. I have also tested it successfully with J9. So I am happy with your change. If Roger & yourself are happy with 8219378, can we Submit-repo it for final test and get that one merged? Many thanks Andrew Andrew Leonard Java Runtimes Development 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: JDK-8219378 NPE in ReflectionFactory.newMethodAccessor when langReflectAccess not initialized 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 access non-public members in java.lang.reflect. > I can try moving the clinit > code from Modifier to AccessibleObject, but I question is there some > reason it is there? Would we be sure in moving it we are not missing > something? ReflectionFactory is the internal support for reflection. The methods that access LangReflectAccess shared secrets should have a Method, Field or Constructor in hand. The ReflectionFactory::newField and newMethod that don't take Field/Method parameter are unused (I suspect they were used by the VM native reflection implementation previously. That led me to suggest to move setLangReflectAccess to AccessibleObject. AccessibleObject is initialized very early during startup by the VM. My proposed fix would change the list of classes loaded during early startup but it would need to look at closely. Having a second thought, my proposed fix can be a follow-on clean up. I'm okay with your point fix that resolves JDK-8219378. I will file a JBS issue for the follow-on clean up. What do you think? thanks Mandy Unless stated otherwise above: IBM United Kingdom Limited - Registered in England and Wales with number 741598. Registered office: PO Box 41, North Harbour, Portsmouth, Hampshire PO6 3AU
Re: Fix proposal: JDK-8219378 NPE in ReflectionFactory.newMethodAccessor when langReflectAccess not initialized
Hi Mandy, I think your last proposal sounds a good plan, push 8219378 to fix the immediate issue and create a new change to refactor the clinit location... As I am not a "Committer" (yet! working on it!) I can't use the Submit repo. I have built your patch and run a bunch of tests locally and it works great. I have also tested it successfully with J9. So I am happy with your change. If Roger & yourself are happy with 8219378, can we Submit-repo it for final test and get that one merged? Many thanks Andrew Andrew Leonard Java Runtimes Development 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: JDK-8219378 NPE in ReflectionFactory.newMethodAccessor when langReflectAccess not initialized 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 access non-public members in java.lang.reflect. > I can try moving the clinit > code from Modifier to AccessibleObject, but I question is there some > reason it is there? Would we be sure in moving it we are not missing > something? ReflectionFactory is the internal support for reflection. The methods that access LangReflectAccess shared secrets should have a Method, Field or Constructor in hand. The ReflectionFactory::newField and newMethod that don't take Field/Method parameter are unused (I suspect they were used by the VM native reflection implementation previously. That led me to suggest to move setLangReflectAccess to AccessibleObject. AccessibleObject is initialized very early during startup by the VM. My proposed fix would change the list of classes loaded during early startup but it would need to look at closely. Having a second thought, my proposed fix can be a follow-on clean up. I'm okay with your point fix that resolves JDK-8219378. I will file a JBS issue for the follow-on clean up. What do you think? thanks Mandy Unless stated otherwise above: IBM United Kingdom Limited - Registered in England and Wales with number 741598. Registered office: PO Box 41, North Harbour, Portsmouth, Hampshire PO6 3AU
Re: Fix proposal: JDK-8219378 NPE in ReflectionFactory.newMethodAccessor when langReflectAccess not initialized
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 access non-public members in java.lang.reflect. I can try moving the clinit code from Modifier to AccessibleObject, but I question is there some reason it is there? Would we be sure in moving it we are not missing something? ReflectionFactory is the internal support for reflection. The methods that access LangReflectAccess shared secrets should have a Method, Field or Constructor in hand. The ReflectionFactory::newField and newMethod that don't take Field/Method parameter are unused (I suspect they were used by the VM native reflection implementation previously. That led me to suggest to move setLangReflectAccess to AccessibleObject. AccessibleObject is initialized very early during startup by the VM. My proposed fix would change the list of classes loaded during early startup but it would need to look at closely. Having a second thought, my proposed fix can be a follow-on clean up. I'm okay with your point fix that resolves JDK-8219378. I will file a JBS issue for the follow-on clean up. What do you think? thanks Mandy
Re: Fix proposal: JDK-8219378 NPE in ReflectionFactory.newMethodAccessor when langReflectAccess not initialized
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 to shake out any issue. Can you
run jdk-submit?
Mandy
On 2/25/19 7:00 AM, Andrew Leonard wrote:
Hi Mandy,
I can confirm "just" moving the clinit code into AccessibleObject does
resolve the J9 issue as well, without changing ReflectionFactory...
What's your thoughts?
Thanks
Andrew
--- old/src/java.base/share/classes/java/lang/reflect/AccessibleObject.java
2019-02-25 11:21:24.0 -0800
+++ new/src/java.base/share/classes/java/lang/reflect/AccessibleObject.java
2019-02-25 11:21:23.0 -0800
@@ -75,6 +75,16 @@
*/
public class AccessibleObject implements AnnotatedElement {
+/*
+ * Bootstrapping protocol between java.lang and java.lang.reflect
+ * packages
+ */
+static {
+ReflectionFactory factory = AccessController.doPrivileged(
+new ReflectionFactory.GetReflectionFactoryAction());
+factory.setLangReflectAccess(new java.lang.reflect.ReflectAccess());
+}
+
static void checkPermission() {
SecurityManager sm = System.getSecurityManager();
if (sm != null) {
--- old/src/java.base/share/classes/java/lang/reflect/Modifier.java
2019-02-25 11:21:25.0 -0800
+++ new/src/java.base/share/classes/java/lang/reflect/Modifier.java
2019-02-25 11:21:24.0 -0800
@@ -47,16 +47,6 @@
*/
public class Modifier {
-/*
- * Bootstrapping protocol between java.lang and java.lang.reflect
- * packages
- */
-static {
-ReflectionFactory factory = AccessController.doPrivileged(
-new ReflectionFactory.GetReflectionFactoryAction());
-factory.setLangReflectAccess(new java.lang.reflect.ReflectAccess());
-}
-
/**
* Return {@code true} if the integer argument includes the
* {@code public} modifier, {@code false} otherwise.
--- old/src/java.base/share/classes/jdk/internal/reflect/ReflectionFactory.java
2019-02-25 11:21:26.0 -0800
+++ new/src/java.base/share/classes/jdk/internal/reflect/ReflectionFactory.java
2019-02-25 11:21:25.0 -0800
@@ -726,13 +726,7 @@
}
private static LangReflectAccess langReflectAccess() {
-if (langReflectAccess == null) {
-// Call a static method to get class java.lang.reflect.Modifier
-// initialized. Its static initializer will cause
-// setLangReflectAccess() to be called from the context of the
-// java.lang.reflect package.
-Modifier.isPublic(Modifier.PUBLIC);
-}
+assert langReflectAccess != null;
return langReflectAccess;
}
Re: Fix proposal: JDK-8219378 NPE in ReflectionFactory.newMethodAccessor when langReflectAccess not initialized
Hi Mandy, I can confirm "just" moving the clinit code into AccessibleObject does resolve the J9 issue as well, without changing ReflectionFactory... What's your thoughts? Thanks Andrew Andrew Leonard Java Runtimes Development 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 , Roger Riggs Cc: core-libs-dev@openjdk.java.net Date: 22/02/2019 18:17 Subject: Re: Fix proposal: JDK-8219378 NPE in ReflectionFactory.newMethodAccessor when langReflectAccess not initialized 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 isn't true since the modifiers field is an int. While the fix looks okay, would you mind trying a different fix moving Modifier:: to AccessibleObject? See this resolves the issue when running on J9? Mandy On 2/22/19 12:49 AM, Andrew Leonard wrote: > Hi Roger, > I had a think about this and a testcase will be difficult, as it was found > during OpenJ9 testing and occured during VM bootstrap, feel free to read > further details here: > https://urldefense.proofpoint.com/v2/url?u=https-3A__github.com_eclipse_openj9_issues_3399-23issuecomment-2D459004840=DwICaQ=jf_iaSHvJObTbx-siA1ZOg=NaV8Iy8Ld-vjpXZFDdTbgGlRTghGHnwM75wUPd5_NUQ=a83hTiGAixSJXbKXDa5xzF-Z_N-iYoFk9QcOrQ3Zgiw=nI8avX9tV8iOOqNvlUPGi6avSq5-h2fXeHvlzjFHeBY= > So the issue was discovered due to the bootstrap behaviour, it was not > observed with Hotspot, however given the obvious missing initialization > check logic in the class it's not to say there's an untested route with > Hotspot that could hit it... I'm not sure how I could scaffold a jtreg > test to replicate the same? > Thanks > Andrew Unless stated otherwise above: IBM United Kingdom Limited - Registered in England and Wales with number 741598. Registered office: PO Box 41, North Harbour, Portsmouth, Hampshire PO6 3AU
Re: Fix proposal: JDK-8219378 NPE in ReflectionFactory.newMethodAccessor when langReflectAccess not initialized
Hi Alan, The J9 tests are run against build "images". Cheers Andrew Andrew Leonard Java Runtimes Development IBM Hursley IBM United Kingdom Ltd Phone internal: 245913, external: 01962 815913 internet email: andrew_m_leon...@uk.ibm.com From: Alan Bateman To: Andrew Leonard , Roger Riggs Cc: core-libs-dev@openjdk.java.net Date: 22/02/2019 14:56 Subject: Re: Fix proposal: JDK-8219378 NPE in ReflectionFactory.newMethodAccessor when langReflectAccess not initialized On 22/02/2019 08:49, Andrew Leonard wrote: > Hi Roger, > I had a think about this and a testcase will be difficult, as it was found > during OpenJ9 testing and occured during VM bootstrap, Can you say if the J9 failure was with an exploded build or an images build? Just asking as suggest we could trigger this with an exploded build during startup with the right test. If it's too much work then no problem, I agree with Roger that the issue is obvious. -Alan Unless stated otherwise above: IBM United Kingdom Limited - Registered in England and Wales with number 741598. Registered office: PO Box 41, North Harbour, Portsmouth, Hampshire PO6 3AU
Re: Fix proposal: JDK-8219378 NPE in ReflectionFactory.newMethodAccessor when langReflectAccess not initialized
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". I can try moving the clinit code from Modifier to AccessibleObject, but I question is there some reason it is there? Would we be sure in moving it we are not missing something? Thanks Andrew Andrew Leonard Java Runtimes Development 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 , Roger Riggs Cc: core-libs-dev@openjdk.java.net Date: 22/02/2019 18:17 Subject: Re: Fix proposal: JDK-8219378 NPE in ReflectionFactory.newMethodAccessor when langReflectAccess not initialized 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 isn't true since the modifiers field is an int. While the fix looks okay, would you mind trying a different fix moving Modifier:: to AccessibleObject? See this resolves the issue when running on J9? Mandy On 2/22/19 12:49 AM, Andrew Leonard wrote: > Hi Roger, > I had a think about this and a testcase will be difficult, as it was found > during OpenJ9 testing and occured during VM bootstrap, feel free to read > further details here: > https://urldefense.proofpoint.com/v2/url?u=https-3A__github.com_eclipse_openj9_issues_3399-23issuecomment-2D459004840=DwICaQ=jf_iaSHvJObTbx-siA1ZOg=NaV8Iy8Ld-vjpXZFDdTbgGlRTghGHnwM75wUPd5_NUQ=a83hTiGAixSJXbKXDa5xzF-Z_N-iYoFk9QcOrQ3Zgiw=nI8avX9tV8iOOqNvlUPGi6avSq5-h2fXeHvlzjFHeBY= > So the issue was discovered due to the bootstrap behaviour, it was not > observed with Hotspot, however given the obvious missing initialization > check logic in the class it's not to say there's an untested route with > Hotspot that could hit it... I'm not sure how I could scaffold a jtreg > test to replicate the same? > Thanks > Andrew Unless stated otherwise above: IBM United Kingdom Limited - Registered in England and Wales with number 741598. Registered office: PO Box 41, North Harbour, Portsmouth, Hampshire PO6 3AU
Re: Fix proposal: JDK-8219378 NPE in ReflectionFactory.newMethodAccessor when langReflectAccess not initialized
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 isn't true since the modifiers field is an int. While the fix looks okay, would you mind trying a different fix moving Modifier:: to AccessibleObject? See this resolves the issue when running on J9? Mandy On 2/22/19 12:49 AM, Andrew Leonard wrote: Hi Roger, I had a think about this and a testcase will be difficult, as it was found during OpenJ9 testing and occured during VM bootstrap, feel free to read further details here: https://github.com/eclipse/openj9/issues/3399#issuecomment-459004840 So the issue was discovered due to the bootstrap behaviour, it was not observed with Hotspot, however given the obvious missing initialization check logic in the class it's not to say there's an untested route with Hotspot that could hit it... I'm not sure how I could scaffold a jtreg test to replicate the same? Thanks Andrew
Re: Fix proposal: JDK-8219378 NPE in ReflectionFactory.newMethodAccessor when langReflectAccess not initialized
On 22/02/2019 08:49, Andrew Leonard wrote: Hi Roger, I had a think about this and a testcase will be difficult, as it was found during OpenJ9 testing and occured during VM bootstrap, Can you say if the J9 failure was with an exploded build or an images build? Just asking as suggest we could trigger this with an exploded build during startup with the right test. If it's too much work then no problem, I agree with Roger that the issue is obvious. -Alan
Re: Fix proposal: JDK-8219378 NPE in ReflectionFactory.newMethodAccessor when langReflectAccess not initialized
Hi Andrew, Thanks for the references and consideration. I agree it will be hard to reproduce. It is an obvious oversight and is localized to a single file so I don't think a test case is necessary. Thanks, Roger On 02/22/2019 03:49 AM, Andrew Leonard wrote: Hi Roger, I had a think about this and a testcase will be difficult, as it was found during OpenJ9 testing and occured during VM bootstrap, feel free to read further details here: https://github.com/eclipse/openj9/issues/3399#issuecomment-459004840 So the issue was discovered due to the bootstrap behaviour, it was not observed with Hotspot, however given the obvious missing initialization check logic in the class it's not to say there's an untested route with Hotspot that could hit it... I'm not sure how I could scaffold a jtreg test to replicate the same? Thanks Andrew Andrew Leonard Java Runtimes Development IBM Hursley IBM United Kingdom Ltd Phone internal: 245913, external: 01962 815913 internet email: andrew_m_leon...@uk.ibm.com From: Roger Riggs To: core-libs-dev@openjdk.java.net, Andrew Leonard Date: 21/02/2019 18:10 Subject: Re: Fix proposal: JDK-8219378 NPE in ReflectionFactory.newMethodAccessor when langReflectAccess not initialized Hi Andrew, Is there a test case? Since the initialization is a side effect of static initialization, it might be hard to trigger just one of those paths. How was it discovered? Thanks, Roger On 02/21/2019 11:57 AM, Alan Bateman wrote: On 21/02/2019 16:49, Roger Riggs wrote: Hi Andrew, I can sponsor; it looks correct to me. Any other reviewers? It looks right but would be good to have a test case that demonstrates the issue. -Alan Unless stated otherwise above: IBM United Kingdom Limited - Registered in England and Wales with number 741598. Registered office: PO Box 41, North Harbour, Portsmouth, Hampshire PO6 3AU
Re: Fix proposal: JDK-8219378 NPE in ReflectionFactory.newMethodAccessor when langReflectAccess not initialized
Hi Roger, I had a think about this and a testcase will be difficult, as it was found during OpenJ9 testing and occured during VM bootstrap, feel free to read further details here: https://github.com/eclipse/openj9/issues/3399#issuecomment-459004840 So the issue was discovered due to the bootstrap behaviour, it was not observed with Hotspot, however given the obvious missing initialization check logic in the class it's not to say there's an untested route with Hotspot that could hit it... I'm not sure how I could scaffold a jtreg test to replicate the same? Thanks Andrew Andrew Leonard Java Runtimes Development IBM Hursley IBM United Kingdom Ltd Phone internal: 245913, external: 01962 815913 internet email: andrew_m_leon...@uk.ibm.com From: Roger Riggs To: core-libs-dev@openjdk.java.net, Andrew Leonard Date: 21/02/2019 18:10 Subject:Re: Fix proposal: JDK-8219378 NPE in ReflectionFactory.newMethodAccessor when langReflectAccess not initialized Hi Andrew, Is there a test case? Since the initialization is a side effect of static initialization, it might be hard to trigger just one of those paths. How was it discovered? Thanks, Roger On 02/21/2019 11:57 AM, Alan Bateman wrote: On 21/02/2019 16:49, Roger Riggs wrote: Hi Andrew, I can sponsor; it looks correct to me. Any other reviewers? It looks right but would be good to have a test case that demonstrates the issue. -Alan Unless stated otherwise above: IBM United Kingdom Limited - Registered in England and Wales with number 741598. Registered office: PO Box 41, North Harbour, Portsmouth, Hampshire PO6 3AU
Re: Fix proposal: JDK-8219378 NPE in ReflectionFactory.newMethodAccessor when langReflectAccess not initialized
Hi Andrew, Is there a test case? Since the initialization is a side effect of static initialization, it might be hard to trigger just one of those paths. How was it discovered? Thanks, Roger On 02/21/2019 11:57 AM, Alan Bateman wrote: On 21/02/2019 16:49, Roger Riggs wrote: Hi Andrew, I can sponsor; it looks correct to me. Any other reviewers? It looks right but would be good to have a test case that demonstrates the issue. -Alan
Re: Fix proposal: JDK-8219378 NPE in ReflectionFactory.newMethodAccessor when langReflectAccess not initialized
On 21/02/2019 16:49, Roger Riggs wrote: Hi Andrew, I can sponsor; it looks correct to me. Any other reviewers? It looks right but would be good to have a test case that demonstrates the issue. -Alan
Re: Fix proposal: JDK-8219378 NPE in ReflectionFactory.newMethodAccessor when langReflectAccess not initialized
Hi Andrew, I can sponsor; it looks correct to me. Any other reviewers? Thanks, Roger On 02/19/2019 11:24 AM, Andrew Leonard wrote: Please can I request a sponsor for this fix to ReflectionFactory where langReflectAccess is not always correctly initialized. Webrev: http://cr.openjdk.java.net/~aleonard/8219378/webrev.00/ Bug: https://bugs.openjdk.java.net/browse/JDK-8219378 Thanks Andrew Andrew Leonard Java Runtimes Development IBM Hursley IBM United Kingdom Ltd Phone internal: 245913, external: 01962 815913 internet email: andrew_m_leon...@uk.ibm.com Unless stated otherwise above: IBM United Kingdom Limited - Registered in England and Wales with number 741598. Registered office: PO Box 41, North Harbour, Portsmouth, Hampshire PO6 3AU
Fix proposal: JDK-8219378 NPE in ReflectionFactory.newMethodAccessor when langReflectAccess not initialized
Please can I request a sponsor for this fix to ReflectionFactory where langReflectAccess is not always correctly initialized. Webrev: http://cr.openjdk.java.net/~aleonard/8219378/webrev.00/ Bug: https://bugs.openjdk.java.net/browse/JDK-8219378 Thanks Andrew Andrew Leonard Java Runtimes Development IBM Hursley IBM United Kingdom Ltd Phone internal: 245913, external: 01962 815913 internet email: andrew_m_leon...@uk.ibm.com Unless stated otherwise above: IBM United Kingdom Limited - Registered in England and Wales with number 741598. Registered office: PO Box 41, North Harbour, Portsmouth, Hampshire PO6 3AU
