Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Shutdown GirlFriday::WorkQueue on connection close #141

Closed

Conversation

emschwar
Copy link
Contributor

Part 2 of 2 to help address #139

Depends on #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
Copy link
Contributor Author

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 #140, which is documented there.

@benlangfeld
Copy link
Member

That comment came across from the commit that was duplicated from #140, and yes, you have indeed resolved it at emschwar@c22948a.

@emschwar
Copy link
Contributor Author

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
Copy link
Member

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

Eric Schwartz added 2 commits August 18, 2014 15:45
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.
* If it failed to open properly, @stream might not be set, which means
  that calling #close on the Blather::Client might not be safe (it will
  raise a StreamNotReady error in the bowels of EventMachine).

* If a stream is not connected (i.e., there is no @stream or it's been
  stopped due to EM having called #unbind on it), then don't try to
  close it on Blather::Client#close
@emschwar
Copy link
Contributor Author

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

@emschwar emschwar closed this Aug 18, 2014
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants