Connection IO not entirely managed by IO Processor

[iande]

The following code illustrates a problem with how the client, io processor and event dispatching interact:

client = OnStomp::Client.open "stomp://localhost"

client.on_connection_closed do
  client.connect
end

This code introduces a race condition in both the 1.0 branch and what will be the 1.1 branch. In the 1.0 branch, the threaded processor works with the connection attribute of a client and as that attribute isn't synchronized, it may be possible to end up with multiple IO processor threads operating on the same connection object. This problem was pointed out in issue #12

In what will be the 1.1 branch, the issue is slightly different but just as serious: the system dead-locks as client.connect will not complete until certain event handlers are processed, but the dispatcher won't trigger those events until it has completed the triggering of on_connection_closed.

The root of the problem is that establishing the connection is being handled in whatever thread invoked client.connect, and the method blocks until the connection hand-shake has completed. The IO processor should handle creating and establishing the connection in addition to managing all the reads and writes.

It's also important to note that there will always be some interaction between the processor thread and the event dispatch thread, if we want to allow before_* events to modify outgoing frames. Our only choice is to delay enqueueing the frame to write until all of the related events have been triggered.

A proper solution to this problem will also provide a more robust solution to issue #12

[celesteking]

on a side note, when RST is received in the middle of communication during transaction from the broker, the following happens:

/usr/lib64/ruby/gems/1.8/gems/onstomp-1.0.5/lib/onstomp/connections/base.rb:314:in `read_nonblock': Connection reset by peer (Errno::ECONNRESET)
        from /usr/lib64/ruby/gems/1.8/gems/onstomp-1.0.5/lib/onstomp/components/threaded_processor.rb:57:in `join'
        from /usr/lib64/ruby/gems/1.8/gems/onstomp-1.0.5/lib/onstomp/components/threaded_processor.rb:57:in `join'
        from /usr/lib64/ruby/gems/1.8/gems/onstomp-1.0.5/lib/onstomp/client.rb:104:in `disconnect'
        from /usr/lib64/ruby/gems/1.8/gems/onstomp-1.0.5/lib/onstomp/client.rb:103:in `tap'
        from /usr/lib64/ruby/gems/1.8/gems/onstomp-1.0.5/lib/onstomp/client.rb:103:in `disconnect'
        from ./lib/sslchecker_client.rb:211:in `main'
        from ./lib/sslchecker_client.rb:224

This is wrong. First of all, you should handle exceptions. Secondly, you should call a callback for that transaction. on_abort would be a good candidate.

btw, i'm no expert, but the code itself is hard to follow.

[iande]

I agree with you on both points. The reason there is only limited exception handling at the moment is that I've been trying to come up with a good way to handle them. Given that the exceptions are being raised within a separate thread, the user won't have the opportunity to rescue them until the IO thread is joined to the main thread (for instance, when client.disconnect is called) and of course by then it's much to late to do anything meaningful with the error. Some events are triggered when IO exceptions are raised (on_connection_terminated, on_connection_closed), but I'm currently re-raising the exception because these events alone don't provide much in the way of useful information as to why the connection has failed.

The RST packet should trigger on_connection_terminated, but a transaction level event or callback would be much more useful in letting the user directly handle a failed transaction, instead of having to do some book-keeping and watching for transaction failure within an on_connection_terminated handler.

I also agree about the code being hard to follow. I've been considering factoring out some of it into separate gems (or removing some of the code in favor of existing gems that do the same thing.) This is particular true of the EventManager and UriConfigurable classes. The actual IO handling and Stomp protocol support will probably always be a bit complex, but I have been working on simplifying them as well.

[celesteking]

Wondering why aren't you using eventmachine for I/O processing?

[iande]

Stomper and OnStomp were initially developed to address I problem I was having with the existing Stomp gem across a semi-unreliable network. Issues with read and write buffering in my environment caused Stomp to hang, which in turn choked the application I needed it for. So I wrote Stomper (and then OnStomp) to be fairly close to a drop-in replacement, but using Ruby's non-blocking IO methods. Switching over to eventmachine would have required re-writing a fair amount of the application itself to run within the event loop.

[iande]

Refactoring the IO processing out into a separate gem. Shooting for v1.1 to have this addressed.

[iande]

I wrote up some thoughts on how I'd like to solve this and other issues: http://mathish.com/2012/01/08/io-unblock.html Feel free to give any feedback on the direction I'm taking.