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

JNLP slaves can fail to correctly negotiate a transport

    XMLWordPrintable

    Details

    • Similar Issues:

      Description

      The changes in PR 41 introduced a regression whereby the read-ahead buffered input stream gets thrown away after the protocol has been detected but before the protocol negotiation starts.

      The result of this is that depending on random timing factors, the capability and mode information that has been sent to the remote side may get lost and one side will infer a capability of 0 while the other side believes the agreed capability to be more.

      When the remote side is assuming chunking, the connection will typically fail immediately with an error such as:

      INFO: Protocol failed to establish channel
      java.io.StreamCorruptedException: invalid stream header: 0A6CACED
          at java.io.ObjectInputStream.readStreamHeader(ObjectInputStream.java:806)
          at java.io.ObjectInputStream.<init>(ObjectInputStream.java:299)
          at hudson.remoting.ObjectInputStreamEx.<init>(ObjectInputStreamEx.java:48)
          at hudson.remoting.ChannelBuilder.makeTransport(ChannelBuilder.java:430)
          at hudson.remoting.ChannelBuilder.negotiate(ChannelBuilder.java:389)
          at hudson.remoting.ChannelBuilder.build(ChannelBuilder.java:310)
          at org.jenkinsci.remoting.engine.JnlpProtocol2.buildChannel(JnlpProtocol2.java:93)
          at org.jenkinsci.remoting.engine.JnlpProtocol.establishChannel(JnlpProtocol.java:79)
          at hudson.remoting.Engine.run(Engine.java:245)
      

      But if the remote side is not assuming chunking then more subtle remoting issues could arise (e.g. if the remote slave is running an older pre-chunking slave.jar and connecting to a newer Jenkins... not that you should be doing that, but some people may... this reason is why I argue the issue is "Critical")

        Attachments

          Issue Links

            Activity

            stephenconnolly Stephen Connolly created issue -
            Hide
            scm_issue_link SCM/JIRA link daemon added a comment -

            Code changed in jenkins
            User: Stephen Connolly
            Path:
            src/main/java/hudson/remoting/Capability.java
            src/main/java/hudson/remoting/ChannelBuilder.java
            src/main/java/org/jenkinsci/remoting/engine/JnlpProtocol.java
            src/main/java/org/jenkinsci/remoting/engine/JnlpProtocol1.java
            src/main/java/org/jenkinsci/remoting/engine/JnlpProtocol2.java
            src/main/java/org/jenkinsci/remoting/engine/JnlpProtocol3.java
            src/test/java/org/jenkinsci/remoting/engine/JnlpProtocol1Test.java
            src/test/java/org/jenkinsci/remoting/engine/JnlpProtocol2Test.java
            src/test/java/org/jenkinsci/remoting/engine/JnlpProtocol3Test.java
            src/test/java/org/jenkinsci/remoting/engine/JnlpProtocolTest.java
            http://jenkins-ci.org/commit/remoting/116315728249eb392b9af0de0be5850959536c03
            Log:
            [FIXED JENKINS-31718] Preserve the BufferedInputStream when building the transport

            • For now I do not have the energy to fix the tests to not need the 'null' backdoor.
            • Most likely some non-mock based tests would be better and likely would have caught this regression sooner.
            Show
            scm_issue_link SCM/JIRA link daemon added a comment - Code changed in jenkins User: Stephen Connolly Path: src/main/java/hudson/remoting/Capability.java src/main/java/hudson/remoting/ChannelBuilder.java src/main/java/org/jenkinsci/remoting/engine/JnlpProtocol.java src/main/java/org/jenkinsci/remoting/engine/JnlpProtocol1.java src/main/java/org/jenkinsci/remoting/engine/JnlpProtocol2.java src/main/java/org/jenkinsci/remoting/engine/JnlpProtocol3.java src/test/java/org/jenkinsci/remoting/engine/JnlpProtocol1Test.java src/test/java/org/jenkinsci/remoting/engine/JnlpProtocol2Test.java src/test/java/org/jenkinsci/remoting/engine/JnlpProtocol3Test.java src/test/java/org/jenkinsci/remoting/engine/JnlpProtocolTest.java http://jenkins-ci.org/commit/remoting/116315728249eb392b9af0de0be5850959536c03 Log: [FIXED JENKINS-31718] Preserve the BufferedInputStream when building the transport For now I do not have the energy to fix the tests to not need the 'null' backdoor. Most likely some non-mock based tests would be better and likely would have caught this regression sooner.
            scm_issue_link SCM/JIRA link daemon made changes -
            Field Original Value New Value
            Status Open [ 1 ] Resolved [ 5 ]
            Resolution Fixed [ 1 ]
            Hide
            scm_issue_link SCM/JIRA link daemon added a comment -

            Code changed in jenkins
            User: Stephen Connolly
            Path:
            src/main/java/org/jenkinsci/remoting/engine/JnlpProtocol1.java
            src/main/java/org/jenkinsci/remoting/engine/JnlpProtocol2.java
            src/main/java/org/jenkinsci/remoting/engine/JnlpProtocol3.java
            src/test/java/org/jenkinsci/remoting/engine/JnlpProtocol1Test.java
            src/test/java/org/jenkinsci/remoting/engine/JnlpProtocol2Test.java
            src/test/java/org/jenkinsci/remoting/engine/JnlpProtocol3Test.java
            src/test/java/org/jenkinsci/remoting/engine/JnlpProtocolTest.java
            http://jenkins-ci.org/commit/remoting/015e280ddc20b858d810349a6243abba6741044b
            Log:
            JENKINS-31718 Fix the tests

            • The tests no longer pass in a null buffered input stream and thus we can remove the null handling from the protocols
            • Unsure whether the buffer may affect JnlpProtocol3 performance, but leaving it there to highlight the potential stream corruption if the buffer gets thrown away.
            Show
            scm_issue_link SCM/JIRA link daemon added a comment - Code changed in jenkins User: Stephen Connolly Path: src/main/java/org/jenkinsci/remoting/engine/JnlpProtocol1.java src/main/java/org/jenkinsci/remoting/engine/JnlpProtocol2.java src/main/java/org/jenkinsci/remoting/engine/JnlpProtocol3.java src/test/java/org/jenkinsci/remoting/engine/JnlpProtocol1Test.java src/test/java/org/jenkinsci/remoting/engine/JnlpProtocol2Test.java src/test/java/org/jenkinsci/remoting/engine/JnlpProtocol3Test.java src/test/java/org/jenkinsci/remoting/engine/JnlpProtocolTest.java http://jenkins-ci.org/commit/remoting/015e280ddc20b858d810349a6243abba6741044b Log: JENKINS-31718 Fix the tests The tests no longer pass in a null buffered input stream and thus we can remove the null handling from the protocols Unsure whether the buffer may affect JnlpProtocol3 performance, but leaving it there to highlight the potential stream corruption if the buffer gets thrown away.
            jglick Jesse Glick made changes -
            Resolution Fixed [ 1 ]
            Status Resolved [ 5 ] Reopened [ 4 ]
            jglick Jesse Glick made changes -
            Status Reopened [ 4 ] Open [ 1 ]
            jglick Jesse Glick made changes -
            Status Open [ 1 ] In Progress [ 3 ]
            danielbeck Daniel Beck made changes -
            Link This issue is duplicated by JENKINS-31715 [ JENKINS-31715 ]
            Hide
            jglick Jesse Glick added a comment -

            Avoid using the FIXED prefix in commits which

            • Are in an origin branch. The JIRA link daemon does not check that they are ancestors of master.
            • Are in a component repository. The FIXED commit should only be in the jenkins repository when integrating a component release with the fix.
            Show
            jglick Jesse Glick added a comment - Avoid using the FIXED prefix in commits which Are in an origin branch. The JIRA link daemon does not check that they are ancestors of master . Are in a component repository. The FIXED commit should only be in the jenkins repository when integrating a component release with the fix.
            jglick Jesse Glick made changes -
            Remote Link This issue links to "PR 65 (Web Link)" [ 13551 ]
            jglick Jesse Glick made changes -
            Link This issue is blocking JENKINS-26580 [ JENKINS-26580 ]
            jglick Jesse Glick made changes -
            Labels regression
            jglick Jesse Glick made changes -
            Labels regression lts-candidate regression
            Hide
            scm_issue_link SCM/JIRA link daemon added a comment -

            Code changed in jenkins
            User: Stephen Connolly
            Path:
            src/main/java/hudson/remoting/Capability.java
            src/main/java/hudson/remoting/ChannelBuilder.java
            src/main/java/org/jenkinsci/remoting/engine/JnlpProtocol.java
            src/main/java/org/jenkinsci/remoting/engine/JnlpProtocol1.java
            src/main/java/org/jenkinsci/remoting/engine/JnlpProtocol2.java
            src/main/java/org/jenkinsci/remoting/engine/JnlpProtocol3.java
            src/test/java/org/jenkinsci/remoting/engine/JnlpProtocol1Test.java
            src/test/java/org/jenkinsci/remoting/engine/JnlpProtocol2Test.java
            src/test/java/org/jenkinsci/remoting/engine/JnlpProtocol3Test.java
            src/test/java/org/jenkinsci/remoting/engine/JnlpProtocolTest.java
            http://jenkins-ci.org/commit/remoting/17aa60dd6c84499962a7117027da3e73e392ce46
            Log:
            [FIXED JENKINS-31718] Preserve the BufferedInputStream when building the transport

            • For now I do not have the energy to fix the tests to not need the 'null' backdoor.
            • Most likely some non-mock based tests would be better and likely would have caught this regression sooner.
            Show
            scm_issue_link SCM/JIRA link daemon added a comment - Code changed in jenkins User: Stephen Connolly Path: src/main/java/hudson/remoting/Capability.java src/main/java/hudson/remoting/ChannelBuilder.java src/main/java/org/jenkinsci/remoting/engine/JnlpProtocol.java src/main/java/org/jenkinsci/remoting/engine/JnlpProtocol1.java src/main/java/org/jenkinsci/remoting/engine/JnlpProtocol2.java src/main/java/org/jenkinsci/remoting/engine/JnlpProtocol3.java src/test/java/org/jenkinsci/remoting/engine/JnlpProtocol1Test.java src/test/java/org/jenkinsci/remoting/engine/JnlpProtocol2Test.java src/test/java/org/jenkinsci/remoting/engine/JnlpProtocol3Test.java src/test/java/org/jenkinsci/remoting/engine/JnlpProtocolTest.java http://jenkins-ci.org/commit/remoting/17aa60dd6c84499962a7117027da3e73e392ce46 Log: [FIXED JENKINS-31718] Preserve the BufferedInputStream when building the transport For now I do not have the energy to fix the tests to not need the 'null' backdoor. Most likely some non-mock based tests would be better and likely would have caught this regression sooner.
            scm_issue_link SCM/JIRA link daemon made changes -
            Status In Progress [ 3 ] Resolved [ 5 ]
            Resolution Fixed [ 1 ]
            Hide
            scm_issue_link SCM/JIRA link daemon added a comment -

            Code changed in jenkins
            User: Stephen Connolly
            Path:
            src/main/java/org/jenkinsci/remoting/engine/JnlpProtocol1.java
            src/main/java/org/jenkinsci/remoting/engine/JnlpProtocol2.java
            src/main/java/org/jenkinsci/remoting/engine/JnlpProtocol3.java
            src/test/java/org/jenkinsci/remoting/engine/JnlpProtocol1Test.java
            src/test/java/org/jenkinsci/remoting/engine/JnlpProtocol2Test.java
            src/test/java/org/jenkinsci/remoting/engine/JnlpProtocol3Test.java
            src/test/java/org/jenkinsci/remoting/engine/JnlpProtocolTest.java
            http://jenkins-ci.org/commit/remoting/7a50cd2e8e8d40b243eab5584e5ae44d48215b2d
            Log:
            JENKINS-31718 Fix the tests

            • The tests no longer pass in a null buffered input stream and thus we can remove the null handling from the protocols
            • Unsure whether the buffer may affect JnlpProtocol3 performance, but leaving it there to highlight the potential stream corruption if the buffer gets thrown away.
            Show
            scm_issue_link SCM/JIRA link daemon added a comment - Code changed in jenkins User: Stephen Connolly Path: src/main/java/org/jenkinsci/remoting/engine/JnlpProtocol1.java src/main/java/org/jenkinsci/remoting/engine/JnlpProtocol2.java src/main/java/org/jenkinsci/remoting/engine/JnlpProtocol3.java src/test/java/org/jenkinsci/remoting/engine/JnlpProtocol1Test.java src/test/java/org/jenkinsci/remoting/engine/JnlpProtocol2Test.java src/test/java/org/jenkinsci/remoting/engine/JnlpProtocol3Test.java src/test/java/org/jenkinsci/remoting/engine/JnlpProtocolTest.java http://jenkins-ci.org/commit/remoting/7a50cd2e8e8d40b243eab5584e5ae44d48215b2d Log: JENKINS-31718 Fix the tests The tests no longer pass in a null buffered input stream and thus we can remove the null handling from the protocols Unsure whether the buffer may affect JnlpProtocol3 performance, but leaving it there to highlight the potential stream corruption if the buffer gets thrown away.
            Hide
            jglick Jesse Glick added a comment -

            Not fixed until integrated into core.

            Show
            jglick Jesse Glick added a comment - Not fixed until integrated into core.
            jglick Jesse Glick made changes -
            Resolution Fixed [ 1 ]
            Status Resolved [ 5 ] Reopened [ 4 ]
            jglick Jesse Glick made changes -
            Status Reopened [ 4 ] Open [ 1 ]
            jglick Jesse Glick made changes -
            Status Open [ 1 ] In Progress [ 3 ]
            jglick Jesse Glick made changes -
            Remote Link This issue links to "PR 1927 (Web Link)" [ 13557 ]
            Hide
            scm_issue_link SCM/JIRA link daemon added a comment -

            Code changed in jenkins
            User: Jesse Glick
            Path:
            pom.xml
            http://jenkins-ci.org/commit/jenkins/e6d8500dd8e9b157ae8c54aec6fd42286e88301a
            Log:
            [FIXED JENKINS-31718] Integrated a new version of Remoting with a fix for a regression in concurrent socket connections.

            Show
            scm_issue_link SCM/JIRA link daemon added a comment - Code changed in jenkins User: Jesse Glick Path: pom.xml http://jenkins-ci.org/commit/jenkins/e6d8500dd8e9b157ae8c54aec6fd42286e88301a Log: [FIXED JENKINS-31718] Integrated a new version of Remoting with a fix for a regression in concurrent socket connections.
            scm_issue_link SCM/JIRA link daemon made changes -
            Status In Progress [ 3 ] Resolved [ 5 ]
            Resolution Fixed [ 1 ]
            Hide
            scm_issue_link SCM/JIRA link daemon added a comment -

            Code changed in jenkins
            User: Kohsuke Kawaguchi
            Path:
            pom.xml
            http://jenkins-ci.org/commit/jenkins/940796f04fc43aed750aff970bc4fad4793f068f
            Log:
            Merge pull request #1927 from jglick/remoting-JENKINS-31718

            JENKINS-31718 Integrated a new version of Remoting

            Compare: https://github.com/jenkinsci/jenkins/compare/e930da45ff4c...940796f04fc4

            Show
            scm_issue_link SCM/JIRA link daemon added a comment - Code changed in jenkins User: Kohsuke Kawaguchi Path: pom.xml http://jenkins-ci.org/commit/jenkins/940796f04fc43aed750aff970bc4fad4793f068f Log: Merge pull request #1927 from jglick/remoting- JENKINS-31718 JENKINS-31718 Integrated a new version of Remoting Compare: https://github.com/jenkinsci/jenkins/compare/e930da45ff4c...940796f04fc4
            Hide
            scm_issue_link SCM/JIRA link daemon added a comment -

            Code changed in jenkins
            User: Jesse Glick
            Path:
            pom.xml
            http://jenkins-ci.org/commit/jenkins/9e321a4a0eefde421709647f0f15cb7bc8da61d1
            Log:
            [FIXED JENKINS-31718] Integrated a new version of Remoting with a fix for a regression in concurrent socket connections.

            (cherry picked from commit e6d8500dd8e9b157ae8c54aec6fd42286e88301a)

            In the project meeting today we decided to pull this in.

            Show
            scm_issue_link SCM/JIRA link daemon added a comment - Code changed in jenkins User: Jesse Glick Path: pom.xml http://jenkins-ci.org/commit/jenkins/9e321a4a0eefde421709647f0f15cb7bc8da61d1 Log: [FIXED JENKINS-31718] Integrated a new version of Remoting with a fix for a regression in concurrent socket connections. (cherry picked from commit e6d8500dd8e9b157ae8c54aec6fd42286e88301a) In the project meeting today we decided to pull this in.
            Hide
            dogfood dogfood added a comment -

            Integrated in jenkins_main_trunk #4373
            [FIXED JENKINS-31718] Integrated a new version of Remoting with a fix for a regression in concurrent socket connections. (Revision e6d8500dd8e9b157ae8c54aec6fd42286e88301a)
            [FIXED JENKINS-31718] Integrated a new version of Remoting with a fix for a regression in concurrent socket connections. (Revision 9e321a4a0eefde421709647f0f15cb7bc8da61d1)

            Result = SUCCESS
            jesse glick : e6d8500dd8e9b157ae8c54aec6fd42286e88301a
            Files :

            • pom.xml

            kohsuke : 9e321a4a0eefde421709647f0f15cb7bc8da61d1
            Files :

            • pom.xml
            Show
            dogfood dogfood added a comment - Integrated in jenkins_main_trunk #4373 [FIXED JENKINS-31718] Integrated a new version of Remoting with a fix for a regression in concurrent socket connections. (Revision e6d8500dd8e9b157ae8c54aec6fd42286e88301a) [FIXED JENKINS-31718] Integrated a new version of Remoting with a fix for a regression in concurrent socket connections. (Revision 9e321a4a0eefde421709647f0f15cb7bc8da61d1) Result = SUCCESS jesse glick : e6d8500dd8e9b157ae8c54aec6fd42286e88301a Files : pom.xml kohsuke : 9e321a4a0eefde421709647f0f15cb7bc8da61d1 Files : pom.xml
            danielbeck Daniel Beck made changes -
            Labels lts-candidate regression 1.625.3-fixed regression
            Hide
            dogfood dogfood added a comment -

            Integrated in jenkins_2.0 #5
            [FIXED JENKINS-31718] Integrated a new version of Remoting with a fix (Revision e6d8500dd8e9b157ae8c54aec6fd42286e88301a)
            [FIXED JENKINS-31718] Integrated a new version of Remoting with a fix (Revision 9e321a4a0eefde421709647f0f15cb7bc8da61d1)

            Result = SUCCESS
            jesse glick : e6d8500dd8e9b157ae8c54aec6fd42286e88301a
            Files :

            • pom.xml

            kohsuke : 9e321a4a0eefde421709647f0f15cb7bc8da61d1
            Files :

            • pom.xml
            Show
            dogfood dogfood added a comment - Integrated in jenkins_2.0 #5 [FIXED JENKINS-31718] Integrated a new version of Remoting with a fix (Revision e6d8500dd8e9b157ae8c54aec6fd42286e88301a) [FIXED JENKINS-31718] Integrated a new version of Remoting with a fix (Revision 9e321a4a0eefde421709647f0f15cb7bc8da61d1) Result = SUCCESS jesse glick : e6d8500dd8e9b157ae8c54aec6fd42286e88301a Files : pom.xml kohsuke : 9e321a4a0eefde421709647f0f15cb7bc8da61d1 Files : pom.xml
            stephenconnolly Stephen Connolly made changes -
            Status Resolved [ 5 ] Closed [ 6 ]
            rtyler R. Tyler Croy made changes -
            Workflow JNJira [ 167158 ] JNJira + In-Review [ 209468 ]

              People

              • Assignee:
                stephenconnolly Stephen Connolly
                Reporter:
                stephenconnolly Stephen Connolly
              • Votes:
                0 Vote for this issue
                Watchers:
                5 Start watching this issue

                Dates

                • Created:
                  Updated:
                  Resolved: