[ 
https://issues.apache.org/jira/browse/MNG-7161?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
 ]

Maarten Mulders updated MNG-7161:
---------------------------------
    Description: 
Our integration tests stopped working after we started to test with a Maven 
{{4.0.0-alpha-1}} nightly which included this commit: 
[https://github.com/apache/maven/commit/195fb626a9a4e5a0774f779b6d8da1cb9ef38468#diff-9c5fb3d1b7e3b0f54bc5c4182965c4fe1f9023d449017cece3005d3f90e8e4d8R310-R317]

In this commit the {{maven-shared-utils}} and the {{jansi}} dependencies are 
being upgraded. When we then run our integration tests we get the following 
null pointer exception:
{code:java}
java.lang.NullPointerException
  at org.fusesource.jansi.AnsiPrintStream.uninstall(AnsiPrintStream.java:79)
  at org.fusesource.jansi.AnsiConsole.systemUninstall(AnsiConsole.java:524)
  at 
org.apache.maven.shared.utils.logging.MessageUtils.doSystemUninstall(MessageUtils.java:101)
  at 
org.apache.maven.shared.utils.logging.MessageUtils.systemUninstall(MessageUtils.java:80)
  at org.apache.maven.cli.MavenCli.main(MavenCli.java:203)
  at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke0(Native 
Method)
  at 
java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
  at 
java.base/jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
  at java.base/java.lang.reflect.Method.invoke(Method.java:566)
  at 
org.codehaus.plexus.classworlds.launcher.Launcher.launchEnhanced(Launcher.java:282)
  at org.codehaus.plexus.classworlds.launcher.Launcher.launch(Launcher.java:225)
  at 
org.codehaus.plexus.classworlds.launcher.Launcher.mainWithExitCode(Launcher.java:406)
  at org.codehaus.plexus.classworlds.launcher.Launcher.main(Launcher.java:347)
{code}
 
 We debugged this and [these 
changes|https://github.com/fusesource/jansi/commit/63bd892b2bdfc253ec119a57bdd42df5e80fd859#diff-d59db8655d9ae2d11948e2b411c34fc9e8513f29065d82c978d7128dafbe3bafR414-R420]
 in JAnsi introduced in the above upgraded version, is the source of the 
exception. The NPE is caused because the {{out}} reference is {{null}} at the 
time it wants to uninstall the {{AnsiOutputStream}}. This reference is nulled 
because we use the Plexus interactivity library which [disposes the 
{{DefaultOutputHandler}}|https://github.com/codehaus-plexus/plexus-interactivity/blob/master/plexus-interactivity-api/src/main/java/org/codehaus/plexus/components/interactivity/DefaultOutputHandler.java#L51-L54]
 on the tear down of Plexus, in which the {{System.out}} reference will be 
closed which is in fact the {{out}} reference of the {{AnsiConsole}} JAnsi will 
be [initialized 
before|https://github.com/apache/maven/blob/3e917677e484067b853eaa4a6de44ebcf5a988de/maven-embedder/src/main/java/org/apache/maven/cli/MavenCli.java#L200]
 the Plexus container). This happens 
[here|https://github.com/apache/maven/blob/3e917677e484067b853eaa4a6de44ebcf5a988de/maven-embedder/src/main/java/org/apache/maven/cli/MavenCli.java#L202],
 so before JAnsi will be uninstalled in 
[here|https://github.com/apache/maven/blob/3e917677e484067b853eaa4a6de44ebcf5a988de/maven-embedder/src/main/java/org/apache/maven/cli/MavenCli.java#L203].

There are two options to solve this:
 1. Report this to JAnsi such that they catch this valid use case and do not 
throw as this worked without any exceptions in older versions.
 2. Fix the {{MessageUtils#doSystemUninstall()}} and catch all exceptions and 
swallow them, as if it can't uninstall it, then Maven itself is not capable of 
fixing this state either. This is already done in a similar way 
[here|https://github.com/apache/maven-shared-utils/blob/17091d82508deb9b7067f3434ba16f660ffc5023/src/main/java/org/apache/maven/shared/utils/logging/MessageUtils.java#L85-L92]
 for removing the shutdown hook.

Our proposal is to do #2 which would make Maven itself resilient to such use 
cases as there are other extensions/plugin out there which also retrieve a 
reference for the system output streams and therefore they would fail with 
Maven 4.0.0. This would also make this part future proof, as when there are 
other errors thrown during the uninstall, Maven itself does not break.

We can as well report this to JAnsi too such that this gets fixed there as well.

 

What are your opinions on that?

  was:
Our integration tests stopped working after we started to test with a Maven 
{{4.0.0-alpha-1}} nightly which included this commit: 
[https://github.com/apache/maven/commit/195fb626a9a4e5a0774f779b6d8da1cb9ef38468#diff-9c5fb3d1b7e3b0f54bc5c4182965c4fe1f9023d449017cece3005d3f90e8e4d8R310-R317]

In this commit the {{maven-shared-utils}} and the {{jansi}} dependencies are 
being upgraded. When we then run our integration tests we get the following 
null pointer exception:
{code:java}
java.lang.NullPointerException
  at org.fusesource.jansi.AnsiPrintStream.uninstall(AnsiPrintStream.java:79)
  at org.fusesource.jansi.AnsiConsole.systemUninstall(AnsiConsole.java:524)
  at 
org.apache.maven.shared.utils.logging.MessageUtils.doSystemUninstall(MessageUtils.java:101)
  at 
org.apache.maven.shared.utils.logging.MessageUtils.systemUninstall(MessageUtils.java:80)
  at org.apache.maven.cli.MavenCli.main(MavenCli.java:203)
  at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke0(Native 
Method)
  at 
java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
  at 
java.base/jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
  at java.base/java.lang.reflect.Method.invoke(Method.java:566)
  at 
org.codehaus.plexus.classworlds.launcher.Launcher.launchEnhanced(Launcher.java:282)
  at org.codehaus.plexus.classworlds.launcher.Launcher.launch(Launcher.java:225)
  at 
org.codehaus.plexus.classworlds.launcher.Launcher.mainWithExitCode(Launcher.java:406)
  at org.codehaus.plexus.classworlds.launcher.Launcher.main(Launcher.java:347)
{code}
 
 We debugged this and [these 
changes|https://github.com/fusesource/jansi/commit/63bd892b2bdfc253ec119a57bdd42df5e80fd859#diff-d59db8655d9ae2d11948e2b411c34fc9e8513f29065d82c978d7128dafbe3bafR414-R420]
 in JAnsi introduced in the above upgraded version, is the source of the 
exception. The NPE is caused because the {{out}} reference is {{null}} at the 
time it wants to uninstall the {{AnsiOutputStream}}. This reference is nulled 
because we use the Plexus interactivity library which [disposes the 
{{DefaultOutputHandler}}|https://github.com/codehaus-plexus/plexus-interactivity/blob/master/plexus-interactivity-api/src/main/java/org/codehaus/plexus/components/interactivity/DefaultOutputHandler.java#L51-L54]
 on the tear down of Plexus, in which the {{System.out}} reference will be 
closed which is in fact the {{out}} reference of the {{AnsiConsole (}}JAnsi 
will be[ initialized 
before|https://github.com/apache/maven/blob/3e917677e484067b853eaa4a6de44ebcf5a988de/maven-embedder/src/main/java/org/apache/maven/cli/MavenCli.java#L200]
 the Plexus container). This happens 
[here|https://github.com/apache/maven/blob/3e917677e484067b853eaa4a6de44ebcf5a988de/maven-embedder/src/main/java/org/apache/maven/cli/MavenCli.java#L202],
 so before JAnsi will be uninstalled in 
[here|https://github.com/apache/maven/blob/3e917677e484067b853eaa4a6de44ebcf5a988de/maven-embedder/src/main/java/org/apache/maven/cli/MavenCli.java#L203].

There are two options to solve this:
 1. Report this to JAnsi such that they catch this valid use case and do not 
throw as this worked without any exceptions in older versions.
 2. Fix the {{MessageUtils#doSystemUninstall()}} and catch all exceptions and 
swallow them, as if it can't uninstall it, then Maven itself is not capable of 
fixing this state either. This is already done in a similar way 
[here|https://github.com/apache/maven-shared-utils/blob/17091d82508deb9b7067f3434ba16f660ffc5023/src/main/java/org/apache/maven/shared/utils/logging/MessageUtils.java#L85-L92]
 for removing the shutdown hook.

Our proposal is to do #2 which would make Maven itself resilient to such use 
cases as there are other extensions/plugin out there which also retrieve a 
reference for the system output streams and therefore they would fail with 
Maven 4.0.0. This would also make this part future proof, as when there are 
other errors thrown during the uninstall, Maven itself does not break.

We can as well report this to JAnsi too such that this gets fixed there as well.

 

What are your opinions on that?


> Error thrown during uninstalling of JAnsi
> -----------------------------------------
>
>                 Key: MNG-7161
>                 URL: https://issues.apache.org/jira/browse/MNG-7161
>             Project: Maven
>          Issue Type: Bug
>    Affects Versions: 4.0.x-candidate, 4.0.0, 4.0.0-alpha-1
>            Reporter: Guy Brand
>            Priority: Critical
>
> Our integration tests stopped working after we started to test with a Maven 
> {{4.0.0-alpha-1}} nightly which included this commit: 
> [https://github.com/apache/maven/commit/195fb626a9a4e5a0774f779b6d8da1cb9ef38468#diff-9c5fb3d1b7e3b0f54bc5c4182965c4fe1f9023d449017cece3005d3f90e8e4d8R310-R317]
> In this commit the {{maven-shared-utils}} and the {{jansi}} dependencies are 
> being upgraded. When we then run our integration tests we get the following 
> null pointer exception:
> {code:java}
> java.lang.NullPointerException
>   at org.fusesource.jansi.AnsiPrintStream.uninstall(AnsiPrintStream.java:79)
>   at org.fusesource.jansi.AnsiConsole.systemUninstall(AnsiConsole.java:524)
>   at 
> org.apache.maven.shared.utils.logging.MessageUtils.doSystemUninstall(MessageUtils.java:101)
>   at 
> org.apache.maven.shared.utils.logging.MessageUtils.systemUninstall(MessageUtils.java:80)
>   at org.apache.maven.cli.MavenCli.main(MavenCli.java:203)
>   at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke0(Native 
> Method)
>   at 
> java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
>   at 
> java.base/jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
>   at java.base/java.lang.reflect.Method.invoke(Method.java:566)
>   at 
> org.codehaus.plexus.classworlds.launcher.Launcher.launchEnhanced(Launcher.java:282)
>   at 
> org.codehaus.plexus.classworlds.launcher.Launcher.launch(Launcher.java:225)
>   at 
> org.codehaus.plexus.classworlds.launcher.Launcher.mainWithExitCode(Launcher.java:406)
>   at org.codehaus.plexus.classworlds.launcher.Launcher.main(Launcher.java:347)
> {code}
>  
>  We debugged this and [these 
> changes|https://github.com/fusesource/jansi/commit/63bd892b2bdfc253ec119a57bdd42df5e80fd859#diff-d59db8655d9ae2d11948e2b411c34fc9e8513f29065d82c978d7128dafbe3bafR414-R420]
>  in JAnsi introduced in the above upgraded version, is the source of the 
> exception. The NPE is caused because the {{out}} reference is {{null}} at the 
> time it wants to uninstall the {{AnsiOutputStream}}. This reference is nulled 
> because we use the Plexus interactivity library which [disposes the 
> {{DefaultOutputHandler}}|https://github.com/codehaus-plexus/plexus-interactivity/blob/master/plexus-interactivity-api/src/main/java/org/codehaus/plexus/components/interactivity/DefaultOutputHandler.java#L51-L54]
>  on the tear down of Plexus, in which the {{System.out}} reference will be 
> closed which is in fact the {{out}} reference of the {{AnsiConsole}} JAnsi 
> will be [initialized 
> before|https://github.com/apache/maven/blob/3e917677e484067b853eaa4a6de44ebcf5a988de/maven-embedder/src/main/java/org/apache/maven/cli/MavenCli.java#L200]
>  the Plexus container). This happens 
> [here|https://github.com/apache/maven/blob/3e917677e484067b853eaa4a6de44ebcf5a988de/maven-embedder/src/main/java/org/apache/maven/cli/MavenCli.java#L202],
>  so before JAnsi will be uninstalled in 
> [here|https://github.com/apache/maven/blob/3e917677e484067b853eaa4a6de44ebcf5a988de/maven-embedder/src/main/java/org/apache/maven/cli/MavenCli.java#L203].
> There are two options to solve this:
>  1. Report this to JAnsi such that they catch this valid use case and do not 
> throw as this worked without any exceptions in older versions.
>  2. Fix the {{MessageUtils#doSystemUninstall()}} and catch all exceptions and 
> swallow them, as if it can't uninstall it, then Maven itself is not capable 
> of fixing this state either. This is already done in a similar way 
> [here|https://github.com/apache/maven-shared-utils/blob/17091d82508deb9b7067f3434ba16f660ffc5023/src/main/java/org/apache/maven/shared/utils/logging/MessageUtils.java#L85-L92]
>  for removing the shutdown hook.
> Our proposal is to do #2 which would make Maven itself resilient to such use 
> cases as there are other extensions/plugin out there which also retrieve a 
> reference for the system output streams and therefore they would fail with 
> Maven 4.0.0. This would also make this part future proof, as when there are 
> other errors thrown during the uninstall, Maven itself does not break.
> We can as well report this to JAnsi too such that this gets fixed there as 
> well.
>  
> What are your opinions on that?



--
This message was sent by Atlassian Jira
(v8.3.4#803005)

Reply via email to