DouweM / shoutout

A Ruby library for easily getting metadata from Shoutcast-compatible audio streaming servers
MIT License
5 stars 2 forks source link

Thread safe #4

Closed confact closed 9 years ago

confact commented 9 years ago

Is Shoutout thread safe? Seems not because my shoutout is hanging sometimes for no reason.

DouweM commented 9 years ago

I think it is thread safe, actually. Could you investigate where and under what circumstances the hang happens?

confact commented 9 years ago

i have added some rescues and more to see if I can catch it and print it out, to see where it hangs. Will come back when I know more.

confact commented 9 years ago

I added timeout for 60 seconds to stream connect and got this in the TTIN stacktrace:

2014-12-11T10:55:56.672Z 944 TID-6jkc8 WARN: shared/bundle/ruby/2.1.0/bundler/gems/shoutout-99317cac7b28/lib/shoutout/stream.rb:36:in `initialize'
shared/bundle/ruby/2.1.0/bundler/gems/shoutout-99317cac7b28/lib/shoutout/stream.rb:36:in `new'
shared/bundle/ruby/2.1.0/bundler/gems/shoutout-99317cac7b28/lib/shoutout/stream.rb:36:in `connect'
app/workers/get_station_song_worker.rb:18:in `block in perform'
/home/deploy/.rvm/rubies/ruby-2.1.5/lib/ruby/2.1.0/timeout.rb:91:in `block in timeout'
/home/deploy/.rvm/rubies/ruby-2.1.5/lib/ruby/2.1.0/timeout.rb:35:in `block in catch'
/home/deploy/.rvm/rubies/ruby-2.1.5/lib/ruby/2.1.0/timeout.rb:35:in `catch'
/home/deploy/.rvm/rubies/ruby-2.1.5/lib/ruby/2.1.0/timeout.rb:35:in `catch'
/home/deploy/.rvm/rubies/ruby-2.1.5/lib/ruby/2.1.0/timeout.rb:106:in `timeout'

This is running on the fixing_hangs branch in my fork and get stuck in TCPSocket.new. the stream i try to run is: http://icelive0.43660-icelive0.cdn.qbrick.com/4658/43660_iskelma_aikakone.aac

This is a part of the code:

begin
    Timeout.timeout(60) do
      @stream = Shoutout::Stream.new(url)
      begin
        okay = @stream.connect
        if okay != false
          @stream.disconnect
        end
      rescue Errno::ECONNREFUSED
        return false
      rescue Errno::ETIMEDOUT
        return false
      rescue Errno::EHOSTUNREACH
        return false
      rescue SocketError
        return false
      rescue Errno::ECONNRESET
        return false
      rescue URI::InvalidURIError
        return false
      end
    end
rescue Timeout::Error
    if (defined?(@stream)).present?
        @stream.disconnect
    end
    return false
end
DouweM commented 9 years ago

Sounds like the default TCPSocket connection timeout is simply too long, are you sure it's got to do with threads?

To catch timeouts quicker, try something like this: http://spin.atomicobject.com/2013/09/30/socket-connection-timeout-ruby/. (I simply did a Google search for "ruby tcp socket timeout")

confact commented 9 years ago

Yea, it's probably that. I tried it out but it didn't work with the gets and puts. I need to create gets functions for handling to just get one line per call for that and i am new to ruby and don't know how to do that.

confact commented 9 years ago

This is fixed, it was a different issue, not related to thread safe. Added in branch memory-leak-fix, mention in issue #5