Re: RFR: 8331977: Crash: SIGSEGV in dlerror()
On Tue, 4 Jun 2024 12:49:18 GMT, Alexey Semenyuk wrote: > We already have [JDK-8263466](https://bugs.openjdk.org/browse/JDK-8263466) to > find the original crash. Thanks, just making sure the issue that there is an issue tracking it as it's a bit unsettling that a re-run may be hiding something serious - PR Comment: https://git.openjdk.org/jdk/pull/19502#issuecomment-2147467566
Re: RFR: 8331977: Crash: SIGSEGV in dlerror()
On Tue, 4 Jun 2024 06:43:12 GMT, Alan Bateman wrote: > If I read JDK-8269403 correctly then there is an issue somewhere that hasn't > been diagnosed. Correct. > A workaround has been put in to "re-run" when there is a crash, thus hiding > the issue. Correct. > Are there follow-up issues created in JBS to continue the hunt for the > original crash? The issue was first reported in [JDK-8263466](https://bugs.openjdk.org/browse/JDK-8263466). It was "fixed" by rerunning the launcher. The fix didn't cover all the scenarios of launcher executions in jpackage tests. One of these uncovered scenarios caused the issue of this PR. We already have [JDK-8263466](https://bugs.openjdk.org/browse/JDK-8263466) to find the original crash. - PR Comment: https://git.openjdk.org/jdk/pull/19502#issuecomment-2147454916
Re: RFR: 8331977: Crash: SIGSEGV in dlerror()
On Fri, 31 May 2024 14:05:07 GMT, Alexey Semenyuk wrote: > Fix MainClassTest class to use HelloApp.AppOutputVerifier class to run app > launcher instead of raw Executor. This makes MainClassTest test run app > launchers with retries. This change addresses the primary issue. > > Fix inconsistencies in HelloApp.AppOutputVerifier class. It used to provide > API allowing to run launchers without retries. It inconsistently allowed the > execution of launchers with suppressed output (stdout and stderr). It > inconsistently executed launchers with/without PATH removed from the > environment. > > These loopholes were eliminated: > > - stdout and stderr of app launchers is never suppressed; > - PATH env variable is always deleted for app launchers on Windows. It is > not deleted on other platforms. This change sets the correct scope of > [JDK-8254920](https://bugs.openjdk.org/browse/JDK-8254920) fix that > introduced the removal of PATH env variable for app launchers; > - app launchers are always executed with retries unless the launcher is > executed with `jpackage.test.noexit` system property set to `true` indicating > the test app will not terminate on its own. > > Other changes are due to changes in HelloApp.AppOutputVerifier class. There is [JDK-8263466](https://bugs.openjdk.org/browse/JDK-8263466) with the same symptoms to be investigated. It is linked to this bug. - PR Comment: https://git.openjdk.org/jdk/pull/19502#issuecomment-2147440644
Re: RFR: 8331977: Crash: SIGSEGV in dlerror()
On Mon, 3 Jun 2024 14:38:03 GMT, Alexey Semenyuk wrote: >> I am yet to see anything that actually explains the cause of the `dlerror` >> crash here ??? > > @dholmes-ora there is no fix for the cause of the `dlerror` crash in this PR. > The PR fixes jpackage tests to rerun a launcher if it crashes. This > workaround for jpackage tests was first introduced in > [JDK-8269403](https://bugs.openjdk.org/browse/JDK-8269403) fix but > MainClassTest was left unfixed back then. This PR complements > [JDK-8269403](https://bugs.openjdk.org/browse/JDK-8269403). @alexeysemenyukoracle If I read JDK-8269403 correctly then there is an issue somewhere that hasn't been diagnosed. A workaround has been put in to "re-run" when there is a crash, thus hiding the issue. Are there follow-up issues created in JBS to continue the hunt for the original crash? - PR Comment: https://git.openjdk.org/jdk/pull/19502#issuecomment-2146731836
Re: RFR: 8331977: Crash: SIGSEGV in dlerror()
On Mon, 3 Jun 2024 14:38:03 GMT, Alexey Semenyuk wrote: >> I am yet to see anything that actually explains the cause of the `dlerror` >> crash here ??? > > @dholmes-ora there is no fix for the cause of the `dlerror` crash in this PR. > The PR fixes jpackage tests to rerun a launcher if it crashes. This > workaround for jpackage tests was first introduced in > [JDK-8269403](https://bugs.openjdk.org/browse/JDK-8269403) fix but > MainClassTest was left unfixed back then. This PR complements > [JDK-8269403](https://bugs.openjdk.org/browse/JDK-8269403). @alexeysemenyukoracle But now there is no bug to investigate the dlerrror crash! If you wanted to make the test more robust a new JBS issue should have been filed for that. - PR Comment: https://git.openjdk.org/jdk/pull/19502#issuecomment-2146669916
Re: RFR: 8331977: Crash: SIGSEGV in dlerror()
On Mon, 3 Jun 2024 08:37:50 GMT, David Holmes wrote: >> Fix MainClassTest class to use HelloApp.AppOutputVerifier class to run app >> launcher instead of raw Executor. This makes MainClassTest test run app >> launchers with retries. This change addresses the primary issue. >> >> Fix inconsistencies in HelloApp.AppOutputVerifier class. It used to provide >> API allowing to run launchers without retries. It inconsistently allowed the >> execution of launchers with suppressed output (stdout and stderr). It >> inconsistently executed launchers with/without PATH removed from the >> environment. >> >> These loopholes were eliminated: >> >> - stdout and stderr of app launchers is never suppressed; >> - PATH env variable is always deleted for app launchers on Windows. It is >> not deleted on other platforms. This change sets the correct scope of >> [JDK-8254920](https://bugs.openjdk.org/browse/JDK-8254920) fix that >> introduced the removal of PATH env variable for app launchers; >> - app launchers are always executed with retries unless the launcher is >> executed with `jpackage.test.noexit` system property set to `true` >> indicating the test app will not terminate on its own. >> >> Other changes are due to changes in HelloApp.AppOutputVerifier class. > > I am yet to see anything that actually explains the cause of the `dlerror` > crash here ??? @dholmes-ora there is no fix for the cause of the `dlerror` crash in this PR. The PR fixes jpackage tests to rerun a launcher if it crashes. This workaround for jpackage tests was first introduced in [JDK-8269403](https://bugs.openjdk.org/browse/JDK-8269403) fix but MainClassTest was left unfixed back then. This PR complements [JDK-8269403](https://bugs.openjdk.org/browse/JDK-8269403). - PR Comment: https://git.openjdk.org/jdk/pull/19502#issuecomment-2145373568
Re: RFR: 8331977: Crash: SIGSEGV in dlerror()
On Fri, 31 May 2024 14:05:07 GMT, Alexey Semenyuk wrote: > Fix MainClassTest class to use HelloApp.AppOutputVerifier class to run app > launcher instead of raw Executor. This makes MainClassTest test run app > launchers with retries. This change addresses the primary issue. > > Fix inconsistencies in HelloApp.AppOutputVerifier class. It used to provide > API allowing to run launchers without retries. It inconsistently allowed the > execution of launchers with suppressed output (stdout and stderr). It > inconsistently executed launchers with/without PATH removed from the > environment. > > These loopholes were eliminated: > > - stdout and stderr of app launchers is never suppressed; > - PATH env variable is always deleted for app launchers on Windows. It is > not deleted on other platforms. This change sets the correct scope of > [JDK-8254920](https://bugs.openjdk.org/browse/JDK-8254920) fix that > introduced the removal of PATH env variable for app launchers; > - app launchers are always executed with retries unless the launcher is > executed with `jpackage.test.noexit` system property set to `true` indicating > the test app will not terminate on its own. > > Other changes are due to changes in HelloApp.AppOutputVerifier class. I am yet to see anything that actually explains the cause of the `dlerror` crash here ??? - PR Comment: https://git.openjdk.org/jdk/pull/19502#issuecomment-2144616488
Re: RFR: 8331977: Crash: SIGSEGV in dlerror()
On Fri, 31 May 2024 14:05:07 GMT, Alexey Semenyuk wrote: > Fix MainClassTest class to use HelloApp.AppOutputVerifier class to run app > launcher instead of raw Executor. This makes MainClassTest test run app > launchers with retries. This change addresses the primary issue. > > Fix inconsistencies in HelloApp.AppOutputVerifier class. It used to provide > API allowing to run launchers without retries. It inconsistently allowed the > execution of launchers with suppressed output (stdout and stderr). It > inconsistently executed launchers with/without PATH removed from the > environment. > > These loopholes were eliminated: > > - stdout and stderr of app launchers is never suppressed; > - PATH env variable is always deleted for app launchers on Windows. It is > not deleted on other platforms. This change sets the correct scope of > [JDK-8254920](https://bugs.openjdk.org/browse/JDK-8254920) fix that > introduced the removal of PATH env variable for app launchers; > - app launchers are always executed with retries unless the launcher is > executed with `jpackage.test.noexit` system property set to `true` indicating > the test app will not terminate on its own. > > Other changes are due to changes in HelloApp.AppOutputVerifier class. Looks good. - Marked as reviewed by almatvee (Reviewer). PR Review: https://git.openjdk.org/jdk/pull/19502#pullrequestreview-2091922907
RFR: 8331977: Crash: SIGSEGV in dlerror()
Fix MainClassTest class to use HelloApp.AppOutputVerifier class to run app launcher instead of raw Executor. This makes MainClassTest test run app launchers with retries. This change addresses the primary issue. Fix inconsistencies in HelloApp.AppOutputVerifier class. It used to provide API allowing to run launchers without retries. It inconsistently allowed the execution of launchers with suppressed output (stdout and stderr). It inconsistently executed launchers with/without PATH removed from the environment. These loopholes were eliminated: - stdout and stderr of app launchers is never suppressed; - PATH env variable is always deleted for app launchers on Windows. It is not deleted on other platforms. This change sets the correct scope of [JDK-8254920](https://bugs.openjdk.org/browse/JDK-8254920) fix that introduced the removal of PATH env variable for app launchers; - app launchers are always executed with retries unless the launcher is executed with `jpackage.test.noexit` system property set to `true` indicating the test app will not terminate on its own. Other changes are due to changes in HelloApp.AppOutputVerifier class. - Commit messages: - Trailing spaces removed. - Run app launcher with retries if "jpackage.test.noexit" jpackage java option is not set or set to "false". Otherwise run app launcher without retries. If the launcher never exits (jpackage.test.noexit=true), it should be terminated externally and thus it should not be restarted. - 8331977: Crash: SIGSEGV in dlerror() Changes: https://git.openjdk.org/jdk/pull/19502/files Webrev: https://webrevs.openjdk.org/?repo=jdk&pr=19502&range=00 Issue: https://bugs.openjdk.org/browse/JDK-8331977 Stats: 122 lines in 5 files changed: 48 ins; 41 del; 33 mod Patch: https://git.openjdk.org/jdk/pull/19502.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/19502/head:pull/19502 PR: https://git.openjdk.org/jdk/pull/19502
Re: RFR: 8331977: Crash: SIGSEGV in dlerror()
On Fri, 31 May 2024 14:05:07 GMT, Alexey Semenyuk wrote: > Fix MainClassTest class to use HelloApp.AppOutputVerifier class to run app > launcher instead of raw Executor. This makes MainClassTest test run app > launchers with retries. This change addresses the primary issue. > > Fix inconsistencies in HelloApp.AppOutputVerifier class. It used to provide > API allowing to run launchers without retries. It inconsistently allowed the > execution of launchers with suppressed output (stdout and stderr). It > inconsistently executed launchers with/without PATH removed from the > environment. > > These loopholes were eliminated: > > - stdout and stderr of app launchers is never suppressed; > - PATH env variable is always deleted for app launchers on Windows. It is > not deleted on other platforms. This change sets the correct scope of > [JDK-8254920](https://bugs.openjdk.org/browse/JDK-8254920) fix that > introduced the removal of PATH env variable for app launchers; > - app launchers are always executed with retries unless the launcher is > executed with `jpackage.test.noexit` system property set to `true` indicating > the test app will not terminate on its own. > > Other changes are due to changes in HelloApp.AppOutputVerifier class. @sashamatveev please review - PR Comment: https://git.openjdk.org/jdk/pull/19502#issuecomment-2143129082
