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

interrupt the app_iter if it tries to write to a closed socket #238

Merged
merged 3 commits into from Mar 28, 2019

Conversation

@mmerickel
Copy link
Member

@mmerickel mmerickel commented Mar 26, 2019

fixes #236

@mmerickel mmerickel changed the title interrupt the app_iter if it tries to write to a closed socket WIP interrupt the app_iter if it tries to write to a closed socket Mar 26, 2019
@mmerickel mmerickel force-pushed the close-app-iter-on-disconnect branch from d4d8729 to 5a5418b Mar 26, 2019
@mmerickel mmerickel force-pushed the close-app-iter-on-disconnect branch from 5a5418b to 29fb97d Mar 26, 2019
if not self.connected:
# if the socket is closed then interrupt the task so that it
# can cleanup possibly before the app_iter is exhausted
# XXX a simple boolean check should not require a lock since it's
Copy link
Member

@bertjwregeer bertjwregeer Mar 26, 2019

s/XXX/NB/ and doing an if on a boolean here is safe in CPython, and probably other interpreters, and there is no race.

I don't think this comment is necessary to be honest.

self.channel.write_soon(app_iter)
return

can_close_app_iter = True
Copy link
Member

@bertjwregeer bertjwregeer Mar 26, 2019

+1 for this solution.

@mmerickel mmerickel changed the title WIP interrupt the app_iter if it tries to write to a closed socket interrupt the app_iter if it tries to write to a closed socket Mar 26, 2019
@bertjwregeer bertjwregeer merged commit bdda44a into master Mar 28, 2019
4 checks passed
@bertjwregeer bertjwregeer deleted the close-app-iter-on-disconnect branch Mar 28, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

2 participants