Uploaded image for project: 'Maven'
  1. Maven
  2. MNG-6170

NPE in cases using Multithreaded -T X versions:set -DnewVersion=1.0-SNAPSHOT

    Details

    • Type: Bug
    • Status: Closed
    • Priority: Major
    • Resolution: Fixed
    • Affects Version/s: 3.1.1, 3.2.5, 3.3.1, 3.3.9
    • Fix Version/s: 3.5.0-beta-1, 3.5.0
    • Component/s: core
    • Labels:
      None

      Description

      Based on the issue marked for the versions-maven-plugin investigation shows that the real cause of this problem is located in maven-core.

      The short description of the problem is calling Maven via: mvn -T 20 versions:set -DnewVersion=1.0-SNAPSHOT you will get an NPE.

      I identified the following point in code as culprit for the problem:

      MultiThreadedBuilder.java

              // for each finished project
              for ( int i = 0; i < analyzer.getNumberOfBuilds(); i++ )
              {
                  try
                  {
                      ProjectSegment projectBuild = service.take().get();
                      if ( reactorContext.getReactorBuildStatus().isHalted() )
                      {
                          break;
                      }
                      final List<MavenProject> newItemsThatCanBeBuilt =
                          analyzer.markAsFinished( projectBuild.getProject() );
                      for ( MavenProject mavenProject : newItemsThatCanBeBuilt )
                      {
                          ProjectSegment scheduledDependent = projectBuildList.get( mavenProject );
                          logger.debug( "Scheduling: " + scheduledDependent );
                          Callable<ProjectSegment> cb =
                              createBuildCallable( rootSession, scheduledDependent, reactorContext, taskSegment, muxer );
                          service.submit( cb );
                      }
                  }
                  catch ( InterruptedException e )
                  {
                      rootSession.getResult().addException( e );
                      break;
                  }
                  catch ( ExecutionException e )
                  {
                      // TODO MNG-5766 changes likely made this redundant 
                      rootSession.getResult().addException( e );
                      break;
                  }
              }
      

      And the problematic part is before the second debugging output line:

                          ProjectSegment scheduledDependent = projectBuildList.get( mavenProject );
                          logger.debug( "Scheduling: " + scheduledDependent );
                          Callable<ProjectSegment> cb =
                              createBuildCallable( rootSession, scheduledDependent, reactorContext, taskSegment, muxer );
                          service.submit( cb );
      

      Cause it happens that the scheduledDependent could be null which will cause the issue.
      This looks like is a regression, cause in Maven 3.0.5 it works without any issue.

      Update:
      I have found other examples where the NPE occurs:

      • mvn -T20 javadoc:aggregate
      • mvn -T20 install:install-file ...
        This can cause confusion if you are using mvn.config and put thing like -T X into it.

        Activity

        Hide
        hboutemy Hervé Boutemy added a comment - - edited

        why priority blocker?
        this has not blocked many people from using Maven 3.1.0 to 3.3.9, isn't it?
        and no real pressure to not release 3.5.0 before this is fixed, no?

        notice: using versions:set in parallel mode is really looking for trouble

        Show
        hboutemy Hervé Boutemy added a comment - - edited why priority blocker? this has not blocked many people from using Maven 3.1.0 to 3.3.9, isn't it? and no real pressure to not release 3.5.0 before this is fixed, no? notice: using versions:set in parallel mode is really looking for trouble
        Hide
        khmarbaise Karl Heinz Marbaise added a comment -

        Using versions:set with -T X could mean to run versions:set parallel (Currently not possible but could improve the run time for larger projects).

        Show
        khmarbaise Karl Heinz Marbaise added a comment - Using versions:set with -T X could mean to run versions:set parallel (Currently not possible but could improve the run time for larger projects).
        Hide
        khmarbaise Karl Heinz Marbaise added a comment -

        So after more detailed analysis:

        The build() Method of MultiThreadedBuilder has a projectDependencyGraph which which lists all projects whereas the projectBuildMap 

        Map<MavenProject, ProjectSegment> projectBuildMap = projectBuilds.selectSegment( taskSegment );
        

        only contains the projects which should be built. In case calling mvn -T X versions:set .. is only the root module. Inside the method multiThreadedProjectTaskSegmentBuild the root module will be scheduled for execution which looks correct so far.

                // schedule independent projects
                for ( MavenProject mavenProject : analyzer.getRootSchedulableBuilds() )
                {
                    ProjectSegment projectSegment = projectBuildList.get( mavenProject );
                    logger.debug( "Scheduling: " + projectSegment.getProject() );
                    Callable<ProjectSegment> cb =
                        createBuildCallable( rootSession, projectSegment, reactorContext, taskSegment, muxer );
                    service.submit( cb );
                }
        

        The second part is:

                for ( int i = 0; i < analyzer.getNumberOfBuilds(); i++ )
                {
                    try
                    {
                        ProjectSegment projectBuild = service.take().get();
                        if ( reactorContext.getReactorBuildStatus().isHalted() )
                        {
                            break;
                        }
                        final List<MavenProject> newItemsThatCanBeBuilt = analyzer.markAsFinished( projectBuild.getProject() );
                        for ( MavenProject mavenProject : newItemsThatCanBeBuilt )
                        {
        ...
                        }
        

        At the moment it looks like the the information of newItemsThatCanBeBuilt is simply wrong in such cases, cause it will return an non empty list where it should be empty where only the root module should be built.

        Show
        khmarbaise Karl Heinz Marbaise added a comment - So after more detailed analysis: The build() Method of MultiThreadedBuilder has a projectDependencyGraph which which lists all projects whereas the projectBuildMap   Map<MavenProject, ProjectSegment> projectBuildMap = projectBuilds.selectSegment( taskSegment ); only contains the projects which should be built. In case calling mvn -T X versions:set .. is only the root module. Inside the method multiThreadedProjectTaskSegmentBuild the root module will be scheduled for execution which looks correct so far. // schedule independent projects for ( MavenProject mavenProject : analyzer.getRootSchedulableBuilds() ) { ProjectSegment projectSegment = projectBuildList.get( mavenProject ); logger.debug( "Scheduling: " + projectSegment.getProject() ); Callable<ProjectSegment> cb = createBuildCallable( rootSession, projectSegment, reactorContext, taskSegment, muxer ); service.submit( cb ); } The second part is: for ( int i = 0; i < analyzer.getNumberOfBuilds(); i++ ) { try { ProjectSegment projectBuild = service.take().get(); if ( reactorContext.getReactorBuildStatus().isHalted() ) { break ; } final List<MavenProject> newItemsThatCanBeBuilt = analyzer.markAsFinished( projectBuild.getProject() ); for ( MavenProject mavenProject : newItemsThatCanBeBuilt ) { ... } At the moment it looks like the the information of newItemsThatCanBeBuilt  is simply wrong in such cases, cause it will return an non empty list where it should be empty where only the root module should be built.
        Hide
        khmarbaise Karl Heinz Marbaise added a comment - - edited

        After more diving into this:

        If I add the following condition this will also prevent the NPE being thrown but I'm not sure if this also just covers the problem.

                        if (analyzer.getNumberOfBuilds() > 1) {
                            final List<MavenProject> newItemsThatCanBeBuilt = analyzer.markAsFinished( projectBuild.getProject() );
                            for ( MavenProject mavenProject : newItemsThatCanBeBuilt )
                            {
        

        In the end it looks like the markAsFinished method is returning wrong results. I'm currently testing with a multi module project:

        • root
        • app
        • asm
        • webgui
        • service
        • service-client
        • domain
        • shade

        webgui has a dependency to service-client and service-client has a dependency to domain. So the method markAsFinished returns domain in the case to build only the root module which is correct from the dependency point of view.

        So markAsFinished returns the project which should be built next based on the reactor build order...So the edge case having to build only a single module (root) seemed to be the real problem here. So introducing the condition if (analyzer.getNumberOfBuilds() > 1) looks correct to me...

        Show
        khmarbaise Karl Heinz Marbaise added a comment - - edited After more diving into this: If I add the following condition this will also prevent the NPE being thrown but I'm not sure if this also just covers the problem. if (analyzer.getNumberOfBuilds() > 1) { final List<MavenProject> newItemsThatCanBeBuilt = analyzer.markAsFinished( projectBuild.getProject() ); for ( MavenProject mavenProject : newItemsThatCanBeBuilt ) { In the end it looks like the markAsFinished method is returning wrong results. I'm currently testing with a multi module project : root app asm webgui service service-client domain shade webgui has a dependency to service-client and service-client has a dependency to domain . So the method markAsFinished returns domain in the case to build only the root module which is correct from the dependency point of view. So markAsFinished returns the project which should be built next based on the reactor build order...So the edge case having to build only a single module (root) seemed to be the real problem here. So introducing the condition if (analyzer.getNumberOfBuilds() > 1) looks correct to me...
        Hide
        rfscholte Robert Scholte added a comment -

        Could MNG-6173 be related?

        Show
        rfscholte Robert Scholte added a comment - Could MNG-6173 be related?
        Hide
        khmarbaise Karl Heinz Marbaise added a comment -

        Not that I see a relationship...here...

        Show
        khmarbaise Karl Heinz Marbaise added a comment - Not that I see a relationship...here...
        Show
        khmarbaise Karl Heinz Marbaise added a comment - Done in 16f69fcb9e408fdc42bc038911bcd37ccae8fc38

          People

          • Assignee:
            khmarbaise Karl Heinz Marbaise
            Reporter:
            khmarbaise Karl Heinz Marbaise
          • Votes:
            1 Vote for this issue
            Watchers:
            7 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development