Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with
or
.
Download ZIP

Loading…

Fixed timeouts in Net::Ping::External #32

Merged
merged 2 commits into from

3 participants

@bcandrea

Using Timeout.timeout with Open3 is not an option with Ruby > 1.9.3
(see https://bugs.ruby-lang.org/issues/5487). Implemented a workaround
based on the code at https://gist.github.com/lpar/1032297

@bcandrea bcandrea Fixed timeouts in Net::Ping::External
Using Timeout.timeout with Open3 is not an option with Ruby > 1.9.3
(see https://bugs.ruby-lang.org/issues/5487). Implemented a workaround
based on the code at https://gist.github.com/lpar/1032297
77fd65a
@djberg96
Owner

Eesh, ok. Good work.

Can you confirm that the code works and the tests pass with JRuby, too?

@bcandrea bcandrea Update travis.yml to trigger CI run
This update should trigger a Travis CI build on different Rubies.
6ae1b88
@bcandrea

Yes, Travis looks reasonably happy. I also ran this simple test on my local Rubies (2.0.0, 2.1.0, 2.1.1 and JRuby 1.7.9):

#!/usr/bin/env ruby
# net-ping/test.rb
$LOAD_PATH.unshift 'lib'
require 'net/ping'
require 'benchmark'

p = Net::Ping::External.new '10.0.0.0'
p.timeout = 1
Benchmark.bm do |x|
  x.report { p.ping }
end

The results for JRuby are:

$ rbenv shell jruby-1.7.9 
$ ruby -v
jruby 1.7.9 (1.9.3p392) 2013-12-06 87b108a on OpenJDK 64-Bit Server VM 1.7.0_51-b00 [linux-amd64]
aleph@eve:~/projects/net-ping$ ./test_ping.rb 
       user     system      total        real
   0.830000   0.620000   1.450000 (  1.017000)
@djberg96 djberg96 merged commit 61cd0a4 into djberg96:master
@djberg96
Owner

Hm, it seems Windows is very unhappy. This is why Travis CI really needs cross platform support. I'll see if I can fix it, but I may have to undo this.

@djberg96
Owner

It looks like Windows doesn't like read_nonblock. Replacing it with a standard read worked. At least, the tests all passed.

@bcandrea

Sorry, my fault (I don't have access to a full-featured Windows test machine unfortunately). The select call on pipes is indeed unsupported on Windows. However, are you sure that the standard read does the job? I tried it quickly and it seems to wait until the ping subprocess exits... and additionally my Process.kill("TERM", pid) fails with Errno::EINVAL on Windows, so I had to switch to Process.kill("KILL", pid), which looks a bit more drastic.

Disclaimer: as I said, my win32 test platform is quite limited (MRI 2.0.0 only). I can spend some more time on this later maybe.

@djberg96
Owner

I ran rake test:external and all tests passed with both mingw (32 and 64 bit versions) and a version built using Visual Studio.

@djberg96
Owner

That's on Windows 7, btw. Are the tests failing for you?

@ianheggie

Pik might be of use for multiple ruby versions under windows. Now just need a os/x test machine ... ;)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Commits on Mar 26, 2014
  1. @bcandrea

    Fixed timeouts in Net::Ping::External

    bcandrea authored
    Using Timeout.timeout with Open3 is not an option with Ruby > 1.9.3
    (see https://bugs.ruby-lang.org/issues/5487). Implemented a workaround
    based on the code at https://gist.github.com/lpar/1032297
  2. @bcandrea

    Update travis.yml to trigger CI run

    bcandrea authored
    This update should trigger a Travis CI build on different Rubies.
This page is out of date. Refresh to see the latest.
View
2  .travis.yml
@@ -29,3 +29,5 @@ matrix:
- rvm: rbx
- rvm: jruby-head
- rvm: ruby-head
+
+# Updated to trigger a build
View
89 lib/net/ping/external.rb
@@ -8,6 +8,9 @@ module Net
# The Ping::External class encapsulates methods for external (system) pings.
class Ping::External < Ping
+
+ ERR_MSG_SIZE = 4096
+
# Pings the host using your system's ping utility and checks for any
# errors or warnings. Returns true if successful, or false if not.
#
@@ -37,27 +40,20 @@ def ping(host = @host)
start_time = Time.now
begin
- err = nil
-
- Timeout.timeout(@timeout){
- Open3.popen3(*pcmd) do |stdin, stdout, stderr, thread|
- err = stderr.gets # Can't chomp yet, might be nil
-
- case thread.value.exitstatus
- when 0
- bool = true # Success, at least one response.
- if err & err =~ /warning/i
- @warning = err.chomp
- end
- when 2
- bool = false # Transmission successful, no response.
- @exception = err.chomp
- else
- bool = false # An error occurred
- @exception = err.chomp
- end
+ exit_code, err = run_with_timeout(*pcmd)
+ case exit_code
+ when 0
+ bool = true # Success, at least one response.
+ if err and err =~ /warning/i
+ @warning = err.chomp
end
- }
+ when 2
+ bool = false # Transmission successful, no response.
+ @exception = err.chomp
+ else
+ bool = false # An error occurred
+ @exception = err.chomp
+ end
rescue Exception => error
@exception = error.message
end
@@ -68,6 +64,59 @@ def ping(host = @host)
bool
end
+ # Runs a specified shell command in a separate thread.
+ # If it exceeds the given timeout in seconds, kills it.
+ # Returns a list of the form
+ #
+ # [exit_code, err]
+ #
+ # which contains any output sent by the command to stderr as a String.
+ # Uses Kernel.select to wait up to the timeout length (in seconds)
+ # before killing the process spawned by Open3.
+ #
+ def run_with_timeout(*command)
+ err = nil
+ exit_code = nil
+ begin
+ # Start task in another thread, which spawns a process
+ stdin, stdout, stderr, thread = Open3.popen3(*command)
+ # Get the pid of the spawned process
+ pid = thread[:pid]
+ start = Time.now
+
+ while (Time.now - start) < timeout and thread.alive?
+ begin
+ err = stderr.read_nonblock(ERR_MSG_SIZE)
+ rescue IO::WaitReadable
+ IO.select([stderr], nil, nil, @timeout)
+ next
+ rescue EOFError
+ # Command has completed, not really an error...
+ break
+ end
+ end
+
+ if thread.alive?
+ # We need to kill the process, because killing the thread leaves
+ # the process alive but detached, annoyingly enough.
+ begin
+ Process.kill("TERM", pid)
+ err = "execution expired"
+ rescue Errno::ESRCH
+ # The process already exited, ignoring
+ end
+ end
+ # Join the thread and get its exit status
+ exit_code = thread.value.exitstatus
+ ensure
+ stdin.close if stdin
+ stdout.close if stdout
+ stderr.close if stderr
+ end
+ err ||= ''
+ return [exit_code, err]
+ end
+
alias ping? ping
alias pingecho ping
end
View
29 test/test_net_ping_external.rb
@@ -13,10 +13,12 @@
class TC_Net_Ping_External < Test::Unit::TestCase
def setup
- @host = 'localhost'
- @bogus = 'foo.bar.baz'
- @pe = Net::Ping::External.new(@host)
- @bad = Net::Ping::External.new(@bogus)
+ @host = 'localhost'
+ @bogus = 'foo.bar.baz'
+ @bcast_ip = '10.0.0.0'
+ @pe = Net::Ping::External.new(@host)
+ @bad = Net::Ping::External.new(@bogus)
+ @unreachable = Net::Ping::External.new(@bcast_ip)
end
test "ping basic functionality" do
@@ -120,10 +122,21 @@ def setup
assert_nil(@pe.warning)
end
+ test "pinging an unreachable host returns after the timeout" do
+ @unreachable.timeout = 1
+ tolerance = 0.5
+ start_time = Time.now
+ @unreachable.ping
+ elapsed = Time.now - start_time
+ assert_true(elapsed < @bad.timeout + tolerance)
+ end
+
def teardown
- @host = nil
- @bogus = nil
- @pe = nil
- @bad = nil
+ @host = nil
+ @bogus = nil
+ @bcast_ip = nil
+ @pe = nil
+ @bad = nil
+ @unreachable = nil
end
end
Something went wrong with that request. Please try again.