Project

General

Profile

Actions

Bug #16780

closed

Net::FTP PUT command issuing Net::ReadTimeout too quickly

Added by rgerard (Ryan Gerard) about 2 years ago. Updated almost 2 years ago.

Status:
Closed
Priority:
Normal
Target version:
-
[ruby-core:97838]

Description

This is my first time writing up an issue for this community, so I apologize if this is written in an abnormal way than your normal issues.

We recently upgraded from ruby 2.6.5 to 2.7.1. Almost immediately, we started seeing issues with some of our application code that utilizes Net::FTP. All calls to #put to an external FTP server we communicate with were resulting in a Net::ReadTimeout exception being throw. Upon examination of the external FTP server, we can see that the file was successfully transferred, although the Net::ReadTimeout exception begin thrown caused confusion on our end.

Looking over the changes that were part of the ruby 2.7.0 release, I believe this commit (https://github.com/ruby/ruby/commit/5be34d6a3310065850c0c530db6936415124b5d9), which was a fix for this bug (https://bugs.ruby-lang.org/issues/16413) is the root of the issue. I translated the japanese, and I understand that the change to add the begin and ensure blocks were to make sure the connection was closed in case an exception is thrown during the transfer. However, the additional change to the connection read_timeout to 1 second was a little puzzling. I can see that this logic was copied from retrbinary and retrlines, but I'm uncertain why the connection read_timeout should be changed to 1 second.

I believe the right fix is to remove the change to the connection read_timeout.

Here is the stack trace for the exception we're getting during the call to #put:

Net::ReadTimeout with #<Socket:(closed)>

/usr/local/lib/ruby/2.7.0/net/protocol.rb:217:in `rbuf_fill'
/usr/local/lib/ruby/2.7.0/net/protocol.rb:159:in `read'
/usr/local/lib/ruby/2.7.0/net/ftp.rb:1475:in `read'
/usr/local/lib/ruby/2.7.0/net/ftp.rb:693:in `block (2 levels) in storbinary'
/usr/local/lib/ruby/2.7.0/net/ftp.rb:308:in `with_binary'
/usr/local/lib/ruby/2.7.0/net/ftp.rb:684:in `block in storbinary'
/usr/local/lib/ruby/2.7.0/monitor.rb:202:in `synchronize'
/usr/local/lib/ruby/2.7.0/monitor.rb:202:in `mon_synchronize'
/usr/local/lib/ruby/2.7.0/net/ftp.rb:683:in `storbinary'
/usr/local/lib/ruby/2.7.0/net/ftp.rb:843:in `putbinaryfile'
/usr/local/lib/ruby/2.7.0/net/ftp.rb:871:in `put'
Actions #1

Updated by rgerard (Ryan Gerard) about 2 years ago

  • Description updated (diff)

Updated by shugo (Shugo Maeda) about 2 years ago

  • Assignee set to naruse (Yui NARUSE)
  • Status changed from Open to Assigned

@naruse (Yui NARUSE) It seems that the change was introduced by your commit. Could you check it?

Updated by koshigoe (Masataka SUZUKI) about 2 years ago

I'm facing the same problem.

The Net::FTP#close suppresses exception about Socket#shutdown and Socket#read.

    def close
      if @sock and not @sock.closed?
        begin
          @sock.shutdown(Socket::SHUT_WR) rescue nil
          orig, self.read_timeout = self.read_timeout, 3
          @sock.read rescue nil
        ensure
          @sock.close
          self.read_timeout = orig
        end
      end
    end

Should I do the same when closing data connection?

diff --git a/lib/net/ftp.rb b/lib/net/ftp.rb
index d1e545c0c8..610027dc38 100644
--- a/lib/net/ftp.rb
+++ b/lib/net/ftp.rb
@@ -634,9 +634,9 @@ def retrbinary(cmd, blocksize, rest_offset = nil) # :yield: data
             while data = conn.read(blocksize)
               yield(data)
             end
-            conn.shutdown(Socket::SHUT_WR)
+            conn.shutdown(Socket::SHUT_WR) rescue nil
             conn.read_timeout = 1
-            conn.read
+            conn.read rescue nil
           ensure
             conn.close if conn
           end
@@ -659,9 +659,9 @@ def retrlines(cmd) # :yield: line
             while line = conn.gets
               yield(line.sub(/\r?\n\z/, ""), !line.match(/\n\z/).nil?)
             end
-            conn.shutdown(Socket::SHUT_WR)
+            conn.shutdown(Socket::SHUT_WR) rescue nil
             conn.read_timeout = 1
-            conn.read
+            conn.read rescue nil
           ensure
             conn.close if conn
           end
@@ -688,9 +688,9 @@ def storbinary(cmd, file, blocksize, rest_offset = nil) # :yield: data
               conn.write(buf)
               yield(buf) if block_given?
             end
-            conn.shutdown(Socket::SHUT_WR)
+            conn.shutdown(Socket::SHUT_WR) rescue nil
             conn.read_timeout = 1
-            conn.read
+            conn.read rescue nil
           ensure
             conn.close if conn
           end
@@ -724,9 +724,9 @@ def storlines(cmd, file) # :yield: line
               conn.write(buf)
               yield(buf) if block_given?
             end
-            conn.shutdown(Socket::SHUT_WR)
+            conn.shutdown(Socket::SHUT_WR) rescue nil
             conn.read_timeout = 1
-            conn.read
+            conn.read rescue nil
           ensure
             conn.close if conn
           end

Updated by shugo (Shugo Maeda) about 2 years ago

  • Backport changed from 2.5: UNKNOWN, 2.6: UNKNOWN, 2.7: UNKNOWN to 2.5: DONTNEED, 2.6: DONTNEED, 2.7: REQUIRED
  • Assignee changed from naruse (Yui NARUSE) to shugo (Shugo Maeda)

koshigoe (Masataka SUZUKI) wrote in #note-3:

The Net::FTP#close suppresses exception about Socket#shutdown and Socket#read.
(snip)
Should I do the same when closing data connection?

Thanks for your patch.

It looks good. I'll merge it.

Actions #5

Updated by shugo (Shugo Maeda) about 2 years ago

  • Status changed from Assigned to Closed

Applied in changeset git|5e81e8675a020ecd493620a4ff38db8fcf4b972a.


Ignore exceptions when closing data connections [Bug #16780]

Patch by koshigoe (Masataka SUZUKI). Thanks!

Updated by nagachika (Tomoyuki Chikanaga) almost 2 years ago

  • Backport changed from 2.5: DONTNEED, 2.6: DONTNEED, 2.7: REQUIRED to 2.5: DONTNEED, 2.6: DONTNEED, 2.7: DONE

ruby_2_7 578bacc471fa3fa14f8607fe67adccce21e81657 merged revision(s) 5e81e8675a020ecd493620a4ff38db8fcf4b972a.

Actions

Also available in: Atom PDF