Janek Suchocki added a comment - 2023-02-09 10:26 Hi basil and allan_burdajewicz , I've seen that JENKINS-70334 has not been included in the yesterday's 2.353.3 LTS release. Is there some hesitance regarding porting it back to LTS because of this issue (59910) still being open?

Basil Crow added a comment - 2023-02-09 16:10 Not that I know of. Questions about backporting should be directed to the Release Lead and/or Release Officer.

Basil Crow added a comment - 2023-02-09 22:18 Maybe that is the scenario that causes this.. that when the AckFilterLayer (or any filter layer) is removed before the end of the ProtocolStack#start ? It's normal, as this logging demonstrates: Feb 09, 2023 1:52:19 PM org.jenkinsci.remoting.engine.WorkDirManager initializeWorkDir INFO: Using /tmp/remote/remoting as a remoting work directory Feb 09, 2023 1:52:19 PM org.jenkinsci.remoting.engine.WorkDirManager setupLogging INFO: Both error and output logs will be printed to /tmp/remote/remoting Feb 09, 2023 1:52:19 PM hudson.remoting.jnlp.Main createEngine INFO: Setting up agent: test Feb 09, 2023 1:52:19 PM hudson.remoting.Engine startEngine INFO: Using Remoting version: 999999-SNAPSHOT (private-02/09/2023 21:51 GMT-basil) Feb 09, 2023 1:52:19 PM org.jenkinsci.remoting.engine.WorkDirManager initializeWorkDir INFO: Using /tmp/remote/remoting as a remoting work directory Feb 09, 2023 1:52:19 PM hudson.remoting.jnlp.Main$CuiListener status INFO: Locating server among [http://127.0.0.1/] Feb 09, 2023 1:52:19 PM org.jenkinsci.remoting.engine.JnlpAgentEndpointResolver resolve INFO: Remoting server accepts the following protocols: [JNLP4-connect, Ping] Feb 09, 2023 1:52:19 PM hudson.remoting.jnlp.Main$CuiListener status INFO: Agent discovery successful Agent address: 127.0.0.1 Agent port: 59100 Identity: 34:ee:ff:fd:a6:3e:ed:fc:aa:76:9d:4b:aa:d6:e1:a0 Feb 09, 2023 1:52:19 PM hudson.remoting.jnlp.Main$CuiListener status INFO: Handshaking Feb 09, 2023 1:52:19 PM hudson.remoting.jnlp.Main$CuiListener status INFO: Connecting to 127.0.0.1:59100 Feb 09, 2023 1:52:19 PM hudson.remoting.jnlp.Main$CuiListener status INFO: Trying protocol: JNLP4-connect Successfully verified list of size 6 org.jenkinsci.remoting.protocol.impl.BIONetworkLayer org.jenkinsci.remoting.protocol.impl.AgentProtocolClientFilterLayer org.jenkinsci.remoting.protocol.impl.AckFilterLayer org.jenkinsci.remoting.protocol.impl.SSLEngineFilterLayer org.jenkinsci.remoting.protocol.impl.ConnectionHeadersFilterLayer org.jenkinsci.remoting.protocol.impl.ChannelApplicationLayer […] Feb 09, 2023 1:52:20 PM org.jenkinsci.remoting.protocol.impl.BIONetworkLayer$Reader run INFO: Waiting for ProtocolStack to start. Successfully verified list of size 6 org.jenkinsci.remoting.protocol.impl.BIONetworkLayer org.jenkinsci.remoting.protocol.impl.AgentProtocolClientFilterLayer org.jenkinsci.remoting.protocol.impl.AckFilterLayer org.jenkinsci.remoting.protocol.impl.SSLEngineFilterLayer org.jenkinsci.remoting.protocol.impl.ConnectionHeadersFilterLayer org.jenkinsci.remoting.protocol.impl.ChannelApplicationLayer […] Successfully verified list of size 5 org.jenkinsci.remoting.protocol.impl.BIONetworkLayer org.jenkinsci.remoting.protocol.impl.AckFilterLayer org.jenkinsci.remoting.protocol.impl.SSLEngineFilterLayer org.jenkinsci.remoting.protocol.impl.ConnectionHeadersFilterLayer org.jenkinsci.remoting.protocol.impl.ChannelApplicationLayer […] Successfully verified list of size 4 org.jenkinsci.remoting.protocol.impl.BIONetworkLayer org.jenkinsci.remoting.protocol.impl.SSLEngineFilterLayer org.jenkinsci.remoting.protocol.impl.ConnectionHeadersFilterLayer org.jenkinsci.remoting.protocol.impl.ChannelApplicationLayer Feb 09, 2023 1:52:22 PM hudson.remoting.jnlp.Main$CuiListener status INFO: Remote identity confirmed: 34:ee:ff:fd:a6:3e:ed:fc:aa:76:9d:4b:aa:d6:e1:a0 Successfully verified list of size 4 org.jenkinsci.remoting.protocol.impl.BIONetworkLayer org.jenkinsci.remoting.protocol.impl.SSLEngineFilterLayer org.jenkinsci.remoting.protocol.impl.ConnectionHeadersFilterLayer org.jenkinsci.remoting.protocol.impl.ChannelApplicationLayer […] Successfully verified list of size 3 org.jenkinsci.remoting.protocol.impl.BIONetworkLayer org.jenkinsci.remoting.protocol.impl.SSLEngineFilterLayer org.jenkinsci.remoting.protocol.impl.ChannelApplicationLayer Feb 09, 2023 1:52:22 PM hudson.remoting.jnlp.Main$CuiListener status INFO: Connected Successfully verified list of size 3 org.jenkinsci.remoting.protocol.impl.BIONetworkLayer org.jenkinsci.remoting.protocol.impl.SSLEngineFilterLayer org.jenkinsci.remoting.protocol.impl.ChannelApplicationLayer […] In other words AgentProtocolClientFilterLayer is first to be removed, followed by AckFilterLayer , followed by ConnectionHeadersFilterLayer . Of the filter layers, the only one that remains by the end is SSLEngineFilterLayer . This could be done by writing a list validation method that walks the whole doubly-linked list and validates that each previous and next pointer is as expected (i.e., as drawn in https://github.com/jenkinsci/remoting/blob/master/docs/protocols.md modulo any node removals). The latter may or may not be easier to build on top of https://github.com/jenkinsci/remoting/pull/615 which simplifies the linked list logic considerably. https://github.com/jenkinsci/remoting/commit/9f6785c250d03881aab8dbce4f3b7f805c1f87c3 (implemented on top of the linked list simplification in https://github.com/jenkinsci/remoting/commit/d65debc3f5409c40f0831d6445b4f932cc335965 ) is an example of what I mean. The current linked list logic in trunk is so bad that 9f6785c250d03881aab8dbce4f3b7f805c1f87c3 fails right away (reproducibly) when applied to trunk (without d65debc3f5409c40f0831d6445b4f932cc335965). But d65debc3f5409c40f0831d6445b4f932cc335965 + 9f6785c250d03881aab8dbce4f3b7f805c1f87c3 passes in my local testing. When I originally saw this, I had great hope that I had fixed the problem, but running d65debc3f5409c40f0831d6445b4f932cc335965 in a loop in https://github.com/jenkinsci/jenkins/pull/7567 still failed. At that point I surmised the next step was to run d65debc3f5409c40f0831d6445b4f932cc335965 + 9f6785c250d03881aab8dbce4f3b7f805c1f87c3 in a loop and see where the verification routine trips, but I lost the will to keep going.

Basil Crow added a comment - 2023-02-10 02:09 A different idea would be to completely delete the problematic Ptr class, instead representing the protocol stack as two (synchronized) lists of ProtocolLayer s (one for the send direction and one for the receive direction) in ProtocolStack . The existing methods in Ptr could all be reimplemented in terms of these lists with much less code. The main functionality in Ptr is to allow one ProtocolLayer to pass control to the next one (in either the send or receive direction), to allow a ProtocolLayer to be closed in one direction first and then the other, and to notify the ProtocolLayer when it has been closed in both directions. This could all be implemented in ProtocolStack with standard Java functionality like ArrayList#indexOf without the need for all the complexity of Ptr . I suspect this approach would have the greatest likelihood of success. But it is also the most work, tantamount to reimplementing the whole class from scratch.

Basil Crow added a comment - 2023-02-14 01:42 A different idea would be to completely delete the problematic Ptr class, instead representing the protocol stack as two (synchronized) lists of ProtocolLayer s (one for the send direction and one for the receive direction) in ProtocolStack . The existing methods in Ptr could all be reimplemented in terms of these lists with much less code. https://github.com/basil/remoting/tree/rewrite is a sketch of what a complete rewrite of this class could look like. The result is 200 lines shorter than the original while also being simpler and easier to reason about: a simpler locking scheme & no custom linked list implementation (to name a few simplifications). It is not quite as optimized as the original code, but I suspect the original code was prematurely optimized and that this rewrite is likely good enough or close to it. It seems to hold up fine to local testing, though I am a bit too afraid to try JnlpSlaveRestarterInstallerTest#tcpReconnection in a loop with this code.