RFR: 8214552: Resolve clash between 4947890 and 8130266
Roger, I just pushed 8130266 which had the CSR approved a couple of days ago only to find that it clashed badly with the change you made in 4947890 to reduce the cost of system property initialisation. It appears that the new code there is not happy if there is no value for the system property. I think java.awt.printerjob just needs to be removed from the new code : If this looks right to you I need to push ASAP : Bug: https://bugs.openjdk.java.net/browse/JDK-8214552 Webrev: http://cr.openjdk.java.net/~prr/8214552/ Separately I think the SystemProps code ought to consider whether it ought to error out the launcher if a property is not defined .. -phil.
On 11/30/18 11:43 AM, Phil Race wrote:
Roger,
I just pushed 8130266 which had the CSR approved a couple of days ago only to find that it clashed badly with the change you made in 4947890 to reduce the cost of system property initialisation. It appears that the new code there is not happy if there is no value for the system property.
Is JDK-8214322 the JBS issue but not JDK-8130266?
I think java.awt.printerjob just needs to be removed from the new code :
If this looks right to you I need to push ASAP :
Bug: https://bugs.openjdk.java.net/browse/JDK-8214552 Webrev: http://cr.openjdk.java.net/~prr/8214552/
src/java.base/share/native/libjava/System.c also needs to be updated. Mandy
Separately I think the SystemProps code ought to consider whether it ought to error out the launcher if a property is not defined ..
-phil.
Sorry, but its actually 8130264 that I meant https://bugs.openjdk.java.net/browse/JDK-8130264 -phil. On 11/30/18 11:54 AM, Mandy Chung wrote:
On 11/30/18 11:43 AM, Phil Race wrote:
Roger,
I just pushed 8130266 which had the CSR approved a couple of days ago only to find that it clashed badly with the change you made in 4947890 to reduce the cost of system property initialisation. It appears that the new code there is not happy if there is no value for the system property.
Is JDK-8214322 the JBS issue but not JDK-8130266?
I think java.awt.printerjob just needs to be removed from the new code :
If this looks right to you I need to push ASAP :
Bug: https://bugs.openjdk.java.net/browse/JDK-8214552 Webrev: http://cr.openjdk.java.net/~prr/8214552/
src/java.base/share/native/libjava/System.c also needs to be updated.
Mandy
Separately I think the SystemProps code ought to consider whether it ought to error out the launcher if a property is not defined ..
-phil.
On 11/30/18 11:56 AM, Phil Race wrote:
Sorry, but its actually 8130264 that I meant
I see it now. System.c has been updated. Your patch looks good.
Separately I think the SystemProps code ought to consider whether it ought to error out the launcher if a property is not defined ..
If not set, it should not set the system property. Mandy
Hi Phil, The patch looks fine. Where does it die? SystemProps should skip null values in putIfAbsent(). Sorry for the turbulence. Roger On 11/30/2018 02:43 PM, Phil Race wrote:
Roger,
I just pushed 8130266 which had the CSR approved a couple of days ago only to find that it clashed badly with the change you made in 4947890 to reduce the cost of system property initialisation. It appears that the new code there is not happy if there is no value for the system property.
I think java.awt.printerjob just needs to be removed from the new code :
If this looks right to you I need to push ASAP :
Bug: https://bugs.openjdk.java.net/browse/JDK-8214552 Webrev: http://cr.openjdk.java.net/~prr/8214552/
Separately I think the SystemProps code ought to consider whether it ought to error out the launcher if a property is not defined ..
-phil.
On 11/30/18 11:54 AM, Roger Riggs wrote:
Hi Phil,
The patch looks fine.
Ok, thanks.
Where does it die? SystemProps should skip null values in putIfAbsent().
You are right it checks and that explains why I didn't see it when I tested. I saw the NPE only after I noticed the code and it seems was just from my debugging I quickly added since System.out is likely not defined at this point ... Anyway we still need this fix. Thanks, -phil.
Sorry for the turbulence.
Roger
On 11/30/2018 02:43 PM, Phil Race wrote:
Roger,
I just pushed 8130266 which had the CSR approved a couple of days ago only to find that it clashed badly with the change you made in 4947890 to reduce the cost of system property initialisation. It appears that the new code there is not happy if there is no value for the system property.
I think java.awt.printerjob just needs to be removed from the new code :
If this looks right to you I need to push ASAP :
Bug: https://bugs.openjdk.java.net/browse/JDK-8214552 Webrev: http://cr.openjdk.java.net/~prr/8214552/
Separately I think the SystemProps code ought to consider whether it ought to error out the launcher if a property is not defined ..
-phil.
participants (3)
-
Mandy Chung
-
Phil Race
-
Roger Riggs