Uploaded image for project: 'Tools (JBoss Tools)'
  1. Tools (JBoss Tools)
  2. JBIDE-9650

[Regression] - Servers under 7 (6 and below) do not suspend deployment scanner before publishing

    Details

      Description

      A critical regression has been discovered while investigating the ability to delay the popup of the web browser until the server is finished deploying.

      A commit was pushed in on July 7th, 59 days ago, reversing the logic that was used to determine if the deployment scanner should be stopped or not. The original (and intended) behaviour was that if the server is started, and a JMX extension is provided, to disable the scanner during publish events.

      The offending commit is here:
      http://source.jboss.org/changelog/JBossTools?cs=32709

      You can see that the logic was removed from one class to another. LocalJBossBehaviorDelegate had the following code:
      protected boolean shouldSuspendScanner()

      { return getActualBehavior().shouldSuspendScanner() && ExtensionManager.getDefault().getJMXRunner() != null; }

      The code in the "actual behaviour" had the following:
      if( getServer().getServerState() != IServer.STATE_STARTED)
      return false;

      The code in LocalJBossBehaviourDelegate was replaced with:
      protected boolean shouldSuspendScanner()

      { return getServer().getServerState() != IServer.STATE_STARTED && ExtensionManager.getDefault().getJMXRunner() != null; }

      You can see that the original code ensured that if the server state was NOT started, it would NOT suspend JMX... ie, that the server being started is a requirement to suspend JMX deployment scanner. In the new code, you can see that it will TRY to suspend if the server is NOT started, and that if the server IS started, it WILL NOT try to suspend.

      It is extremely obvious that no testing for this usecase was performed during the time period around this patch.

        Gliffy Diagrams

          Activity

          Hide
          rob.stryker Rob Stryker added a comment -

          fix was released to m4. Sucks that this exists in m3. #&$(@)@$#@*Đ™©¥£şç€

          Show
          rob.stryker Rob Stryker added a comment - fix was released to m4. Sucks that this exists in m3. #& $ (@)@$#@*Đ™©¥£şç€
          Hide
          maxandersen Max Rydahl Andersen added a comment -

          reopening for reval for M3

          Show
          maxandersen Max Rydahl Andersen added a comment - reopening for reval for M3
          Hide
          rob.stryker Rob Stryker added a comment -

          Patch for branch

          Show
          rob.stryker Rob Stryker added a comment - Patch for branch
          Hide
          rob.stryker Rob Stryker added a comment -

          committed to 3.3.0.m3 branch

          Show
          rob.stryker Rob Stryker added a comment - committed to 3.3.0.m3 branch
          Hide
          lzoubek Libor Zoubek added a comment -

          I am not able to reproduce this regression on M3 build eariler than 5th sep .2011. i was trying it by simply deploying larger project (seam 2) to EAP 5. From my point of view behaviour is same as with your patch applied.

          Rob, could you suggest better testing scenario?

          Show
          lzoubek Libor Zoubek added a comment - I am not able to reproduce this regression on M3 build eariler than 5th sep .2011. i was trying it by simply deploying larger project (seam 2) to EAP 5. From my point of view behaviour is same as with your patch applied. Rob, could you suggest better testing scenario?
          Hide
          lzoubek Libor Zoubek added a comment -

          verified by not being able to reproduce regression, examining the patch and trusting Rob

          Show
          lzoubek Libor Zoubek added a comment - verified by not being able to reproduce regression, examining the patch and trusting Rob

            People

            • Assignee:
              lzoubek Libor Zoubek
              Reporter:
              rob.stryker Rob Stryker
            • Votes:
              0 Vote for this issue
              Watchers:
              1 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development