Re: [OpenJDK 2D-Dev] RFR: JDK-8260432: allocateSpaceForGP in freetypeScaler.c might leak memory [v2]

[Aleksey Shipilev]

On Wed, 27 Jan 2021 13:09:56 GMT, Matthias Baesken  wrote:

>> The function  AllocateSpaceForGP in freetypeScaler.c calls potentially 2 
>> times malloc ; however the memory is not always freed correctly in case of 
>> errors.
>> See also the related  sonar issue :
>> https://sonarcloud.io/project/issues?id=shipilev_jdk=c=AXck8B_SBBG2CXpcngxr=false=BLOCKER=BUG
>
> Matthias Baesken has updated the pull request incrementally with one 
> additional commit since the last revision:
> 
>   JDK-8260426

This looks fine to me, modulo stylistic nits

src/java.desktop/share/native/libfontmanager/freetypeScaler.c line 1289:

> 1287: /* failure if any of mallocs failed */
> 1288: if (gpdata->pointTypes == NULL || gpdata->pointCoords == NULL) {
> 1289: if (gpdata->pointTypes != NULL)  { free(gpdata->pointTypes); 
> gpdata->pointTypes = NULL; }

You might want to add an extra space before `gpdata->pointTypes = NULL;` to 
align the statements vertically. You call.

-

Marked as reviewed by shade (Reviewer).

PR: https://git.openjdk.java.net/jdk/pull/2250

Re: [OpenJDK 2D-Dev] RFR: JDK-8260432: allocateSpaceForGP in freetypeScaler.c might leak memory [v2]

[Aleksey Shipilev]

On Thu, 28 Jan 2021 07:53:44 GMT, Aleksey Shipilev  wrote:

>> Matthias Baesken has updated the pull request incrementally with one 
>> additional commit since the last revision:
>> 
>>   JDK-8260426
>
> This looks fine to me, modulo stylistic nits

Pull from master to get x86_32 GHA jobs fixed and running.

> src/java.desktop/share/native/libfontmanager/freetypeScaler.c line 1289:
> 
>> 1287: /* failure if any of mallocs failed */
>> 1288: if (gpdata->pointTypes == NULL || gpdata->pointCoords == NULL) {
>> 1289: if (gpdata->pointTypes != NULL)  { free(gpdata->pointTypes); 
>> gpdata->pointTypes = NULL; }
> 
> You might want to add an extra space before `gpdata->pointTypes = NULL;` to 
> align the statements vertically. You call.

Can we also style it consistently, like:

  if (gpdata->pointTypes != NULL)  { 
  free(gpdata->pointTypes);
  gpdata->pointTypes = NULL;
  }
  if (gpdata->pointCoords != NULL) {
  free(gpdata->pointCoords);
  gpdata->pointCoords = NULL;
  }

-

PR: https://git.openjdk.java.net/jdk/pull/2250

Re: [OpenJDK 2D-Dev] RFR: JDK-8260432: allocateSpaceForGP in freetypeScaler.c might leak memory [v2]

[Matthias Baesken]

On Wed, 27 Jan 2021 12:16:47 GMT, Thomas Stuefe  wrote:

>> Yes, I think that is better: `realloc` can fail as well, and we don't want 
>> to leak those?
>
> Please also reset the free'd pointers to NULL in GPData* before returning.

Hello  Thomas and Aleksey, I moved the free calls and added the NULL settings. 
Are you fine with the latest commit?
Thanks, Matthias

-

PR: https://git.openjdk.java.net/jdk/pull/2250

Re: [OpenJDK 2D-Dev] RFR: 8197825: [Test] Intermiitent timeout with javax/swing JColorChooser Test [v3]

[Sergey Bylokhov]

On Thu, 28 Jan 2021 00:41:13 GMT, Sergey Bylokhov  wrote:

>> src/java.desktop/windows/native/libawt/windows/awt_Toolkit.cpp line 2611:
>> 
>>> 2609: Sleep(100);
>>> 2610: }
>>> 2611: 
>> 
>> It can cause infinite looping as the message queue to get DISPOSE events is 
>> removed so if we are not getting disposed by now, we probably will not get 
>> DISPOSE event ever causing infinite loop.
>> XToolkit, LWToolkit does not have this infinite loop.
>
> My point is that this is not a test bug, so the test should not be changed.

Please take a look at the "AwtToolkit::Dispose()" method, on how much stuff 
should be done to properly shutdown the toolkit. This Dispose() method is 
executed immediately when we exit the message loop in the 
"Java_sun_awt_windows_WToolkit_eventLoop". So when the shutdown hook is 
executed we should have the message loop, then we call tk.QuitMessageLoop to 
stop it, and wait until all code in the Dispose() is executed. But since the 
IsDisposed() return false we for unknow reason hang, does it mean that the 
message loop still operates? Or we got some error during "QuitMessageLoop"?

-

PR: https://git.openjdk.java.net/jdk/pull/2220

Re: [OpenJDK 2D-Dev] RFR: 8197825: [Test] Intermiitent timeout with javax/swing JColorChooser Test [v3]

[Prasanta Sadhukhan]

> This test was failing in our nightly mach5 testing. Appropriate stability 
> code in form of waitForIdle and delay is added.
> mach5 job running for several iterations on all platforms is ok. Link in JBS.

Prasanta Sadhukhan has updated the pull request incrementally with one 
additional commit since the last revision:

  Test changes reverted

-

Changes:
  - all: https://git.openjdk.java.net/jdk/pull/2220/files
  - new: https://git.openjdk.java.net/jdk/pull/2220/files/87d371ad..6fdcc0d7

Webrevs:
 - full: https://webrevs.openjdk.java.net/?repo=jdk=2220=02
 - incr: https://webrevs.openjdk.java.net/?repo=jdk=2220=01-02

  Stats: 32 lines in 1 file changed: 0 ins; 12 del; 20 mod
  Patch: https://git.openjdk.java.net/jdk/pull/2220.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/2220/head:pull/2220

PR: https://git.openjdk.java.net/jdk/pull/2220

Re: [OpenJDK 2D-Dev] RFR: JDK-8260518: Change default -mmacosx-version-min to 10.12

[Sergey Bylokhov]

On Wed, 27 Jan 2021 20:43:44 GMT, Phil Race  wrote:

>> To guarantee backwards compatible binaries on Macos, we use the option 
>> -mmacosx-version-min. This is currently set to 10.9, which is a really 
>> ancient version. I propose we bump this to 10.12, which is still a rather 
>> conservative old version (support ended in 2019).
>> 
>> The driving issue for bumping this now is the aarch64 port, where building 
>> for aarch64 requires the version min to be set to 11.0. Having a large gap 
>> between the target versions becomes problematic as we hit a lot of 
>> deprecation warnings in shared code. To be able to fix these deprecation 
>> warnings, we need a smaller version gap.
>> 
>> Just bumping us to 10.12 triggers warnings in libsplashscreen, so I will 
>> temporarily add "deprecated-declarations" to the list of disabled warnings 
>> there until they can be fixed in JDK-8260402.
>
> Marked as reviewed by prr (Reviewer).

Will this affect the minimum version of macOS which will be able to run on? 
don't we need a CSR(not sure)?

-

PR: https://git.openjdk.java.net/jdk/pull/2268

Re: [OpenJDK 2D-Dev] RFR: 6986863: ProfileDeferralMgr throwing ConcurrentModificationException [v6]

[Sergey Bylokhov]

> This change intended to enhance the lazy initialization of the standard color 
> profiles concurrently by different threads.
> 
> We defer standard profiles loading because most of UI application uses a 
> small amount of data from the profile like numComponents and colorSpaceType, 
> and this data is known in advance. But any other profile-related activity 
> (like a color conversion, profile data accesses, etc.) triggers profile 
> activation when we load all profile data to the memory.
> 
> Before the fix for JDK-6793818, we defer only one sRGB color profile, see: 
> https://github.com/openjdk/jdk/commit/2726f2a3621dd2562d4fb660b4c3d376c65027aa
> 
> Notes about the link above:
> - The code in the ProfileDeferralMgr, which contain the Vector of profiles 
> for activation does not use any synchronization
> - The `activateDeferredProfile` and `activate` methods are implemented to 
> throw `ProfileDataException`, but this exception is ignored during activation 
> process: 
> 
> https://github.com/openjdk/jdk/commit/2726f2a3621dd2562d4fb660b4c3d376c65027aa#diff-0839c25a6c999452be28b431c54d5daa91364d302cfda6efa5c56421c2f2bdcbR96
> 
> The fix:
>  - Drops the usage of ProfileDeferralMgr (which contained the Vector of 
> profiles for activation) and ProfileActivator machinery. Instead, we will 
> have just one `ICC_Profile.activate()` method which will activate and 
> initialize the `ICC_Profile.cmmProfile` if it is null
>  - The `activate` method implementation mimics the old behavior when the 
> CMMException and IOException were wrapped by the ProfileDataException, and 
> the ProfileDataException itself was ignored during activation - > so an 
> exception will not be thrown in the method itself, but only when the null 
> profile will be used.
> 
> See some comments inline.

Sergey Bylokhov has updated the pull request with a new target base due to a 
merge or a rebase. The pull request now contains 17 commits:

 - Merge branch 'JDK-6986863' of https://github.com/mrserb/jdk into JDK-6986863
 - Merge branch 'master' into JDK-6986863
 - Merge branch 'master' into JDK-6986863
 - Merge branch 'master' into JDK-6986863
 - update dates
 - Update ProfileActivationDuringPropertyAccess.java
 - Merge branch 'master' into JDK-6986863
 - Merge branch 'master' into JDK-6986863
 - Merge branch 'master' into JDK-6986863
 - Merge branch 'master' into JDK-6986863
 - ... and 7 more: https://git.openjdk.java.net/jdk/compare/7030d2e0...e0e59caf

-

Changes: https://git.openjdk.java.net/jdk/pull/1613/files
 Webrev: https://webrevs.openjdk.java.net/?repo=jdk=1613=05
  Stats: 520 lines in 9 files changed: 183 ins; 265 del; 72 mod
  Patch: https://git.openjdk.java.net/jdk/pull/1613.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/1613/head:pull/1613

PR: https://git.openjdk.java.net/jdk/pull/1613

Re: [OpenJDK 2D-Dev] RFR: 6986863: ProfileDeferralMgr throwing ConcurrentModificationException [v5]

[Sergey Bylokhov]

On Wed, 27 Jan 2021 18:38:27 GMT, Alexander Zuev  wrote:

>> Sergey Bylokhov has updated the pull request with a new target base due to a 
>> merge or a rebase. The pull request now contains 14 commits:
>> 
>>  - Merge branch 'master' into JDK-6986863
>>  - Merge branch 'master' into JDK-6986863
>>  - Update ProfileActivationDuringPropertyAccess.java
>>  - Merge branch 'master' into JDK-6986863
>>  - Merge branch 'master' into JDK-6986863
>>  - Merge branch 'master' into JDK-6986863
>>  - Merge branch 'master' into JDK-6986863
>>  - Delete the DeferralMgr machinery
>>  - Update ICC_Profile.java
>>  - Wrong spec in ProfileDeferralInfo
>>  - ... and 4 more: 
>> https://git.openjdk.java.net/jdk/compare/14522800...ecfc1ed8
>
> src/java.desktop/share/classes/sun/java2d/cmm/lcms/LCMSTransform.java line 2:
> 
>> 1: /*
>> 2:  * Copyright (c) 2007, 2020, Oracle and/or its affiliates. All rights 
>> reserved.
> 
> Review took so long that years here and in other headers of modified files 
> needs to be updated.

Fixed.

> test/jdk/java/awt/color/ICC_ProfileRGB/MTMatrixAccess.java line 54:
> 
>> 52: }
>> 53: try {
>> 54: rgb.getMatrix();
> 
> What should happen when this method is called on profile that is not 
> activated yet? Should you just check that it returns the non-null array of 
> correct size?

This method always returns the array, only an exception can prevent that.

-

PR: https://git.openjdk.java.net/jdk/pull/1613

Re: [OpenJDK 2D-Dev] RFR: 8197825: [Test] Intermiitent timeout with javax/swing JColorChooser Test [v2]

[Sergey Bylokhov]

On Wed, 27 Jan 2021 12:35:38 GMT, Prasanta Sadhukhan  
wrote:

>> Prasanta Sadhukhan has updated the pull request incrementally with one 
>> additional commit since the last revision:
>> 
>>   Prevent infinite loop
>
> src/java.desktop/windows/native/libawt/windows/awt_Toolkit.cpp line 2611:
> 
>> 2609: Sleep(100);
>> 2610: }
>> 2611: 
> 
> It can cause infinite looping as the message queue to get DISPOSE events is 
> removed so if we are not getting disposed by now, we probably will not get 
> DISPOSE event ever causing infinite loop.
> XToolkit, LWToolkit does not have this infinite loop.

My point is that this is not a test bug, so the test should not be changed.

-

PR: https://git.openjdk.java.net/jdk/pull/2220

Re: [OpenJDK 2D-Dev] RFR: 8197825: [Test] Intermiitent timeout with javax/swing JColorChooser Test [v2]

[Alexey Ivanov]

On Wed, 27 Jan 2021 12:39:01 GMT, Prasanta Sadhukhan  
wrote:

>> This test was failing in our nightly mach5 testing. Appropriate stability 
>> code in form of waitForIdle and delay is added.
>> mach5 job running for several iterations on all platforms is ok. Link in JBS.
>
> Prasanta Sadhukhan has updated the pull request incrementally with one 
> additional commit since the last revision:
> 
>   Prevent infinite loop

test/jdk/javax/swing/JColorChooser/Test6827032.java line 78:

> 76: } finally {
> 77: if (frame != null) {
> 78: SwingUtilities.invokeAndWait(() -> frame.dispose());

Shall the frame be `volatile` too? It's accessed from main thread for the 
null-check.

-

PR: https://git.openjdk.java.net/jdk/pull/2220

Re: [OpenJDK 2D-Dev] RFR: JDK-8260518: Change default -mmacosx-version-min to 10.12

[Phil Race]

On Wed, 27 Jan 2021 19:23:48 GMT, Erik Joelsson  wrote:

> To guarantee backwards compatible binaries on Macos, we use the option 
> -mmacosx-version-min. This is currently set to 10.9, which is a really 
> ancient version. I propose we bump this to 10.12, which is still a rather 
> conservative old version (support ended in 2019).
> 
> The driving issue for bumping this now is the aarch64 port, where building 
> for aarch64 requires the version min to be set to 11.0. Having a large gap 
> between the target versions becomes problematic as we hit a lot of 
> deprecation warnings in shared code. To be able to fix these deprecation 
> warnings, we need a smaller version gap.
> 
> Just bumping us to 10.12 triggers warnings in libsplashscreen, so I will 
> temporarily add "deprecated-declarations" to the list of disabled warnings 
> there until they can be fixed in JDK-8260402.

Marked as reviewed by prr (Reviewer).

-

PR: https://git.openjdk.java.net/jdk/pull/2268

Re: [OpenJDK 2D-Dev] RFR: JDK-8260518: Change default -mmacosx-version-min to 10.12

[Tim Bell]

On Wed, 27 Jan 2021 19:23:48 GMT, Erik Joelsson  wrote:

> To guarantee backwards compatible binaries on Macos, we use the option 
> -mmacosx-version-min. This is currently set to 10.9, which is a really 
> ancient version. I propose we bump this to 10.12, which is still a rather 
> conservative old version (support ended in 2019).
> 
> The driving issue for bumping this now is the aarch64 port, where building 
> for aarch64 requires the version min to be set to 11.0. Having a large gap 
> between the target versions becomes problematic as we hit a lot of 
> deprecation warnings in shared code. To be able to fix these deprecation 
> warnings, we need a smaller version gap.
> 
> Just bumping us to 10.12 triggers warnings in libsplashscreen, so I will 
> temporarily add "deprecated-declarations" to the list of disabled warnings 
> there until they can be fixed in JDK-8260402.

Looks good.

-

Marked as reviewed by tbell (Reviewer).

PR: https://git.openjdk.java.net/jdk/pull/2268

[OpenJDK 2D-Dev] RFR: JDK-8260518: Change default -mmacosx-version-min to 10.12

[Erik Joelsson]

To guarantee backwards compatible binaries on Macos, we use the option 
-mmacosx-version-min. This is currently set to 10.9, which is a really ancient 
version. I propose we bump this to 10.12, which is still a rather conservative 
old version (support ended in 2019).

The driving issue for bumping this now is the aarch64 port, where building for 
aarch64 requires the version min to be set to 11.0. Having a large gap between 
the target versions becomes problematic as we hit a lot of deprecation warnings 
in shared code. To be able to fix these deprecation warnings, we need a smaller 
version gap.

Just bumping us to 10.12 triggers warnings in libsplashscreen, so I will 
temporarily add "deprecated-declarations" to the list of disabled warnings 
there until they can be fixed in JDK-8260402.

-

Commit messages:
 - Bump macosx version min/max to 10.12

Changes: https://git.openjdk.java.net/jdk/pull/2268/files
 Webrev: https://webrevs.openjdk.java.net/?repo=jdk=2268=00
  Issue: https://bugs.openjdk.java.net/browse/JDK-8260518
  Stats: 4 lines in 3 files changed: 1 ins; 0 del; 3 mod
  Patch: https://git.openjdk.java.net/jdk/pull/2268.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/2268/head:pull/2268

PR: https://git.openjdk.java.net/jdk/pull/2268

Re: [OpenJDK 2D-Dev] RFR: 6986863: ProfileDeferralMgr throwing ConcurrentModificationException [v5]

[Alexander Zuev]

On Fri, 22 Jan 2021 10:35:13 GMT, Sergey Bylokhov  wrote:

>> This change intended to enhance the lazy initialization of the standard 
>> color profiles concurrently by different threads.
>> 
>> We defer standard profiles loading because most of UI application uses a 
>> small amount of data from the profile like numComponents and colorSpaceType, 
>> and this data is known in advance. But any other profile-related activity 
>> (like a color conversion, profile data accesses, etc.) triggers profile 
>> activation when we load all profile data to the memory.
>> 
>> Before the fix for JDK-6793818, we defer only one sRGB color profile, see: 
>> https://github.com/openjdk/jdk/commit/2726f2a3621dd2562d4fb660b4c3d376c65027aa
>> 
>> Notes about the link above:
>> - The code in the ProfileDeferralMgr, which contain the Vector of profiles 
>> for activation does not use any synchronization
>> - The `activateDeferredProfile` and `activate` methods are implemented to 
>> throw `ProfileDataException`, but this exception is ignored during 
>> activation process: 
>> 
>> https://github.com/openjdk/jdk/commit/2726f2a3621dd2562d4fb660b4c3d376c65027aa#diff-0839c25a6c999452be28b431c54d5daa91364d302cfda6efa5c56421c2f2bdcbR96
>> 
>> The fix:
>>  - Drops the usage of ProfileDeferralMgr (which contained the Vector of 
>> profiles for activation) and ProfileActivator machinery. Instead, we will 
>> have just one `ICC_Profile.activate()` method which will activate and 
>> initialize the `ICC_Profile.cmmProfile` if it is null
>>  - The `activate` method implementation mimics the old behavior when the 
>> CMMException and IOException were wrapped by the ProfileDataException, and 
>> the ProfileDataException itself was ignored during activation - > so an 
>> exception will not be thrown in the method itself, but only when the null 
>> profile will be used.
>> 
>> See some comments inline.
>
> Sergey Bylokhov has updated the pull request with a new target base due to a 
> merge or a rebase. The pull request now contains 14 commits:
> 
>  - Merge branch 'master' into JDK-6986863
>  - Merge branch 'master' into JDK-6986863
>  - Update ProfileActivationDuringPropertyAccess.java
>  - Merge branch 'master' into JDK-6986863
>  - Merge branch 'master' into JDK-6986863
>  - Merge branch 'master' into JDK-6986863
>  - Merge branch 'master' into JDK-6986863
>  - Delete the DeferralMgr machinery
>  - Update ICC_Profile.java
>  - Wrong spec in ProfileDeferralInfo
>  - ... and 4 more: 
> https://git.openjdk.java.net/jdk/compare/14522800...ecfc1ed8

test/jdk/java/awt/color/ICC_ProfileRGB/MTMatrixAccess.java line 54:

> 52: }
> 53: try {
> 54: rgb.getMatrix();

What should happen when this method is called on profile that is not activated 
yet? Should you just check that it returns the non-null array of correct size?

-

PR: https://git.openjdk.java.net/jdk/pull/1613

Re: [OpenJDK 2D-Dev] RFR: 6986863: ProfileDeferralMgr throwing ConcurrentModificationException [v5]

[Alexander Zuev]

On Fri, 22 Jan 2021 10:35:13 GMT, Sergey Bylokhov  wrote:

>> This change intended to enhance the lazy initialization of the standard 
>> color profiles concurrently by different threads.
>> 
>> We defer standard profiles loading because most of UI application uses a 
>> small amount of data from the profile like numComponents and colorSpaceType, 
>> and this data is known in advance. But any other profile-related activity 
>> (like a color conversion, profile data accesses, etc.) triggers profile 
>> activation when we load all profile data to the memory.
>> 
>> Before the fix for JDK-6793818, we defer only one sRGB color profile, see: 
>> https://github.com/openjdk/jdk/commit/2726f2a3621dd2562d4fb660b4c3d376c65027aa
>> 
>> Notes about the link above:
>> - The code in the ProfileDeferralMgr, which contain the Vector of profiles 
>> for activation does not use any synchronization
>> - The `activateDeferredProfile` and `activate` methods are implemented to 
>> throw `ProfileDataException`, but this exception is ignored during 
>> activation process: 
>> 
>> https://github.com/openjdk/jdk/commit/2726f2a3621dd2562d4fb660b4c3d376c65027aa#diff-0839c25a6c999452be28b431c54d5daa91364d302cfda6efa5c56421c2f2bdcbR96
>> 
>> The fix:
>>  - Drops the usage of ProfileDeferralMgr (which contained the Vector of 
>> profiles for activation) and ProfileActivator machinery. Instead, we will 
>> have just one `ICC_Profile.activate()` method which will activate and 
>> initialize the `ICC_Profile.cmmProfile` if it is null
>>  - The `activate` method implementation mimics the old behavior when the 
>> CMMException and IOException were wrapped by the ProfileDataException, and 
>> the ProfileDataException itself was ignored during activation - > so an 
>> exception will not be thrown in the method itself, but only when the null 
>> profile will be used.
>> 
>> See some comments inline.
>
> Sergey Bylokhov has updated the pull request with a new target base due to a 
> merge or a rebase. The pull request now contains 14 commits:
> 
>  - Merge branch 'master' into JDK-6986863
>  - Merge branch 'master' into JDK-6986863
>  - Update ProfileActivationDuringPropertyAccess.java
>  - Merge branch 'master' into JDK-6986863
>  - Merge branch 'master' into JDK-6986863
>  - Merge branch 'master' into JDK-6986863
>  - Merge branch 'master' into JDK-6986863
>  - Delete the DeferralMgr machinery
>  - Update ICC_Profile.java
>  - Wrong spec in ProfileDeferralInfo
>  - ... and 4 more: 
> https://git.openjdk.java.net/jdk/compare/14522800...ecfc1ed8

src/java.desktop/share/classes/sun/java2d/cmm/lcms/LCMSTransform.java line 2:

> 1: /*
> 2:  * Copyright (c) 2007, 2020, Oracle and/or its affiliates. All rights 
> reserved.

Review took so long that years here and in other headers of modified files 
needs to be updated.

-

PR: https://git.openjdk.java.net/jdk/pull/1613

Re: [OpenJDK 2D-Dev] RFR: 8253795: Implementation of JEP 391: macOS/AArch64 Port [v6]

[Vladimir Kempik]

On Wed, 27 Jan 2021 08:36:19 GMT, Magnus Ihse Bursie  wrote:

> Build changes per se now looks okay. However, I agree with Erik that unless 
> this PR can wait for the JNF removal, at the very least the build docs needs 
> to be updated to explain how to successfully build for this platform. (I can 
> live with the configure command line hack, since it's temporary -- otherwise 
> I'd have requested a new configure argument.) This can be done in this PR or 
> a follow-up PR.

I believe it's better be done under separate PR/bugfix, so it can be completely 
reverted once JNF removed.

-

PR: https://git.openjdk.java.net/jdk/pull/2200

Re: [OpenJDK 2D-Dev] RFR: 8253795: Implementation of JEP 391: macOS/AArch64 Port [v7]

[Anton Kozlov]

> Please review the implementation of JEP 391: macOS/AArch64 Port.
> 
> It's heavily based on existing ports to linux/aarch64, macos/x86_64, and 
> windows/aarch64. 
> 
> Major changes are in:
> * src/hotspot/cpu/aarch64: support of the new calling convention (subtasks 
> JDK-8253817, JDK-8253818)
> * src/hotspot/os_cpu/bsd_aarch64: copy of os_cpu/linux_aarch64 with necessary 
> adjustments (JDK-8253819)
> * src/hotspot/share, test/hotspot/gtest: support of write-xor-execute (W^X), 
> required on macOS/AArch64 platform. It's implemented with 
> pthread_jit_write_protect_np provided by Apple. The W^X mode is local to a 
> thread, so W^X mode change relates to the java thread state change (for java 
> threads). In most cases, JVM executes in write-only mode, except when calling 
> a generated stub like SafeFetch, which requires a temporary switch to 
> execute-only mode. The same execute-only mode is enabled when a java thread 
> executes in java or native states. This approach of managing W^X mode turned 
> out to be simple and efficient enough.
> * src/jdk.hotspot.agent: serviceability agent implementation (JDK-8254941)

Anton Kozlov has updated the pull request incrementally with two additional 
commits since the last revision:

 - Update copyright year for BsdAARCH64ThreadContext.java
 - Fix inclusing of StubRoutines header

-

Changes:
  - all: https://git.openjdk.java.net/jdk/pull/2200/files
  - new: https://git.openjdk.java.net/jdk/pull/2200/files/f1ef6240..9d8b07c2

Webrevs:
 - full: https://webrevs.openjdk.java.net/?repo=jdk=2200=06
 - incr: https://webrevs.openjdk.java.net/?repo=jdk=2200=05-06

  Stats: 5 lines in 4 files changed: 0 ins; 2 del; 3 mod
  Patch: https://git.openjdk.java.net/jdk/pull/2200.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/2200/head:pull/2200

PR: https://git.openjdk.java.net/jdk/pull/2200

Re: [OpenJDK 2D-Dev] RFR: JDK-8260432: allocateSpaceForGP in freetypeScaler.c might leak memory [v2]

[Matthias Baesken]

> The function  AllocateSpaceForGP in freetypeScaler.c calls potentially 2 
> times malloc ; however the memory is not always freed correctly in case of 
> errors.
> See also the related  sonar issue :
> https://sonarcloud.io/project/issues?id=shipilev_jdk=c=AXck8B_SBBG2CXpcngxr=false=BLOCKER=BUG

Matthias Baesken has updated the pull request incrementally with one additional 
commit since the last revision:

  JDK-8260426

-

Changes:
  - all: https://git.openjdk.java.net/jdk/pull/2250/files
  - new: https://git.openjdk.java.net/jdk/pull/2250/files/5c867221..afda19e5

Webrevs:
 - full: https://webrevs.openjdk.java.net/?repo=jdk=2250=01
 - incr: https://webrevs.openjdk.java.net/?repo=jdk=2250=00-01

  Stats: 7 lines in 1 file changed: 2 ins; 2 del; 3 mod
  Patch: https://git.openjdk.java.net/jdk/pull/2250.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/2250/head:pull/2250

PR: https://git.openjdk.java.net/jdk/pull/2250

Re: [OpenJDK 2D-Dev] RFR: 8197825: [Test] Intermiitent timeout with javax/swing JColorChooser Test [v2]

[Prasanta Sadhukhan]

On Wed, 27 Jan 2021 12:35:38 GMT, Prasanta Sadhukhan  
wrote:

>> This test was failing in our nightly mach5 testing. Appropriate stability 
>> code in form of waitForIdle and delay is added.
>> mach5 job running for several iterations on all platforms is ok. Link in JBS.
>
> Prasanta Sadhukhan has updated the pull request incrementally with one 
> additional commit since the last revision:
> 
>   Prevent infinite loop

src/java.desktop/windows/native/libawt/windows/awt_Toolkit.cpp line 2611:

> 2609: Sleep(100);
> 2610: }
> 2611: 

It can cause infinite looping as the message queue to get DISPOSE events is 
removed so if we are not getting disposed by now, we probably will not get 
DISPOSE event ever causing infinite loop.
XToolkit, LWToolkit does not have this infinite loop.

-

PR: https://git.openjdk.java.net/jdk/pull/2220

Re: [OpenJDK 2D-Dev] RFR: 8197825: [Test] Intermiitent timeout with javax/swing JColorChooser Test [v2]

[Prasanta Sadhukhan]

> This test was failing in our nightly mach5 testing. Appropriate stability 
> code in form of waitForIdle and delay is added.
> mach5 job running for several iterations on all platforms is ok. Link in JBS.

Prasanta Sadhukhan has updated the pull request incrementally with one 
additional commit since the last revision:

  Prevent infinite loop

-

Changes:
  - all: https://git.openjdk.java.net/jdk/pull/2220/files
  - new: https://git.openjdk.java.net/jdk/pull/2220/files/3db928b1..87d371ad

Webrevs:
 - full: https://webrevs.openjdk.java.net/?repo=jdk=2220=01
 - incr: https://webrevs.openjdk.java.net/?repo=jdk=2220=00-01

  Stats: 6 lines in 2 files changed: 0 ins; 5 del; 1 mod
  Patch: https://git.openjdk.java.net/jdk/pull/2220.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/2220/head:pull/2220

PR: https://git.openjdk.java.net/jdk/pull/2220

Re: [OpenJDK 2D-Dev] RFR: JDK-8260432: allocateSpaceForGP in freetypeScaler.c might leak memory

[Thomas Stuefe]

On Wed, 27 Jan 2021 12:02:06 GMT, Aleksey Shipilev  wrote:

>> Then we would free   as well  for the realloc code path
>> } else {
>> /* do we have enough space? */
>> ...
>> }
>> Is this okay?
>
> Yes, I think that is better: `realloc` can fail as well, and we don't want to 
> leak those?

Please also reset the free'd pointers to NULL in GPData* before returning.

-

PR: https://git.openjdk.java.net/jdk/pull/2250

[OpenJDK 2D-Dev] Integrated: 8260314: Replace border="1" on tables with CSS

[Alexey Ivanov]

On Sat, 23 Jan 2021 19:06:05 GMT, Alexey Ivanov  wrote:

> Replace presentational attribute `border="1"` on `` element with CSS 
> styles:
> table {border: outset 1px}
> th, td {border: inset 1px} 
> 
> These declarations produce the same rendering as the default one in Firefox 
> and Edge. 
> 
> Another option is to use `solid` in both cases.

This pull request has now been integrated.

Changeset: 7ed591cc
Author:Alexey Ivanov 
URL:   https://git.openjdk.java.net/jdk/commit/7ed591cc
Stats: 36 lines in 6 files changed: 0 ins; 1 del; 35 mod

8260314: Replace border="1" on tables with CSS

Reviewed-by: serb

-

PR: https://git.openjdk.java.net/jdk/pull/2210

Re: [OpenJDK 2D-Dev] RFR: JDK-8260432: allocateSpaceForGP in freetypeScaler.c might leak memory

[Matthias Baesken]

On Wed, 27 Jan 2021 11:41:48 GMT, Aleksey Shipilev  wrote:

>> The function  AllocateSpaceForGP in freetypeScaler.c calls potentially 2 
>> times malloc ; however the memory is not always freed correctly in case of 
>> errors.
>> See also the related  sonar issue :
>> https://sonarcloud.io/project/issues?id=shipilev_jdk=c=AXck8B_SBBG2CXpcngxr=false=BLOCKER=BUG
>
> src/java.desktop/share/native/libfontmanager/freetypeScaler.c line 1290:
> 
>> 1288: 
>> 1289: /* failure if any of mallocs failed */
>> 1290: if (gpdata->pointTypes == NULL || gpdata->pointCoords == NULL)
> 
> I think it would be cleaner to free the remaining allocations on the failing 
> path:
> 
> if (gpdata->pointTypes == NULL || gpdata->pointCoords == NULL) {
>if (gpdata->pointTypes != NULL)  free(gpdata->pointTypes);
>if (gpdata->pointCoords != NULL) free(gpdata->pointCoords);
>return 0;
> } else {
>return 1;
> }

Then we would free   as well  for the realloc code path
} else {
/* do we have enough space? */
...
}
Is this okay?

-

PR: https://git.openjdk.java.net/jdk/pull/2250

Re: [OpenJDK 2D-Dev] RFR: JDK-8260432: allocateSpaceForGP in freetypeScaler.c might leak memory

[Aleksey Shipilev]

On Wed, 27 Jan 2021 11:59:29 GMT, Matthias Baesken  wrote:

>> src/java.desktop/share/native/libfontmanager/freetypeScaler.c line 1290:
>> 
>>> 1288: 
>>> 1289: /* failure if any of mallocs failed */
>>> 1290: if (gpdata->pointTypes == NULL || gpdata->pointCoords == NULL)
>> 
>> I think it would be cleaner to free the remaining allocations on the failing 
>> path:
>> 
>> if (gpdata->pointTypes == NULL || gpdata->pointCoords == NULL) {
>>if (gpdata->pointTypes != NULL)  free(gpdata->pointTypes);
>>if (gpdata->pointCoords != NULL) free(gpdata->pointCoords);
>>return 0;
>> } else {
>>return 1;
>> }
>
> Then we would free   as well  for the realloc code path
> } else {
> /* do we have enough space? */
> ...
> }
> Is this okay?

Yes, I think that is better: `realloc` can fail as well, and we don't want to 
leak those?

-

PR: https://git.openjdk.java.net/jdk/pull/2250

Re: [OpenJDK 2D-Dev] RFR: JDK-8260432: allocateSpaceForGP in freetypeScaler.c might leak memory

[Aleksey Shipilev]

On Wed, 27 Jan 2021 08:39:33 GMT, Matthias Baesken  wrote:

> The function  AllocateSpaceForGP in freetypeScaler.c calls potentially 2 
> times malloc ; however the memory is not always freed correctly in case of 
> errors.
> See also the related  sonar issue :
> https://sonarcloud.io/project/issues?id=shipilev_jdk=c=AXck8B_SBBG2CXpcngxr=false=BLOCKER=BUG

Changes requested by shade (Reviewer).

src/java.desktop/share/native/libfontmanager/freetypeScaler.c line 1290:

> 1288: 
> 1289: /* failure if any of mallocs failed */
> 1290: if (gpdata->pointTypes == NULL || gpdata->pointCoords == NULL)

I think it would be cleaner to free the remaining allocations on the failing 
path:

if (gpdata->pointTypes == NULL || gpdata->pointCoords == NULL) {
   if (gpdata->pointTypes != NULL)  free(gpdata->pointTypes);
   if (gpdata->pointCoords != NULL) free(gpdata->pointCoords);
   return 0;
} else {
   return 1;
}

-

PR: https://git.openjdk.java.net/jdk/pull/2250

Re: [OpenJDK 2D-Dev] RFR: 8253795: Implementation of JEP 391: macOS/AArch64 Port [v3]

[Andrew Haley]

On 1/26/21 6:42 PM, erik.joels...@oracle.com wrote:
> My understanding is that Apple chose to not provide JNF for aarch64, so
> if you want to build OpenJDK, you first need to build JNF yourself (it's
> available in github). Phil is working on removing this dependency
> completely, which will solve this issue [1].
>
> In the meantime, I don't think we should rely on finding JNF in
> unsupported locations inside Xcode. Could we wait with integrating this
> port until it can be built without such hacks?

That sounds right to me.

In the meantime, there is some cleanup work to be done in mainline
on slow_signature_handler, which will potentially make the AArch64
back end merge much simpler.

-- 
Andrew Haley  (he/him)
Java Platform Lead Engineer
Red Hat UK Ltd. 
https://keybase.io/andrewhaley
EAC8 43EB D3EF DB98 CC77 2FAD A5CD 6035 332F A671

[OpenJDK 2D-Dev] RFR: JDK-8260432: allocateSpaceForGP in freetypeScaler.c might leak memory

[Matthias Baesken]

The function  AllocateSpaceForGP in freetypeScaler.c calls potentially 2 times 
malloc ; however the memory is not always freed correctly in case of errors.
See also the related  sonar issue :
https://sonarcloud.io/project/issues?id=shipilev_jdk=c=AXck8B_SBBG2CXpcngxr=false=BLOCKER=BUG

-

Commit messages:
 - JDK-8260432

Changes: https://git.openjdk.java.net/jdk/pull/2250/files
 Webrev: https://webrevs.openjdk.java.net/?repo=jdk=2250=00
  Issue: https://bugs.openjdk.java.net/browse/JDK-8260432
  Stats: 4 lines in 1 file changed: 2 ins; 0 del; 2 mod
  Patch: https://git.openjdk.java.net/jdk/pull/2250.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/2250/head:pull/2250

PR: https://git.openjdk.java.net/jdk/pull/2250

Re: [OpenJDK 2D-Dev] RFR: 8253795: Implementation of JEP 391: macOS/AArch64 Port [v3]

[David Holmes]

I don't know why the Skara tools decided to associate my comment with 
Alan Hayward's comment as they are not at all related. :(


David

On 27/01/2021 4:50 pm, David Holmes wrote:

On Tue, 26 Jan 2021 12:34:11 GMT, Alan Hayward 
 wrote:


AIUI, the configure line needs passing a prebuilt JavaNativeFoundation framework
ie:
`--with-extra-ldflags='-F 
/Applications/Xcode.app/Contents/SharedFrameworks/ContentDeliveryServices.framework/Versions/A/itms/java/Frameworks/'`

Otherwise there will be missing _JNFNative* functions.

Is this the long term plan? Or will eventually the required code be moved into 
JDK and/or the xcode one automatically get picked up by the configure scripts?


There is ongoing work by P. Race to eliminate dependence on JNF at all



AIUI, the configure line needs passing a prebuilt JavaNativeFoundation framework
ie:
`--with-extra-ldflags='-F 
/Applications/Xcode.app/Contents/SharedFrameworks/ContentDeliveryServices.framework/Versions/A/itms/java/Frameworks/'`
Otherwise there will be missing _JNFNative* functions.
Is this the long term plan? Or will eventually the required code be moved into 
JDK and/or the xcode one automatically get picked up by the configure scripts?


There is ongoing work by P. Race to eliminate dependence on JNF at all


Ok, that's great.
In the meantime is it worth adding something to the MacOS section of 
doc/building.* ?
(I pieced together the required line from multiple posts in a mailing list)


Hi Anton,

Just to add weight to comments already made by @coleenp and @stefank , I also 
find the W^X coding support to be too intrusive and polluting of the shared 
code. I would much rather see this support pushed down as far as possible, to 
minimise the impact and to use ifdef'd code for macos/Aarch64 (via 
MACOS_AARCH64_ONLY macro) rather than providing empty methods.

Thanks,
David

-

PR: https://git.openjdk.java.net/jdk/pull/2200

Re: [OpenJDK 2D-Dev] RFR: 8253795: Implementation of JEP 391: macOS/AArch64 Port [v6]

[Magnus Ihse Bursie]

On Tue, 26 Jan 2021 21:59:03 GMT, Anton Kozlov  wrote:

>> Please review the implementation of JEP 391: macOS/AArch64 Port.
>> 
>> It's heavily based on existing ports to linux/aarch64, macos/x86_64, and 
>> windows/aarch64. 
>> 
>> Major changes are in:
>> * src/hotspot/cpu/aarch64: support of the new calling convention (subtasks 
>> JDK-8253817, JDK-8253818)
>> * src/hotspot/os_cpu/bsd_aarch64: copy of os_cpu/linux_aarch64 with 
>> necessary adjustments (JDK-8253819)
>> * src/hotspot/share, test/hotspot/gtest: support of write-xor-execute (W^X), 
>> required on macOS/AArch64 platform. It's implemented with 
>> pthread_jit_write_protect_np provided by Apple. The W^X mode is local to a 
>> thread, so W^X mode change relates to the java thread state change (for java 
>> threads). In most cases, JVM executes in write-only mode, except when 
>> calling a generated stub like SafeFetch, which requires a temporary switch 
>> to execute-only mode. The same execute-only mode is enabled when a java 
>> thread executes in java or native states. This approach of managing W^X mode 
>> turned out to be simple and efficient enough.
>> * src/jdk.hotspot.agent: serviceability agent implementation (JDK-8254941)
>
> Anton Kozlov has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   Redo buildsys fix

Build changes per se now looks okay. However, I agree with Erik that unless 
this PR can wait for the JNF removal, at the very least the build docs needs to 
be updated to explain how to successfully build for this platform. (I can live 
with the configure command line hack, since it's temporary -- otherwise I'd 
have requested a new configure argument.) This can be done in this PR or a 
follow-up PR.

-

Marked as reviewed by ihse (Reviewer).

PR: https://git.openjdk.java.net/jdk/pull/2200

Re: [OpenJDK 2D-Dev] RFR: 8253795: Implementation of JEP 391: macOS/AArch64 Port [v3]

[Magnus Ihse Bursie]

On Tue, 26 Jan 2021 22:04:48 GMT, Vladimir Kempik  wrote:

>> make/autoconf/build-aux/autoconf-config.guess line 1275:
>> 
>>> 1273: UNAME_PROCESSOR="aarch64"
>>> 1274: fi
>>> 1275:   fi ;;
>> 
>> Almost, but not quite, correct. We cannot change the autoconf-config.guess 
>> file due to license restrictions (the license allows redistribution, not 
>> modifications). Instead we have the config.guess file which "wraps" 
>> autoconf-config.guess and makes pre-/post-call modifications to work around 
>> limitations in the autoconf original file. So you need to check there if you 
>> are getting incorrect results back and adjust it in that case. See the 
>> already existing clauses in that file.
>
> Hello
> I have updated PR and moved this logic to make/autoconf/build-aux/config.guess
> It's pretty similar to i386 -> x86_64 fix-up on macos_intel

Thanks. That looks better.

-

PR: https://git.openjdk.java.net/jdk/pull/2200

Re: [OpenJDK 2D-Dev] RFR: 8253795: Implementation of JEP 391: macOS/AArch64 Port [v6]

[Magnus Ihse Bursie]

On Mon, 25 Jan 2021 16:00:23 GMT, Bernhard Urban-Forster  
wrote:

>> @luhenry , could you please check this comment, I think SA-agent was MS's 
>> job here.
>
> The target is identified by the header file now:
> https://github.com/openjdk/jdk/pull/2200/files#diff-51442e74eeef2163f0f0643df0ae995083f666367e25fba2b527a9a5bc8743a6R35-R41
> 
> Do you think there is any problem with this approach?

@lewurm No, that's okay. I just wanted to know that this had not been lost.

-

PR: https://git.openjdk.java.net/jdk/pull/2200