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

ActionCable, sometimes add_channel is not called. #25030

Merged
merged 1 commit into from
Jul 3, 2016

Conversation

mmmpa
Copy link
Contributor

@mmmpa mmmpa commented May 16, 2016

Summary

One channel could not broadcast all messages. That was happened when one connected, after no one has a connection after someone disconnected.

In a case,

class OneChannel < ApplicationCable::Channel
  def subscribed
    stream_from 'one_stream'
  end

  def unsubscribed
     ActionCable.server.broadcast('one_stream', {type: 'leave'})
  end
end

Sometimes (about once every 7 times), SubscriberMap#broadcast is called after all channels are removed from @subscribers of SubscriberMap.

Then

@subscribers # => {one_stream: []}

Then next consumer comes, SubscriberMap#add_channel is not called, because new_channel = !@subscribers.key?(channel) is false.

SubscriberMap do not add channel for one_stream, until SubscriberMap#add_subscriber is called again after all channels are removed again.

@rails-bot
Copy link

Thanks for the pull request, and welcome! The Rails team is excited to review your changes, and you should hear from @kaspth (or someone else) soon.

If any changes to this PR are deemed necessary, please add them as extra commits. This ensures that the reviewer can see what has changed since they last reviewed the code. Due to the way GitHub handles out-of-date commits, this should also make it reasonably obvious what issues have or haven't been addressed. Large or tricky changes may require several passes of review and changes.

Please see the contribution instructions for more information.

@maclover7
Copy link
Contributor

Can you add a regression test?

@mmmpa
Copy link
Contributor Author

mmmpa commented May 17, 2016

I have no idea for the test.

@maclover7
Copy link
Contributor

Hmm, maybe try adding a test where @subscribers is an empty Hash, then check out this new behavior with a call to broadcast?

@mmmpa
Copy link
Contributor Author

mmmpa commented May 17, 2016

Thank you. I tried.

@matthewd matthewd merged commit 550303c into rails:master Jul 3, 2016
matthewd added a commit that referenced this pull request Jul 3, 2016
ActionCable, sometimes add_channel is not called.
@matthewd
Copy link
Member

matthewd commented Jul 3, 2016

Backported as 81a16cb. Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants