Uploaded image for project: 'Jenkins'
  1. Jenkins
  2. JENKINS-24914

Jenkins.doQuietDown(boolean block, int timeout) blocks while jobs running (incorrect doc)

    XMLWordPrintable

    Details

    • Type: Bug
    • Status: Resolved (View Workflow)
    • Priority: Minor
    • Resolution: Fixed
    • Component/s: core
    • Labels:
      None
    • Environment:
      Jenkins ver. 1.565
    • Similar Issues:

      Description

      Jenkins.doQuietDown(boolean block, int timeout) blocks as long as there are jobs running. This is not what it says in the documentation and a less useful behavior in my point of view.

      boolean block
      usage="Block until the system really quiets down and no builds are running"
      int timeout
      usage="If non-zero, only block up to the specified number of milliseconds"

      I suggest the implementation should do what it says in the doc, i.e. only wait timeout, then return even though jobs are still running.

      This error should be easily exposed by a unit test...

        Attachments

          Issue Links

            Activity

            Hide
            danielbeck Daniel Beck added a comment -

            At first glace the code looks fairly straightforward and correct
            https://github.com/jenkinsci/jenkins/blob/stable/core/src/main/java/jenkins/model/Jenkins.java#L2881

            So you're specifying e.g. block=true timeout=10000 for the CLI command, and it waits for much longer than 10 seconds?

            Show
            danielbeck Daniel Beck added a comment - At first glace the code looks fairly straightforward and correct https://github.com/jenkinsci/jenkins/blob/stable/core/src/main/java/jenkins/model/Jenkins.java#L2881 So you're specifying e.g. block=true timeout=10000 for the CLI command, and it waits for much longer than 10 seconds?
            Hide
            marcus_phi Marcus Philip added a comment -

            I agree that the code seems like ot should return. But it doesn't. Here's a test to prove it. The test runs forever.

            The method under test is copied from the real Jenkins class. I just remove the Jenkins singleton stuff to make it simpler, but I think it should not change anything.

            package jenkins.model;
            
            import java.io.IOException;
            
            import org.junit.Test;
            
            public class JenkinsTest {
                private transient volatile boolean isQuietingDown = false;
            
                public void doQuietDown(boolean block,int timeout) throws InterruptedException, IOException {
                    synchronized (this) {
                        isQuietingDown  = true;
                    }
                    if (block) {
                        if (timeout > 0) {
                            timeout += System.currentTimeMillis();
                        }
                        while (isQuietingDown
                               && (timeout <= 0 || System.currentTimeMillis() < timeout)
                               && true) {
                            System.out.println("time: " + System.currentTimeMillis() + " Sleeping ...");
                            Thread.sleep(1000);
                        }
                    }
                    System.out.println("doQuietDown returning");
                }
                @Test
                public void testDoQuietDownBooleanInt() throws Exception {
                    doQuietDown(true, 100);
                }
            }
            
            Show
            marcus_phi Marcus Philip added a comment - I agree that the code seems like ot should return. But it doesn't. Here's a test to prove it. The test runs forever. The method under test is copied from the real Jenkins class. I just remove the Jenkins singleton stuff to make it simpler, but I think it should not change anything. package jenkins.model; import java.io.IOException; import org.junit.Test; public class JenkinsTest { private transient volatile boolean isQuietingDown = false ; public void doQuietDown( boolean block, int timeout) throws InterruptedException, IOException { synchronized ( this ) { isQuietingDown = true ; } if (block) { if (timeout > 0) { timeout += System .currentTimeMillis(); } while (isQuietingDown && (timeout <= 0 || System .currentTimeMillis() < timeout) && true ) { System .out.println( "time: " + System .currentTimeMillis() + " Sleeping ..." ); Thread .sleep(1000); } } System .out.println( "doQuietDown returning" ); } @Test public void testDoQuietDownBooleanInt() throws Exception { doQuietDown( true , 100); } }
            Hide
            danielbeck Daniel Beck added a comment - - edited

            System.currentTimeMillis() doesn't fit into the int, it overflows and is negative (for the next week or so), resulting in an infinite loop.

            Show
            danielbeck Daniel Beck added a comment - - edited System.currentTimeMillis() doesn't fit into the int, it overflows and is negative (for the next week or so), resulting in an infinite loop.
            Hide
            marcus_phi Marcus Philip added a comment -

            Indeed. That's it. Never put currentTimeMillis() in an int... Maybe a unit test for this...

            I did a search through jenkins core and could not find any other incorrect usage of System.currentTimeMillis().

            Show
            marcus_phi Marcus Philip added a comment - Indeed. That's it. Never put currentTimeMillis() in an int... Maybe a unit test for this... I did a search through jenkins core and could not find any other incorrect usage of System.currentTimeMillis().
            Hide
            danielbeck Daniel Beck added a comment - - edited

            Too small (or rather too opaque) to unit test IMO. Fixed locally, will commit this week.

            Show
            danielbeck Daniel Beck added a comment - - edited Too small (or rather too opaque) to unit test IMO. Fixed locally, will commit this week.
            Hide
            scm_issue_link SCM/JIRA link daemon added a comment -

            Code changed in jenkins
            User: Daniel Beck
            Path:
            core/src/main/java/jenkins/model/Jenkins.java
            http://jenkins-ci.org/commit/jenkins/adf8305f8b128c856a6de33409595fe14e01fd73
            Log:
            [FIXED JENKINS-24914] Don't block indefinitely every other month

            Show
            scm_issue_link SCM/JIRA link daemon added a comment - Code changed in jenkins User: Daniel Beck Path: core/src/main/java/jenkins/model/Jenkins.java http://jenkins-ci.org/commit/jenkins/adf8305f8b128c856a6de33409595fe14e01fd73 Log: [FIXED JENKINS-24914] Don't block indefinitely every other month
            Hide
            scm_issue_link SCM/JIRA link daemon added a comment -

            Code changed in jenkins
            User: Oleg Nenashev
            Path:
            core/src/main/java/jenkins/model/Jenkins.java
            http://jenkins-ci.org/commit/jenkins/d045618e0a009cae321f6b048a7507ec0b3b8eac
            Log:
            Merge pull request #1421 from daniel-beck/JENKINS-24914

            [FIXED JENKINS-24914] Don't block indefinitely every other month

            Compare: https://github.com/jenkinsci/jenkins/compare/657d7151b149...d045618e0a00

            Show
            scm_issue_link SCM/JIRA link daemon added a comment - Code changed in jenkins User: Oleg Nenashev Path: core/src/main/java/jenkins/model/Jenkins.java http://jenkins-ci.org/commit/jenkins/d045618e0a009cae321f6b048a7507ec0b3b8eac Log: Merge pull request #1421 from daniel-beck/ JENKINS-24914 [FIXED JENKINS-24914] Don't block indefinitely every other month Compare: https://github.com/jenkinsci/jenkins/compare/657d7151b149...d045618e0a00
            Hide
            dogfood dogfood added a comment -

            Integrated in jenkins_main_trunk #3735
            [FIXED JENKINS-24914] Don't block indefinitely every other month (Revision adf8305f8b128c856a6de33409595fe14e01fd73)

            Result = SUCCESS
            daniel-beck : adf8305f8b128c856a6de33409595fe14e01fd73
            Files :

            • core/src/main/java/jenkins/model/Jenkins.java
            Show
            dogfood dogfood added a comment - Integrated in jenkins_main_trunk #3735 [FIXED JENKINS-24914] Don't block indefinitely every other month (Revision adf8305f8b128c856a6de33409595fe14e01fd73) Result = SUCCESS daniel-beck : adf8305f8b128c856a6de33409595fe14e01fd73 Files : core/src/main/java/jenkins/model/Jenkins.java

              People

              Assignee:
              danielbeck Daniel Beck
              Reporter:
              marcus_phi Marcus Philip
              Votes:
              0 Vote for this issue
              Watchers:
              3 Start watching this issue

                Dates

                Created:
                Updated:
                Resolved: