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

AsyncFutureImpl#get(timeout) implementation may throw TimeoutException before the timeout time passes

    XMLWordPrintable

    Details

    • Type: Bug
    • Status: Resolved (View Workflow)
    • Priority: Critical
    • Resolution: Fixed
    • Component/s: remoting
    • Labels:
      None
    • Similar Issues:

      Description

      It happens, because "wait(timeout)" is not in the loop. Object#wait(long) is explicit about that: "A thread can also wake up without being notified, interrupted, or timing out, a so-called spurious wakeup. While this will rarely occur in practice, applications must guard against it by testing for the condition that should have caused the thread to be awakened, and continuing to wait if the condition is not satisfied. In other words, waits should always occur in loops..."

      https://docs.oracle.com/javase/8/docs/api/java/lang/Object.html#wait-long-

      Impact:

      • AsyncFutureImpl is being used explicitly in JarCacheSupport. Jar resolution with a timeout may fail due to the issue
      • The class is overridden in Jenkins Core's public API, e.g. hudson.model.queue.FutureImpl. It's hard to estimate risks, but it looks bad.

        Attachments

          Issue Links

            Activity

            Hide
            scm_issue_link SCM/JIRA link daemon added a comment -

            Code changed in jenkins
            User: Oleg Nenashev
            Path:
            src/main/java/hudson/remoting/AsyncFutureImpl.java
            http://jenkins-ci.org/commit/remoting/3f07563743ef1874057eeb17e04847170436b39f
            Log:
            JENKINS-48309 - Prevent TimeoutException in AsyncFutureImpl in the case of spurious wakeups (#240)

            • JENKINS-48309 - Prevent TimeoutException in AsyncFutureImpl in the case of spurious wakeups.

            TL;DR: A large part of Jenkins’ get-with-timeout logic is a subject for improper exit before the timeout.

            • JENKINS-48309 - I was too lazy to use nanos in the beginning, address the comment from @stephenc
            • JENKINS-48309 - Use wait with nanos in hope there are Java implementations which really support nano timeouts.
            • JENKINS-48309 - And again, @stephenc prevents a major embarassement due to the stupid typo
            Show
            scm_issue_link SCM/JIRA link daemon added a comment - Code changed in jenkins User: Oleg Nenashev Path: src/main/java/hudson/remoting/AsyncFutureImpl.java http://jenkins-ci.org/commit/remoting/3f07563743ef1874057eeb17e04847170436b39f Log: JENKINS-48309 - Prevent TimeoutException in AsyncFutureImpl in the case of spurious wakeups (#240) JENKINS-48309 - Prevent TimeoutException in AsyncFutureImpl in the case of spurious wakeups. TL;DR: A large part of Jenkins’ get-with-timeout logic is a subject for improper exit before the timeout. JENKINS-48309 - I was too lazy to use nanos in the beginning, address the comment from @stephenc JENKINS-48309 - Use wait with nanos in hope there are Java implementations which really support nano timeouts. JENKINS-48309 - And again, @stephenc prevents a major embarassement due to the stupid typo
            Hide
            scm_issue_link SCM/JIRA link daemon added a comment -

            Code changed in jenkins
            User: Oleg Nenashev
            Path:
            core/src/main/java/hudson/Launcher.java
            core/src/main/java/hudson/model/Computer.java
            core/src/main/java/hudson/slaves/ChannelPinger.java
            core/src/main/java/hudson/slaves/SlaveComputer.java
            core/src/main/java/jenkins/FilePathFilter.java
            core/src/main/java/jenkins/slaves/StandardOutputSwapper.java
            pom.xml
            test/src/test/java/hudson/bugs/JnlpAccessWithSecuredHudsonTest.java
            test/src/test/java/jenkins/security/Security218CliTest.java
            http://jenkins-ci.org/commit/jenkins/cb3990a4d6094260bea4571e7079fd0e3949047f
            Log:
            Update to Remoting 3.15 and Cleanup issues in Channel#current() usages (#3145)

            Pulls in fixes for: JENKINS-48133, JENKINS-48055, JENKINS-37566, JENKINS-48309, JENKINS-47965, JENKINS-48130, JENKINS-37670, JENKINS-37566, JENKINS-46724

            This change also adds some missing null/closing channel checks in the core.
            In some cases the change prevents spawning threads if the channel is in the invalid state.

            Show
            scm_issue_link SCM/JIRA link daemon added a comment - Code changed in jenkins User: Oleg Nenashev Path: core/src/main/java/hudson/Launcher.java core/src/main/java/hudson/model/Computer.java core/src/main/java/hudson/slaves/ChannelPinger.java core/src/main/java/hudson/slaves/SlaveComputer.java core/src/main/java/jenkins/FilePathFilter.java core/src/main/java/jenkins/slaves/StandardOutputSwapper.java pom.xml test/src/test/java/hudson/bugs/JnlpAccessWithSecuredHudsonTest.java test/src/test/java/jenkins/security/Security218CliTest.java http://jenkins-ci.org/commit/jenkins/cb3990a4d6094260bea4571e7079fd0e3949047f Log: Update to Remoting 3.15 and Cleanup issues in Channel#current() usages (#3145) Pulls in fixes for: JENKINS-48133 , JENKINS-48055 , JENKINS-37566 , JENKINS-48309 , JENKINS-47965 , JENKINS-48130 , JENKINS-37670 , JENKINS-37566 , JENKINS-46724 This change also adds some missing null/closing channel checks in the core. In some cases the change prevents spawning threads if the channel is in the invalid state.
            Hide
            oleg_nenashev Oleg Nenashev added a comment -

            The change has been released in Remoting 3.15 and integrated towards Jenkins 2.98

            Show
            oleg_nenashev Oleg Nenashev added a comment - The change has been released in Remoting 3.15 and integrated towards Jenkins 2.98

              People

              • Assignee:
                oleg_nenashev Oleg Nenashev
                Reporter:
                oleg_nenashev Oleg Nenashev
              • Votes:
                0 Vote for this issue
                Watchers:
                3 Start watching this issue

                Dates

                • Created:
                  Updated:
                  Resolved: