Re: RFR: 8308748: JNU_GetStringPlatformChars may write to String's internal memory array [v2]
On Fri, 26 May 2023 19:54:07 GMT, Rudi Horn wrote: >> This change prevents the contents of the internal string array from being >> copied back when releasing it. > > Rudi Horn has updated the pull request with a new target base due to a merge > or a rebase. The pull request now contains one commit: > > Use JNI_ABORT to release string bytes Marked as reviewed by alanb (Reviewer). - PR Review: https://git.openjdk.org/jdk/pull/14117#pullrequestreview-1454646625
Re: RFR: 8308748: JNU_GetStringPlatformChars may write to String's internal memory array [v2]
On Wed, 31 May 2023 11:49:00 GMT, Rudi Horn wrote: > I have not checked other uses, but they are probably not as critical yet > given the special status and frequent use of strings. I can possibly revisit > this issue in the future in a further issue / PR if I find time at some stage. Okay, we should create an issue in JBS to follow-up on that, as there may be other opportunities to avoid the copy back. - PR Comment: https://git.openjdk.org/jdk/pull/14117#issuecomment-1571415559
Re: RFR: 8308748: JNU_GetStringPlatformChars may write to String's internal memory array [v2]
On Fri, 26 May 2023 19:54:07 GMT, Rudi Horn wrote: >> This change prevents the contents of the internal string array from being >> copied back when releasing it. > > Rudi Horn has updated the pull request with a new target base due to a merge > or a rebase. The pull request now contains one commit: > > Use JNI_ABORT to release string bytes Looks good for core-libs. - Marked as reviewed by rriggs (Reviewer). PR Review: https://git.openjdk.org/jdk/pull/14117#pullrequestreview-1454378418
Re: RFR: 8308748: JNU_GetStringPlatformChars may write to String's internal memory array [v2]
On Fri, 26 May 2023 19:54:07 GMT, Rudi Horn wrote: >> This change prevents the contents of the internal string array from being >> copied back when releasing it. > > Rudi Horn has updated the pull request with a new target base due to a merge > or a rebase. The pull request now contains one commit: > > Use JNI_ABORT to release string bytes This change looks good to me. Please await a core-libs review before integration though. Thanks. - Marked as reviewed by dholmes (Reviewer). PR Review: https://git.openjdk.org/jdk/pull/14117#pullrequestreview-1454299683
Re: RFR: 8308748: JNU_GetStringPlatformChars may write to String's internal memory array [v2]
On Fri, 26 May 2023 19:54:07 GMT, Rudi Horn wrote: >> This change prevents the contents of the internal string array from being >> copied back when releasing it. > > Rudi Horn has updated the pull request with a new target base due to a merge > or a rebase. The pull request now contains one commit: > > Use JNI_ABORT to release string bytes I have not checked other uses, but they are probably not as critical yet given the special status and frequent use of strings. I can possibly revisit this issue in the future in a further issue / PR if I find time at some stage. - PR Comment: https://git.openjdk.org/jdk/pull/14117#issuecomment-1570043153
Re: RFR: 8308748: JNU_GetStringPlatformChars may write to String's internal memory array [v2]
On Sat, 27 May 2023 14:21:28 GMT, Alan Bateman wrote: > This looks okay. Have you checked the other usages of > ReleasePrimitiveArrayCritical (e.g. libzip) see if they need the copy back > when releasing? I can immediately see code in `Java_java_util_zip_Inflater_inflateBytesBytes` that needs the same fix in relation to the input array. Even if there is no issue with read-only memory when writing back an unchanged array, we should avoid unnecessary copying. That said I'm not sure it is up to this issue/PR to take responsibility for policing all uses of `ReleasePrimitiveArrayCritical` in the JDK libraries. - PR Comment: https://git.openjdk.org/jdk/pull/14117#issuecomment-1566418737
Re: RFR: 8308748: JNU_GetStringPlatformChars may write to String's internal memory array [v2]
On Fri, 26 May 2023 19:54:07 GMT, Rudi Horn wrote: >> This change prevents the contents of the internal string array from being >> copied back when releasing it. > > Rudi Horn has updated the pull request with a new target base due to a merge > or a rebase. The pull request now contains one commit: > > Use JNI_ABORT to release string bytes This looks okay. Have you checked the other usages of ReleasePrimitiveArrayCritical (e.g. libzip) see if they need the copy back when releasing? - PR Comment: https://git.openjdk.org/jdk/pull/14117#issuecomment-1565450604
Re: RFR: 8308748: JNU_GetStringPlatformChars may write to String's internal memory array [v2]
> This change prevents the contents of the internal string array from being > copied back when releasing it. Rudi Horn has updated the pull request with a new target base due to a merge or a rebase. The pull request now contains one commit: Use JNI_ABORT to release string bytes - Changes: https://git.openjdk.org/jdk/pull/14117/files Webrev: https://webrevs.openjdk.org/?repo=jdk&pr=14117&range=01 Stats: 3 lines in 1 file changed: 0 ins; 0 del; 3 mod Patch: https://git.openjdk.org/jdk/pull/14117.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/14117/head:pull/14117 PR: https://git.openjdk.org/jdk/pull/14117
