adhearsion / blather

XMPP/Jabber Library and DSL for Ruby written on EventMachine and Nokogiri.
adhearsion.com/blather
Other
557 stars 89 forks source link

Shutdown GirlFriday::WorkQueue on connection close #141

Closed emschwar closed 10 years ago

emschwar commented 10 years ago

Part 2 of 2 to help address https://github.com/adhearsion/blather/issues/139

Depends on https://github.com/adhearsion/blather/pull/140 (for lazy initialization of handler_queue)

Since the handler_queue is lazily initialized, all we have to do at connection close time is shutdown the queue and nil it out. Any subsequent reference (such as in #receive_data) will automatically re-create it, if necessary.

emschwar commented 10 years ago

Honest question-- what docs would you like to see here? As far as I'm concerned, this should be invisible to a Blather user; other than the :workqueue_count that I added in https://github.com/adhearsion/blather/pull/140, which is documented there.

benlangfeld commented 10 years ago

That comment came across from the commit that was duplicated from #140, and yes, you have indeed resolved it at https://github.com/emschwar/blather/commit/c22948ad0df7c76a4af99f7c5ad1816ed3cdcbfc.

emschwar commented 10 years ago

This last commit arose because I noticed that if there was an error setting up the stream in the first place (say, we provided a bad password and thus never successfully auth'd) that the workqueues weren't getting closed. I traced this back to another issue, which was that calling Blather::Client#close on a client without a @stream that was configured properly in post_init meant that we'd raise a RuntimeError.new("Stream not ready!") in the bowels of EventMachine when trying to shut it down.

This commit could possibly be a separate PR, but since I need it as part of this effort to ensure workqueues always get shutdown properly, I hope you'll allow it here.

benlangfeld commented 10 years ago

Once #140 is squashed/rebased, please rebase this atop that.

emschwar commented 10 years ago

Rebasing off a rebased branch left git looking ugly; I'm going to close this and re-open based off of #140