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

RedisChannelLayer.receive runs forever #44

Closed
AlexejStukov opened this issue May 24, 2017 · 4 comments
Closed

RedisChannelLayer.receive runs forever #44

AlexejStukov opened this issue May 24, 2017 · 4 comments

Comments

@AlexejStukov
Copy link

RedisChannelLayer.receive, that was reworked in #43, now hangs forever if called with block. That was not the case in the previous version (it returned (None, None). This prevents channels Worker from exiting if termed is set to true (e.g. in the signal handler).

AlexejStukov pushed a commit to AlexejStukov/asgi_redis that referenced this issue May 24, 2017
This should fix django#44. Hope it does not break anything else.
@andrewgodwin
Copy link
Member

The ASGI spec says "block means that the call can take as long as it likes", so this is valid behaviour. What needs fixing here is the channels Worker; if you could file a bug against the channels project we can fix it over there so an interrupt correctly yanks it out of any receive call.

@AlexejStukov
Copy link
Author

AlexejStukov commented May 24, 2017

@andrewgodwin: Done.
@andrewgodwin & @proofit404: Maybe I will try my luck at fixing this in channels tomorrow. Thanks for your work, guys!

@AlexejStukov
Copy link
Author

@andrewgodwin maybe the ASGI spec should be reworded, because in the same paragraph there is the sentence: "If block is True, then it will not return until after a built-in timeout or a message arrives;"
For me that means that it should eventually return after a non infinite timeout.

@andrewgodwin
Copy link
Member

Yes, I will clarify that paragraph so it's clear it is allowed to block forever.

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 a pull request may close this issue.

2 participants