Re: [9] Review request for 8151189: Possible getAppContext() NPE in java.awt.Desktop and java.awt.Taskbar
On 16.11.16 0:16, Phil Race wrote:
A "main" appcontext will be created if you are a standalone app, but
not if running in webstart (although the means of determining that is
somewhat hokey) :-
if (numAppContexts.get() == 0) {
if (System.getProperty("javaplugin.version")
== null &&
System.getProperty("javawebstart.version") == null) {
initMainAppContext();
So if that is not initialised it appears to rely solely on an appcontext
being
associated with the current threadgroup - or a parent threadgroup.
If for some reason this does not return an appcontext you'll get the NPE.
This doesn't have to mean it is the Toolkit thread.
Yes, that's true that main appcontext is not created in case of applets
and webstart. And this is a feature. If we will get null appcontext here
we are in incorrect state and should not try to workaround it, but
should fail instead.
For example in the proposed fix we save Desktop to static var after
security checks, so if the user will be able to call this methods after
he will get Desktop object even if it is not have such permission.
This example may be a little contrived but it illustrates the problem :-
If the webstart system property is set you will never see "got desktop"
printed
because the finalizer thread gets an exception.
===
import java.awt.Desktop;
public class GD {
public void finalize() {
System.out.println("get desktop");
System.out.println(Desktop.getDesktop());
System.out.println("got desktop");
}
public static void main(String[] args) {
if (args.length == 0) {
System.out.println("Setting webstart version");
System.setProperty("javawebstart.version", "8");
}
while (true) {
new GD();
System.gc();
}
}
}
-phil.
On 11/15/2016 10:48 AM, Phil Race wrote:
So are you saying we will never call this from the Toolkit thread, so
provably
there will never be an NPE ?
Seems we have had a ton of NPE bugs from getAppContext() returning null
so I am not so confident about that.
-phil.
On 11/15/2016 10:44 AM, Sergey Bylokhov wrote:
I guess this should be closed as not a defect. getAppContext() can
return null if it will be called from the toolkit thread. If this
method is called by the user then appcontext should not be null,
additionally we should not cache this value in the static, so all
other code will use this cached static value.
On 15.11.16 20:38, Phil Race wrote:
+1
-phil.
On 11/15/2016 08:24 AM, Alexander Zvegintsev wrote:
Hello,
please review the fix
http://cr.openjdk.java.net/~azvegint/jdk/9/8151189/00/
for the issue
https://bugs.openjdk.java.net/browse/JDK-8151189
--
Best regards, Sergey.
Re: [9] Review request for 8151189: Possible getAppContext() NPE in java.awt.Desktop and java.awt.Taskbar
I forgot to save an instance of the Desktop class to the static variable.
http://cr.openjdk.java.net/~azvegint/jdk/9/8151189/01/
Thanks,
Alexander.
On 11/16/16 12:16 AM, Phil Race wrote:
A "main" appcontext will be created if you are a standalone app, but
not if running in webstart (although the means of determining that is
somewhat hokey) :-
if (numAppContexts.get() == 0) {
if
(System.getProperty("javaplugin.version") == null &&
System.getProperty("javawebstart.version") == null) {
initMainAppContext();
So if that is not initialised it appears to rely solely on an
appcontext being
associated with the current threadgroup - or a parent threadgroup.
If for some reason this does not return an appcontext you'll get the NPE.
This doesn't have to mean it is the Toolkit thread.
This example may be a little contrived but it illustrates the problem :-
If the webstart system property is set you will never see "got
desktop" printed
because the finalizer thread gets an exception.
===
import java.awt.Desktop;
public class GD {
public void finalize() {
System.out.println("get desktop");
System.out.println(Desktop.getDesktop());
System.out.println("got desktop");
}
public static void main(String[] args) {
if (args.length == 0) {
System.out.println("Setting webstart version");
System.setProperty("javawebstart.version", "8");
}
while (true) {
new GD();
System.gc();
}
}
}
-phil.
On 11/15/2016 10:48 AM, Phil Race wrote:
So are you saying we will never call this from the Toolkit thread, so
provably
there will never be an NPE ?
Seems we have had a ton of NPE bugs from getAppContext() returning null
so I am not so confident about that.
-phil.
On 11/15/2016 10:44 AM, Sergey Bylokhov wrote:
I guess this should be closed as not a defect. getAppContext() can
return null if it will be called from the toolkit thread. If this
method is called by the user then appcontext should not be null,
additionally we should not cache this value in the static, so all
other code will use this cached static value.
On 15.11.16 20:38, Phil Race wrote:
+1
-phil.
On 11/15/2016 08:24 AM, Alexander Zvegintsev wrote:
Hello,
please review the fix
http://cr.openjdk.java.net/~azvegint/jdk/9/8151189/00/
for the issue
https://bugs.openjdk.java.net/browse/JDK-8151189
Re: [9] Review request for 8151189: Possible getAppContext() NPE in java.awt.Desktop and java.awt.Taskbar
A "main" appcontext will be created if you are a standalone app, but
not if running in webstart (although the means of determining that is
somewhat hokey) :-
if (numAppContexts.get() == 0) {
if
(System.getProperty("javaplugin.version") == null &&
System.getProperty("javawebstart.version") == null) {
initMainAppContext();
So if that is not initialised it appears to rely solely on an appcontext
being
associated with the current threadgroup - or a parent threadgroup.
If for some reason this does not return an appcontext you'll get the NPE.
This doesn't have to mean it is the Toolkit thread.
This example may be a little contrived but it illustrates the problem :-
If the webstart system property is set you will never see "got desktop"
printed
because the finalizer thread gets an exception.
===
import java.awt.Desktop;
public class GD {
public void finalize() {
System.out.println("get desktop");
System.out.println(Desktop.getDesktop());
System.out.println("got desktop");
}
public static void main(String[] args) {
if (args.length == 0) {
System.out.println("Setting webstart version");
System.setProperty("javawebstart.version", "8");
}
while (true) {
new GD();
System.gc();
}
}
}
-phil.
On 11/15/2016 10:48 AM, Phil Race wrote:
So are you saying we will never call this from the Toolkit thread, so
provably
there will never be an NPE ?
Seems we have had a ton of NPE bugs from getAppContext() returning null
so I am not so confident about that.
-phil.
On 11/15/2016 10:44 AM, Sergey Bylokhov wrote:
I guess this should be closed as not a defect. getAppContext() can
return null if it will be called from the toolkit thread. If this
method is called by the user then appcontext should not be null,
additionally we should not cache this value in the static, so all
other code will use this cached static value.
On 15.11.16 20:38, Phil Race wrote:
+1
-phil.
On 11/15/2016 08:24 AM, Alexander Zvegintsev wrote:
Hello,
please review the fix
http://cr.openjdk.java.net/~azvegint/jdk/9/8151189/00/
for the issue
https://bugs.openjdk.java.net/browse/JDK-8151189
Re: [9] Review request for 8151189: Possible getAppContext() NPE in java.awt.Desktop and java.awt.Taskbar
So are you saying we will never call this from the Toolkit thread, so provably there will never be an NPE ? Seems we have had a ton of NPE bugs from getAppContext() returning null so I am not so confident about that. -phil. On 11/15/2016 10:44 AM, Sergey Bylokhov wrote: I guess this should be closed as not a defect. getAppContext() can return null if it will be called from the toolkit thread. If this method is called by the user then appcontext should not be null, additionally we should not cache this value in the static, so all other code will use this cached static value. On 15.11.16 20:38, Phil Race wrote: +1 -phil. On 11/15/2016 08:24 AM, Alexander Zvegintsev wrote: Hello, please review the fix http://cr.openjdk.java.net/~azvegint/jdk/9/8151189/00/ for the issue https://bugs.openjdk.java.net/browse/JDK-8151189
Re: [9] Review request for 8151189: Possible getAppContext() NPE in java.awt.Desktop and java.awt.Taskbar
I guess this should be closed as not a defect. getAppContext() can return null if it will be called from the toolkit thread. If this method is called by the user then appcontext should not be null, additionally we should not cache this value in the static, so all other code will use this cached static value. On 15.11.16 20:38, Phil Race wrote: +1 -phil. On 11/15/2016 08:24 AM, Alexander Zvegintsev wrote: Hello, please review the fix http://cr.openjdk.java.net/~azvegint/jdk/9/8151189/00/ for the issue https://bugs.openjdk.java.net/browse/JDK-8151189 -- Best regards, Sergey.
Re: [9] Review request for 8151189: Possible getAppContext() NPE in java.awt.Desktop and java.awt.Taskbar
+1 --Semyon On 15.11.2016 20:38, Phil Race wrote: +1 -phil. On 11/15/2016 08:24 AM, Alexander Zvegintsev wrote: Hello, please review the fix http://cr.openjdk.java.net/~azvegint/jdk/9/8151189/00/ for the issue https://bugs.openjdk.java.net/browse/JDK-8151189
Re: [9] Review request for 8151189: Possible getAppContext() NPE in java.awt.Desktop and java.awt.Taskbar
+1 -phil. On 11/15/2016 08:24 AM, Alexander Zvegintsev wrote: Hello, please review the fix http://cr.openjdk.java.net/~azvegint/jdk/9/8151189/00/ for the issue https://bugs.openjdk.java.net/browse/JDK-8151189
[9] Review request for 8151189: Possible getAppContext() NPE in java.awt.Desktop and java.awt.Taskbar
Hello, please review the fix http://cr.openjdk.java.net/~azvegint/jdk/9/8151189/00/ for the issue https://bugs.openjdk.java.net/browse/JDK-8151189 -- Thanks, Alexander.
