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

FilePath.read() triggers remote issues if the file is not read until EOF (Pipe is already closed)

    • Icon: Bug Bug
    • Resolution: Fixed
    • Icon: Critical Critical
    • core
    • None

      If a stream is open remotely using FilePath.read() and the file is not read to the very end, the log will be spammed with exceptions like:

      Jan 10, 2011 1:36:51 PM hudson.remoting.ProxyOutputStream$Chunk$1 run
        WARNING: Failed to write to stream
         java.io.IOException: Pipe is already closed
         at hudson.remoting.FastPipedOutputStream.write(FastPipedOutputStream.java:147)
         at hudson.remoting.FastPipedOutputStream.write(FastPipedOutputStream.java:131)
         at hudson.remoting.ProxyOutputStream$Chunk$1.run(ProxyOutputStream.java:185) 
      

      ...and finally disconnect the slave. Several plug-ins are affected by this bug, for instance the Emma-plug-in. Marked as critical since it yet another remote issue and keeps us from upgrade. It is also easy to reproduce.

      To reproduce, just create a publisher plug-in with a perform() that looks like this:

        @Override
        public boolean perform(AbstractBuild build, Launcher launcher, BuildListener listener) throws IOException {
            InputStream in = build.getWorkspace().child("large_file_in_workspace").read();
            // Just read one byte
            System.out.println(in.read());
            // Close it and watch the hudson log grow and slave (most often) disconnect.
            in.close();
            return true;
        } 
      

      Test the plug-in with Hudson 1.390+. Make sure the new publisher plug-in is used in a project running on a slave (and also make sure that the "large_file_in_workspace"-file exists in the remote workspace).

          [JENKINS-8592] FilePath.read() triggers remote issues if the file is not read until EOF (Pipe is already closed)

          glundh added a comment -

          Hi! I'm really glad that someone decides to tackle this one. So thanks for jumping on it. Really bad remoting issues have been plaguing Hudson since 20 releases back.

          Great too see that you can get a first fix out this quickly. It will most certainly make the IOExceptions (and the remote disconnects) go away for the publisher plug-ins affected by this issue.

          But still. Hudson do need a mechanism to allow the receiver to notify the sender that it should stop pushing stream data. I myself were in a situation were I tried to read a header of a huge artifact. Pushing the full file would not really work in this situation. (Of course it is possible to work around the issue in the plug-in code, but the main issue will still be lingering in the Hudson core).

          I have spent some time thinking about how this should be done by the remoting library, without the need for a larger refactoring, but have not come up with a decent solution. So I'm more than glad that you are prepared to tackle it.

          glundh added a comment - Hi! I'm really glad that someone decides to tackle this one. So thanks for jumping on it. Really bad remoting issues have been plaguing Hudson since 20 releases back. Great too see that you can get a first fix out this quickly. It will most certainly make the IOExceptions (and the remote disconnects) go away for the publisher plug-ins affected by this issue. But still. Hudson do need a mechanism to allow the receiver to notify the sender that it should stop pushing stream data. I myself were in a situation were I tried to read a header of a huge artifact. Pushing the full file would not really work in this situation. (Of course it is possible to work around the issue in the plug-in code, but the main issue will still be lingering in the Hudson core). I have spent some time thinking about how this should be done by the remoting library, without the need for a larger refactoring, but have not come up with a decent solution. So I'm more than glad that you are prepared to tackle it.

          Code changed in jenkins
          User: Kohsuke Kawaguchi
          Path:
          remoting/src/main/java/hudson/remoting/Pipe.java
          remoting/src/test/java/hudson/remoting/PipeTest.java
          http://jenkins-ci.org/commit/core/758aedf885d4eee2c24ec9889e5886826af9a500
          Log:
          Merge commit 'c628bdd32be8fdf0f663b373836585498e1ecaa1'

          • commit 'c628bdd32be8fdf0f663b373836585498e1ecaa1':
            First pass at a fix for JENKINS-8592.

          SCM/JIRA link daemon added a comment - Code changed in jenkins User: Kohsuke Kawaguchi Path: remoting/src/main/java/hudson/remoting/Pipe.java remoting/src/test/java/hudson/remoting/PipeTest.java http://jenkins-ci.org/commit/core/758aedf885d4eee2c24ec9889e5886826af9a500 Log: Merge commit 'c628bdd32be8fdf0f663b373836585498e1ecaa1' commit 'c628bdd32be8fdf0f663b373836585498e1ecaa1': First pass at a fix for JENKINS-8592 .

          Code changed in jenkins
          User: Kohsuke Kawaguchi
          Path:
          remoting/src/main/java/hudson/remoting/Channel.java
          remoting/src/main/java/hudson/remoting/Command.java
          remoting/src/main/java/hudson/remoting/Pipe.java
          remoting/src/main/java/hudson/remoting/PipeWindow.java
          remoting/src/main/java/hudson/remoting/ProxyOutputStream.java
          remoting/src/test/java/hudson/remoting/PipeTest.java
          http://jenkins-ci.org/commit/core/73a8007d43a7317f168709539ed6c15466cf9044
          Log:
          JENKINS-8592 let the writer end know if the reader end of the pipe
          failed.

          SCM/JIRA link daemon added a comment - Code changed in jenkins User: Kohsuke Kawaguchi Path: remoting/src/main/java/hudson/remoting/Channel.java remoting/src/main/java/hudson/remoting/Command.java remoting/src/main/java/hudson/remoting/Pipe.java remoting/src/main/java/hudson/remoting/PipeWindow.java remoting/src/main/java/hudson/remoting/ProxyOutputStream.java remoting/src/test/java/hudson/remoting/PipeTest.java http://jenkins-ci.org/commit/core/73a8007d43a7317f168709539ed6c15466cf9044 Log: JENKINS-8592 let the writer end know if the reader end of the pipe failed.

          nairb774 added a comment -

          Should be fixed as of 1.397

          nairb774 added a comment - Should be fixed as of 1.397

          glundh added a comment -

          According to the Issue-discussion in the mailing-list, issues should only be marked as fixed when the changes is released. And since the last round of patches (Kohsuke's + your review patch) is not released, I propose we keep it open (due to the sake of traceability).

          I hope this is OK (yes, this specific issue matters quite greatly to me )

          glundh added a comment - According to the Issue-discussion in the mailing-list, issues should only be marked as fixed when the changes is released. And since the last round of patches (Kohsuke's + your review patch) is not released, I propose we keep it open (due to the sake of traceability). I hope this is OK (yes, this specific issue matters quite greatly to me )

          Code changed in jenkins
          User: Brian Atkinson
          Path:
          remoting/src/main/java/hudson/remoting/Channel.java
          remoting/src/main/java/hudson/remoting/Command.java
          remoting/src/main/java/hudson/remoting/Pipe.java
          remoting/src/main/java/hudson/remoting/PipeWindow.java
          remoting/src/main/java/hudson/remoting/ProxyOutputStream.java
          remoting/src/test/java/hudson/remoting/PipeTest.java
          http://jenkins-ci.org/commit/core/4c10efe1a9285bcc6eed193ede82c0bdd39d658a
          Log:
          Merge branch 'JENKINS-8592'

          SCM/JIRA link daemon added a comment - Code changed in jenkins User: Brian Atkinson Path: remoting/src/main/java/hudson/remoting/Channel.java remoting/src/main/java/hudson/remoting/Command.java remoting/src/main/java/hudson/remoting/Pipe.java remoting/src/main/java/hudson/remoting/PipeWindow.java remoting/src/main/java/hudson/remoting/ProxyOutputStream.java remoting/src/test/java/hudson/remoting/PipeTest.java http://jenkins-ci.org/commit/core/4c10efe1a9285bcc6eed193ede82c0bdd39d658a Log: Merge branch ' JENKINS-8592 '

          dogfood added a comment -

          Integrated in jenkins_main_trunk #519
          JENKINS-8592 let the writer end know if the reader end of the pipe

          Kohsuke Kawaguchi :
          Files :

          • remoting/src/main/java/hudson/remoting/Pipe.java
          • remoting/src/main/java/hudson/remoting/ProxyOutputStream.java
          • remoting/src/test/java/hudson/remoting/PipeTest.java
          • remoting/src/main/java/hudson/remoting/PipeWindow.java
          • remoting/src/main/java/hudson/remoting/Channel.java
          • remoting/src/main/java/hudson/remoting/Command.java

          dogfood added a comment - Integrated in jenkins_main_trunk #519 JENKINS-8592 let the writer end know if the reader end of the pipe Kohsuke Kawaguchi : Files : remoting/src/main/java/hudson/remoting/Pipe.java remoting/src/main/java/hudson/remoting/ProxyOutputStream.java remoting/src/test/java/hudson/remoting/PipeTest.java remoting/src/main/java/hudson/remoting/PipeWindow.java remoting/src/main/java/hudson/remoting/Channel.java remoting/src/main/java/hudson/remoting/Command.java

          gjeudy added a comment -

          I was told in a comment that this issue might fix JENKINS-6355 indirectly. I can confirm it has not fixed JENKINS-6355.

          I'm using 1.400 and emma plugin 1.21 and issue still occurs intermittently.

          gjeudy added a comment - I was told in a comment that this issue might fix JENKINS-6355 indirectly. I can confirm it has not fixed JENKINS-6355 . I'm using 1.400 and emma plugin 1.21 and issue still occurs intermittently.

          gjeudy added a comment -

          sorry the JIRA in the previous comment should be: JENKINS-7586

          gjeudy added a comment - sorry the JIRA in the previous comment should be: JENKINS-7586

          kutzi added a comment -

          Since there were several changes committed for this, I assume it's fixed.
          Please reopen, if not.

          kutzi added a comment - Since there were several changes committed for this, I assume it's fixed. Please reopen, if not.

            nairb774 nairb774
            GLundh GLundh
            Votes:
            2 Vote for this issue
            Watchers:
            4 Start watching this issue

              Created:
              Updated:
              Resolved: