Re: RFR: 8256465: [macos11] Java frame and dialog presented full screen freeze application [v3]
On Thu, 13 May 2021 20:43:01 GMT, Phil Race wrote: >> Tejpal Rebari has updated the pull request incrementally with one additional >> commit since the last revision: >> >> code cleanup > > src/java.desktop/macosx/classes/sun/lwawt/macosx/CPlatformWindow.java line > 325: > >> 323: responder = createPlatformResponder(); >> 324: contentView.initialize(peer, responder); >> 325: setAllowAutomaticWindowTabbing(); > > This is in the initialize() method that is being called for every window. > That means the value could be changed by the app (back and forth!) during the > execution of the program. > We don't want that. So add it in a static initializer block for the class, so > it is only ever read ONCE and guaranteed to be read before the first window > is created. This then means the Java method isn't needed and the native > method will be static. Yeah, i have changed the method call to the native method in a static block and added the doPrivileged around getProperty. > src/java.desktop/macosx/classes/sun/lwawt/macosx/CPlatformWindow.java line > 607: > >> 605: >> nativeSetAllowAutomaticTabbingProperty(allowAutomaticWindowTabbing); >> 606: } >> 607: > > So we need a doPrivilged around the call to getProperty - in the new location > of the static block Done - PR: https://git.openjdk.java.net/jdk/pull/3407
Re: RFR: 8256465: [macos11] Java frame and dialog presented full screen freeze application [v3]
On Thu, 13 May 2021 21:37:18 GMT, Kevin Rushforth wrote:
>> src/java.desktop/macosx/native/libawt_lwawt/awt/AWTWindow.m line 1106:
>>
>>> 1104: {
>>> 1105: JNI_COCOA_ENTER(env);
>>> 1106: if (@available(macOS 10.12, *)) {
>>
>> @kevinrushforth said that since we set MIN_SDK (not sure of the exact
>> variable name) to 10.12, that this is compiled down to a no-op .. which
>> means it is useless and doesn't protect you from making the call on 10.11
>> So you might as well remove it. It won't prevent the crash that will happen
>> on 10.11.
>> @mrserb also pointed out people might then copy this pattern not realising
>> it does not work, and there's a better way ... apparently ...
>
> Right. @johanvos discovered this fun fact about `@available` when he got a
> crash report from a user. He filed
> [JDK-8266743](https://bugs.openjdk.java.net/browse/JDK-8266743), which
> describes this problem.
>
> The setting of minimum version of macOS is controlled by the
> `-mmacosx-version-min` compile and link flag. The minimum version is defined
> in
> [make/autoconf/flags.m4](https://github.com/openjdk/jdk/blob/master/make/autoconf/flags.m4#L136)
> and used in
> [make/autoconf/flags-cflags.m4](https://github.com/openjdk/jdk/blob/master/make/autoconf/flags-cflags.m4#L555).
>
> One thing I don't know (and can't try, since I don't have access to a macOS
> system that old) is whether the JDK will fail somewhere else anyway (e.g., if
> they check for a minimum OS at start up). So this might be a moot point, but
> as it stands, I think @mrserb is right that we should avoid this pattern. I
> would probably just remove it, but you could decide to use something like
> `respondsToSelector` (which is what I think Sergey was suggesting).
Since OpenJFX does not have its own launcher and IIRC JDK only recently (JDK 17
b08 https://bugs.openjdk.java.net/browse/JDK-8260518 ) set the minimum to 10.12
it is possible that the submitter of the FX crash was using A JDK prior to
that, in which case I am sure the Java Launcher would start up fine and you'd
crash only when calling this code. So also I think very aguably *library* code
has another reason to avoid this pattern.
And verifying what happens on 10.11 might be best done with a launcher from JDK
17 b07 or later .. also @kevinrushforth you might want to add some of these
thoughts to the FX bug.
-
PR: https://git.openjdk.java.net/jdk/pull/3407
Re: RFR: 8256465: [macos11] Java frame and dialog presented full screen freeze application [v3]
On Thu, 13 May 2021 20:46:28 GMT, Phil Race wrote:
>> Tejpal Rebari has updated the pull request incrementally with one additional
>> commit since the last revision:
>>
>> code cleanup
>
> src/java.desktop/macosx/native/libawt_lwawt/awt/AWTWindow.m line 1106:
>
>> 1104: {
>> 1105: JNI_COCOA_ENTER(env);
>> 1106: if (@available(macOS 10.12, *)) {
>
> @kevinrushforth said that since we set MIN_SDK (not sure of the exact
> variable name) to 10.12, that this is compiled down to a no-op .. which means
> it is useless and doesn't protect you from making the call on 10.11
> So you might as well remove it. It won't prevent the crash that will happen
> on 10.11.
> @mrserb also pointed out people might then copy this pattern not realising it
> does not work, and there's a better way ... apparently ...
Right. @johanvos discovered this fun fact about `@available` when he got a
crash report from a user. He filed
[JDK-8266743](https://bugs.openjdk.java.net/browse/JDK-8266743), which
describes this problem.
The setting of minimum version of macOS is controlled by the
`-mmacosx-version-min` compile and link flag. The minimum version is defined in
[make/autoconf/flags.m4](https://github.com/openjdk/jdk/blob/master/make/autoconf/flags.m4#L136)
and used in
[make/autoconf/flags-cflags.m4](https://github.com/openjdk/jdk/blob/master/make/autoconf/flags-cflags.m4#L555).
One thing I don't know (and can't try, since I don't have access to a macOS
system that old) is whether the JDK will fail somewhere else anyway (e.g., if
they check for a minimum OS at start up). So this might be a moot point, but as
it stands, I think @mrserb is right that we should avoid this pattern. I would
probably just remove it, but you could decide to use something like
`respondsToSelector` (which is what I think Sergey was suggesting).
-
PR: https://git.openjdk.java.net/jdk/pull/3407
Re: RFR: 8256465: [macos11] Java frame and dialog presented full screen freeze application [v3]
On Tue, 11 May 2021 12:02:48 GMT, Tejpal Rebari wrote: >>> Is there any place where we specify that "-Djdk.allowTabbedWindows" is a >>> new jdk property That's what the CSR is for. - PR: https://git.openjdk.java.net/jdk/pull/3407
Re: RFR: 8256465: [macos11] Java frame and dialog presented full screen freeze application [v3]
On Tue, 11 May 2021 11:43:35 GMT, Tejpal Rebari wrote:
>> Hi All,
>> Please review the following fix for jdk17.
>>
>> Issue : On MacOS 11 Java Frame and JDialog application is freezing in Full
>> Screen when the System Preference -> General -> Prefer Tabs is set to "Full
>> Screen". It is also freezing in normal screen when Prefer Tabs is set to
>> "Always".
>> It doesn't freeze when the Prefer tabs is set to "never".
>>
>> Fix : The default value of allowsAutomaticWindowTabbing is 0/NO in MacOS
>> prior to bigSur(11)
>> (in the AWTWindow.m file), so the issue is not seen in mac os 10.13 10.14
>> and 10.15.
>> From MacOS 11 onwards this value is set to 1/YES and the issue is seen.
>> This issue can also be reproduced in MacOS 10.15 by setting
>> setAllowsAutomaticWindowTabbing to true in the AWTWindow.m file.
>>
>> Fix is to set allowsAutomaticWindowTabbing to No for all the MacOS release
>> staring from 10.12.
>> The allowsAutomaticTabbing was introduced in MacOS 10.12 but the default
>> value changed in macos11.
>>
>> Test : Added a manual test and tested on MacOS 10.15 and 11.
>> All the internal tests run are green.
>
> Tejpal Rebari has updated the pull request incrementally with one additional
> commit since the last revision:
>
> code cleanup
src/java.desktop/macosx/classes/sun/lwawt/macosx/CPlatformWindow.java line 325:
> 323: responder = createPlatformResponder();
> 324: contentView.initialize(peer, responder);
> 325: setAllowAutomaticWindowTabbing();
This is in the initialize() method that is being called for every window.
That means the value could be changed by the app (back and forth!) during the
execution of the program.
We don't want that. So add it in a static initializer block for the class, so
it is only ever read ONCE and guaranteed to be read before the first window is
created. This then means the Java method isn't needed and the native method
will be static.
src/java.desktop/macosx/classes/sun/lwawt/macosx/CPlatformWindow.java line 607:
> 605:
> nativeSetAllowAutomaticTabbingProperty(allowAutomaticWindowTabbing);
> 606: }
> 607:
So we need a doPrivilged around the call to getProperty - in the new location
of the static block
src/java.desktop/macosx/native/libawt_lwawt/awt/AWTWindow.m line 1106:
> 1104: {
> 1105: JNI_COCOA_ENTER(env);
> 1106: if (@available(macOS 10.12, *)) {
@kevinrushforth said that since we set MIN_SDK (not sure of the exact variable
name) to 10.12, that this is compiled down to a no-op .. which means it is
useless and doesn't protect you from making the call on 10.11
So you might as well remove it. It won't prevent the crash that will happen on
10.11.
@mrserb also pointed out people might then copy this pattern not realising it
does not work, and there's a better way ... apparently ...
-
PR: https://git.openjdk.java.net/jdk/pull/3407
Re: RFR: 8256465: [macos11] Java frame and dialog presented full screen freeze application [v3]
On Tue, 11 May 2021 11:43:35 GMT, Tejpal Rebari wrote: >> Hi All, >> Please review the following fix for jdk17. >> >> Issue : On MacOS 11 Java Frame and JDialog application is freezing in Full >> Screen when the System Preference -> General -> Prefer Tabs is set to "Full >> Screen". It is also freezing in normal screen when Prefer Tabs is set to >> "Always". >> It doesn't freeze when the Prefer tabs is set to "never". >> >> Fix : The default value of allowsAutomaticWindowTabbing is 0/NO in MacOS >> prior to bigSur(11) >> (in the AWTWindow.m file), so the issue is not seen in mac os 10.13 10.14 >> and 10.15. >> From MacOS 11 onwards this value is set to 1/YES and the issue is seen. >> This issue can also be reproduced in MacOS 10.15 by setting >> setAllowsAutomaticWindowTabbing to true in the AWTWindow.m file. >> >> Fix is to set allowsAutomaticWindowTabbing to No for all the MacOS release >> staring from 10.12. >> The allowsAutomaticTabbing was introduced in MacOS 10.12 but the default >> value changed in macos11. >> >> Test : Added a manual test and tested on MacOS 10.15 and 11. >> All the internal tests run are green. > > Tejpal Rebari has updated the pull request incrementally with one additional > commit since the last revision: > > code cleanup Is there any place where we specify that "-Djdk.allowTabbedWindows" is a new jdk property which is for the macos automatic window tabbing. This is off by default and can be set using -Djdk.allowTabbedWindows=true/TRUE/TrUE ... . - PR: https://git.openjdk.java.net/jdk/pull/3407
Re: RFR: 8256465: [macos11] Java frame and dialog presented full screen freeze application [v3]
> Hi All, > Please review the following fix for jdk17. > > Issue : On MacOS 11 Java Frame and JDialog application is freezing in Full > Screen when the System Preference -> General -> Prefer Tabs is set to "Full > Screen". It is also freezing in normal screen when Prefer Tabs is set to > "Always". > It doesn't freeze when the Prefer tabs is set to "never". > > Fix : The default value of allowsAutomaticWindowTabbing is 0/NO in MacOS > prior to bigSur(11) > (in the AWTWindow.m file), so the issue is not seen in mac os 10.13 10.14 > and 10.15. > From MacOS 11 onwards this value is set to 1/YES and the issue is seen. > This issue can also be reproduced in MacOS 10.15 by setting > setAllowsAutomaticWindowTabbing to true in the AWTWindow.m file. > > Fix is to set allowsAutomaticWindowTabbing to No for all the MacOS release > staring from 10.12. > The allowsAutomaticTabbing was introduced in MacOS 10.12 but the default > value changed in macos11. > > Test : Added a manual test and tested on MacOS 10.15 and 11. > All the internal tests run are green. Tejpal Rebari has updated the pull request incrementally with one additional commit since the last revision: code cleanup - Changes: - all: https://git.openjdk.java.net/jdk/pull/3407/files - new: https://git.openjdk.java.net/jdk/pull/3407/files/d85a002d..8193b3e4 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk&pr=3407&range=02 - incr: https://webrevs.openjdk.java.net/?repo=jdk&pr=3407&range=01-02 Stats: 2 lines in 1 file changed: 0 ins; 2 del; 0 mod Patch: https://git.openjdk.java.net/jdk/pull/3407.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/3407/head:pull/3407 PR: https://git.openjdk.java.net/jdk/pull/3407
