stompgem / stomp

A ruby gem for sending and receiving messages from a Stomp protocol compliant message queue. Includes: failover logic, ssl support.
http://stomp.github.com
Apache License 2.0
152 stars 80 forks source link

stomp 1.1 multithreading #46

Closed ripienaar closed 12 years ago

ripienaar commented 12 years ago

hello,

Given the code


require 'rubygems'
require 'stomp'
require 'pp'

class StompLogger
  def on_miscerr(params, errstr)
    puts "Unexpected error on connection to stomp://%s@%s:%d %s" % [params[:cur_login], params[:cur_host], params[:cur_port], errstr]
    pp caller
  end
end

options = { :stompuser => "dev", :stomppass => "secret", :stompserver => "1.2.3.4", :stompport => 6163 }

connection = {:hosts => [
                          {:login => options[:stompuser], :passcode => options[:stomppass], :host => options[:stompserver], :port => options[:stompport]},
                        ], :logger => StompLogger.new, :connect_headers => {:"heart-beat" => "10000,10000", :host => "stomp", :"accept-version" => "1.1,1.0"}}

conn = Stomp::Connection.open(connection)

p conn.connection_frame.headers

Thread.new(conn) do |conn|
  while true
    p conn.receive
  end
end

puts "Sleeping....."

sleep 10000

So there's a background thread that just loops and receives and prints what it gets while the rest of the program continues. I am not sure if this is the intended use pattern for achiving this but it does work in 1.0 mode quite well and I've used this style of code all over.

When 1.1 protocol heartbeats are enabled this happens after some time:

{"heart-beat"=>"10000,10000", "version"=>"1.1", "session"=>"ID:stomp-34412-1340368192346-3:100", "server"=>"ActiveMQ/5.6.0"}
Sleeping.....
Unexpected error on connection to stomp://dev@1.2.3.4:6163 receive failed: execution expired
["/home/rip/.gem/gems/stomp-1.2.3/lib/stomp/connection.rb:428:in `__old_receive'",
 "/home/rip/.gem/gems/stomp-1.2.3/lib/stomp/connection.rb:438:in `receive'",
 "test.rb:24",
gmallard commented 12 years ago

Thanks. This recreates on my development machine. Will do some more investigation over the weekend.

The code looks a bit strange because there is no subscribe, but that has no bearing on the issue described.

ripienaar commented 12 years ago

Indeed, usually would have a subscribe this was the simplest code to demonstrate the problem

I just took this code from my stomp irb (ripienaar/stomp-irb) which starts off without a subscription but it makes no diff either way.

thanks!

gmallard commented 12 years ago

The cause is something that was anticipated during 1.1 development, and was ...... apparently only partially coded for in the heartbeat receive thread. Clearly we missed something. I believe what is happening is that in the '_receive' method, this line:

line = read_socket.gets

is consuming heartbeat data (a '\n') and not handling it correctly. That line normally consumes an entire command, such as 'CONNECTED\n" or 'MESSAGE\n'.

When I have a fix that works I'll report here.

gmallard commented 12 years ago

I can get you a gem to smoke test if you would like. Let me know if you want to test that way ....

.... Or in connection.rb, you can replace line #519, this line:

line = read_socket.gets

with this code:

          line = ''
          if @protocol == Stomp::SPL_10 || (@protocol >= Stomp::SPL_11 && !@hbr)
            line = read_socket.gets # The old way
          else # We are >= 1.1 and receiving heartbeats.
            while true
              line = read_socket.gets # Data from wire
              break unless line == "\n"
              line = ''
              @lr = Time.now.to_f
            end
          end

Let me know.

Very sweet. The first real 1.1 bug.

ripienaar commented 12 years ago

either is fine, I can test this fix in a hour or two :)

gmallard commented 12 years ago

Go ahead and put the patch in please.

I'll cut and release a new gem when you feel you are complete with your 1.1 testing.

But lets see if anything else is uncovered in that process.

ripienaar commented 12 years ago

Patch applied and it works - I have a bunch of other 1.1 fixes to do before I can use this in my larger projects

The stomp-irb in my github account has now been updated to include 1.1 and from doing a bunch of sending/receiving/subscribing etc this patch seems to do it for me

gmallard commented 12 years ago

Fix is in 4375706 and will be in the next release.

ripienaar commented 12 years ago

thank you :)