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::Connection#poll issues Hard Receive #30

Closed gmallard closed 12 years ago

gmallard commented 12 years ago

In the current gem, Connection#poll will hang in a hard receive, waiting on messages when none can possibly be present.

It should be asynchronous, and return 'nil' if there is nothing to be received.

morellon commented 12 years ago

Hi. Guy. Why do you think this is a unwanted behavior?

gmallard commented 12 years ago

Hi Thiago - A couple of reasons:

The comment and rdoc say:

Return a pending message if one is available, otherwise return nil

Plus, I know that is how it behaved in the past - you get a nil if there is nothing to read. It should basically allow asynchronous receives.

And the code looks like that is what it is supposed to do.

This is something that was introduced into the gem between 1.1.9 and 1.1.10. That is all I know right now.

Try this code with 1.1.9 and with 1.1.10:

require 'rubygems'
require 'stomp'
#
c = Stomp::Connection.new("me", "mypass", "localhost", 61613)
c.subscribe("/queue/nothing.here")
puts "starting poll"
m = c.poll
p [ "msg", m ]
puts "ending poll"
gmallard commented 12 years ago

Bisect tells me:

245e734a0f4a3c3097fdffb7c8dc9b2380c98958 is the first bad commit

gmallard commented 12 years ago

I understand what broke, and why it broke.

I thought we had a test that would have detected this - but we do not.

Basically, leaving the extra line ends when a message is read, breaks this test in the 'poll' method:

return nil if @socket.nil? || !@socket.ready?
gmallard commented 12 years ago

Fixed by: ce8335fb2f87f472a6e7ae1625172901e8db9e3a

This commit reverts 245e734 but retains logic added since the reverted commit.

morellon commented 12 years ago

You are totally right about that, Guy. At first, when you commented, I thought it was supposed to be a feature :P Nicely done.