You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@netbeans.apache.org by GitBox <gi...@apache.org> on 2021/03/21 20:27:28 UTC

[GitHub] [netbeans] lbruun opened a new pull request #2700: [NETBEANS-1428] Fix for profiler crash - with CI/CD workflow

lbruun opened a new pull request #2700:
URL: https://github.com/apache/netbeans/pull/2700


   This is a fix for [NETBEANS-1428]
   
   This PR replaces #2021.
   This PR builds upon the fixes made by **Peter Hull** (I've cherry-picked commits from PR-2021) but corrects a number of issues, changes copyright from Oracle to ASF and finally adds a GitHub Actions workflow which builds the native binary on Windows, Linux and MacOS in a manner which is consistent, transparent and reproducible. The core component of the fix is still Peter's. Peter analyzed the bug, diagnosed it and came up with a solution (partly based on discussion on the OpenJDK mailing list) so all credit to Peter!!.
   
   ### Background
   The reason for the bug is that the original C code in lib.profiler, which acts as JNI interface, made an incorrect assumption about how to convert from `jint` to `jmethodID` and in doing so effectively relied on an undocumented JVM internal. This assumption was broken with Java 9 on Windows, hence the crash. Peter's fix is to have a HashMap in the C code which can translate from `jint` to `jmethodID`  but only do so on platforms where the two are of different implementation and therefore need conversion.
   
   It can be said that the bug existed on all platforms, but because of some JVM internals only _manifests_ itself on the Windows platform. Peter's fix targets _all_ platforms and this is important as we are then immune to the bug if the JVM in future changes its (undocumented) internals for Linux or MacOS. Therefore it is important to re-build binaries not only for Windows, but for other platforms too.
   
   
   ### Produced artifacts
   The GitHub Workflow added in this PR produce binaries for
   
   - Linux  x86_64 , 32-bit and 64-bit
   - Windows  x86_64,   32-bit and 64-bit
   - MacOS  x86_64,  64-bit only
   
   as all other platforms are now considered unsupported from NetBeans Profiler point of view. 
   
   The GitHub workflow produces a ZIP file, `profiler-external-binaries-ASF.zip` which can be downloaded from the "Artifacts" section of a particular run in the GitHub Actions UI. This ZIP is suitable for upload to https://netbeans.osuosl.org/binaries/.
   
   
   ### Why another PR?
   
   Although the base of the fix in #2021, the C code, was/is fully correct, I felt there were too many changes required to make the PR work (reproducible build, rather than as a one-time lucky punch) and only the PR creator would have permission to make those changes. It could potentially take many ping-pong discussions. Also this PR has a clear view of how the fix can be incorporated into the NetBeans distribution following acceptance of the PR. See below.
   
   ### Testing this PR
   
   or rather : testing the new binaries which are a result of this PR.
   
   1. Be sure that your NetBeans IDE uses a 64-bit JDK. In order to actually see the bug you need to be on Windows and your IDE must be using Java 9 or later. Start a "Methods" Profiling session and you'll immediately experience a crash. (If you are not on this combination then your testing is still valuable because there's a need to verify that the news binaries _still_ works  ... the fix to the C code applies to all 64-bit platforms even if the bug only manifests itself on Windows)
   1. Download the `profiler-external-binaries-ASF.zip` from the "Artifacts" sections of the [GitHub Actions UI](https://github.com/apache/netbeans/actions/runs/508711834) and explode it onto your NetBeans IDE installation directory, folder `netbeans/profiler/` so that files from the ZIP overwrite what you already have on your harddisk. (you should make a backup prior to this of the files that will be overwritten)
   1. Restart the IDE.
   1. Start a "Methods" Profiling session. It must now work.
   
   
   ### Next steps
   
   This is essentially a two-step process which requires not one, but two, PRs. This is the first of those PRs.
   
   Once this PR is accepted the GitHub workflow will run and produce artifact. Someone with proper permission must then upload this to 
   https://netbeans.osuosl.org/binaries/. (naming the file `profiler-external-binaries-ASF.zip`)
   
   I'll then produce a **follow-up PR** which incorporates the new ZIP in the Ant build scripts like this;
   
   Currently the Ant build script looks like this:
   
   ```xml
       <target name="-process.release.files">
           <unzip src="external/profiler-external-binaries-8.2.zip" 
                  overwrite="false"
                  dest="${profiler.cluster}"/>
       </target>
   ```
   
   in the future it will look like this:
   
   ```xml
       <target name="-process.release.files">
           <!-- CDDL licensed binaries, from the Oracle days -->
           <unzip src="external/profiler-external-binaries-8.2.zip" 
                  overwrite="false"
                  dest="${profiler.cluster}"/>
       </target>
           <!-- AL2 licensed binaries, build from ASF code -->
       <target name="-process.release.files">
           <unzip src="external/profiler-external-binaries-ASF.zip" 
                  overwrite="true"
                  dest="${profiler.cluster}"/>
       </target>
   ```
   
   In other words: We retain all esoteric binaries from the CDDL licensed bundle but those are **overlayed** with ones created in ASF (meaning those that are AL2 licensed). The new ASF bundle only include those platforms listed at the top and which are probably the only platforms we can truly support going forward. So for the Windows, MacOS and Linux platforms it will be the binaries from the ASF build which are effectively being used ... and since these are the only ones which has been rebuild it is also the only ones which includes the bug fix for [NETBEANS-1428] but I think that is acceptable as we have no knowledge that the bug will manifest itself on those esoteric platforms. Nor do we have any means of re-building for those platforms. So we leave them in the dust.
   
   
   
   
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For additional commands, e-mail: notifications-help@netbeans.apache.org

For further information about the NetBeans mailing lists, visit:
https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists


[GitHub] [netbeans] lbruun commented on pull request #2700: [NETBEANS-1428] Fix for profiler crash - with CI/CD workflow

Posted by GitBox <gi...@apache.org>.
lbruun commented on pull request #2700:
URL: https://github.com/apache/netbeans/pull/2700#issuecomment-764957436


   > 
   > 
   > A cause for celebration.
   > 
   > > Someone with proper permission must then upload this
   > 
   > So should the internals change in the future, the manual step is required to upload?
   > 
   > > if the JVM in future changes its (undocumented) internals for Linux or MacOS ... as all other platforms are now considered unsupported from NetBeans Profiler
   > 
   > It it correct that these other platforms will continue to work (assuming the internals aren't changed)?
   > 
   > Can you get any reviewers assigned?
   
   


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For additional commands, e-mail: notifications-help@netbeans.apache.org

For further information about the NetBeans mailing lists, visit:
https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists


[GitHub] [netbeans] lbruun commented on pull request #2700: [NETBEANS-1428] Fix for profiler crash - with CI/CD workflow

Posted by GitBox <gi...@apache.org>.
lbruun commented on pull request #2700:
URL: https://github.com/apache/netbeans/pull/2700#issuecomment-812087409


   Merging as per the discussion on the dev mailing list.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For additional commands, e-mail: notifications-help@netbeans.apache.org

For further information about the NetBeans mailing lists, visit:
https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists


[GitHub] [netbeans] pedro-w commented on pull request #2700: [NETBEANS-1428] Fix for profiler crash - with CI/CD workflow

Posted by GitBox <gi...@apache.org>.
pedro-w commented on pull request #2700:
URL: https://github.com/apache/netbeans/pull/2700#issuecomment-812126154


   This is great. Thanks @lbruun !


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For additional commands, e-mail: notifications-help@netbeans.apache.org

For further information about the NetBeans mailing lists, visit:
https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists


[GitHub] [netbeans] pedro-w commented on pull request #2700: [NETBEANS-1428] Fix for profiler crash - with CI/CD workflow

Posted by GitBox <gi...@apache.org>.
pedro-w commented on pull request #2700:
URL: https://github.com/apache/netbeans/pull/2700#issuecomment-764816147


   Big thumbs up @lbruun, I'm very glad you've been able to finish this off properly.


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For additional commands, e-mail: notifications-help@netbeans.apache.org

For further information about the NetBeans mailing lists, visit:
https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists


[GitHub] [netbeans] errael commented on pull request #2700: [NETBEANS-1428] Fix for profiler crash - with CI/CD workflow

Posted by GitBox <gi...@apache.org>.
errael commented on pull request #2700:
URL: https://github.com/apache/netbeans/pull/2700#issuecomment-764803320


   A cause for celebration.
   
   > Someone with proper permission must then upload this
   
   So should the internals change in the future, the manual step is required to upload?
   
   > if the JVM in future changes its (undocumented) internals for Linux or MacOS ... as all other platforms are now considered unsupported from NetBeans Profiler
   
   It it correct that these other platforms will continue to work (assuming the internals aren't changed)?
   
   Can you get any reviewers assigned?


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For additional commands, e-mail: notifications-help@netbeans.apache.org

For further information about the NetBeans mailing lists, visit:
https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists


[GitHub] [netbeans] lbruun closed pull request #2700: [NETBEANS-1428] Fix for profiler crash - with CI/CD workflow

Posted by GitBox <gi...@apache.org>.
lbruun closed pull request #2700:
URL: https://github.com/apache/netbeans/pull/2700


   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For additional commands, e-mail: notifications-help@netbeans.apache.org

For further information about the NetBeans mailing lists, visit:
https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists


[GitHub] [netbeans] lbruun closed pull request #2700: [NETBEANS-1428] Fix for profiler crash - with CI/CD workflow

Posted by GitBox <gi...@apache.org>.
lbruun closed pull request #2700:
URL: https://github.com/apache/netbeans/pull/2700


   


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For additional commands, e-mail: notifications-help@netbeans.apache.org

For further information about the NetBeans mailing lists, visit:
https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists


[GitHub] [netbeans] lbruun commented on pull request #2700: [NETBEANS-1428] Fix for profiler crash - with CI/CD workflow

Posted by GitBox <gi...@apache.org>.
lbruun commented on pull request #2700:
URL: https://github.com/apache/netbeans/pull/2700#issuecomment-764957436


   > 
   > 
   > A cause for celebration.
   > 
   > > Someone with proper permission must then upload this
   > 
   > So should the internals change in the future, the manual step is required to upload?
   > 
   > > if the JVM in future changes its (undocumented) internals for Linux or MacOS ... as all other platforms are now considered unsupported from NetBeans Profiler
   > 
   > It it correct that these other platforms will continue to work (assuming the internals aren't changed)?
   > 
   > Can you get any reviewers assigned?
   
   


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For additional commands, e-mail: notifications-help@netbeans.apache.org

For further information about the NetBeans mailing lists, visit:
https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists


[GitHub] [netbeans] lbruun closed pull request #2700: [NETBEANS-1428] Fix for profiler crash - with CI/CD workflow

Posted by GitBox <gi...@apache.org>.
lbruun closed pull request #2700:
URL: https://github.com/apache/netbeans/pull/2700


   


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For additional commands, e-mail: notifications-help@netbeans.apache.org

For further information about the NetBeans mailing lists, visit:
https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists


[GitHub] [netbeans] errael commented on pull request #2700: [NETBEANS-1428] Fix for profiler crash - with CI/CD workflow

Posted by GitBox <gi...@apache.org>.
errael commented on pull request #2700:
URL: https://github.com/apache/netbeans/pull/2700#issuecomment-764803320


   A cause for celebration.
   
   > Someone with proper permission must then upload this
   
   So should the internals change in the future, the manual step is required to upload?
   
   > if the JVM in future changes its (undocumented) internals for Linux or MacOS ... as all other platforms are now considered unsupported from NetBeans Profiler
   
   It it correct that these other platforms will continue to work (assuming the internals aren't changed)?
   
   Can you get any reviewers assigned?


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For additional commands, e-mail: notifications-help@netbeans.apache.org

For further information about the NetBeans mailing lists, visit:
https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists


[GitHub] [netbeans] pedro-w commented on pull request #2700: [NETBEANS-1428] Fix for profiler crash - with CI/CD workflow

Posted by GitBox <gi...@apache.org>.
pedro-w commented on pull request #2700:
URL: https://github.com/apache/netbeans/pull/2700#issuecomment-764816147


   Big thumbs up @lbruun, I'm very glad you've been able to finish this off properly.


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For additional commands, e-mail: notifications-help@netbeans.apache.org

For further information about the NetBeans mailing lists, visit:
https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists


[GitHub] [netbeans] lbruun merged pull request #2700: [NETBEANS-1428] Fix for profiler crash - with CI/CD workflow

Posted by GitBox <gi...@apache.org>.
lbruun merged pull request #2700:
URL: https://github.com/apache/netbeans/pull/2700


   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For additional commands, e-mail: notifications-help@netbeans.apache.org

For further information about the NetBeans mailing lists, visit:
https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists