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

Continue with the Sender loop despite closed streams #424

Merged
merged 1 commit into from Aug 16, 2015

Conversation

Projects
None yet
3 participants
@awpr
Member

awpr commented Aug 15, 2015

No description provided.

@awpr

This comment has been minimized.

Member

awpr commented Aug 15, 2015

Finally finished rebasing my changes onto the recent priority changes. I added a hack to close all streams immediately and confirmed that without this change, this made the whole connection end, and with the change, the connection survived.

@awpr

This comment has been minimized.

Member

awpr commented Aug 15, 2015

Context: my comment on #423

@awpr

This comment has been minimized.

Member

awpr commented Aug 15, 2015

(the diff looks big, but it's basically just indenting a bunch of lines by 4 spaces and moving 'loop' outside of 'unlessClosed').

@creichert

This comment has been minimized.

Member

creichert commented Aug 16, 2015

BTW, you can diff with no whitespace using the w=1 query parameter. https://github.com/yesodweb/wai/pull/424/files?w=1

creichert added a commit that referenced this pull request Aug 16, 2015

Merge pull request #424 from awpr/master
Continue with the Sender loop despite closed streams

@creichert creichert merged commit cc4c1bc into yesodweb:master Aug 16, 2015

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
@creichert

This comment has been minimized.

Member

creichert commented Aug 16, 2015

@awpr This is great stuff, thanks a lot

@kazu-yamamoto

This comment has been minimized.

Contributor

kazu-yamamoto commented Aug 17, 2015

@awpr Please separate "layout change only" commits and other commits. If mixed, it's quite hard to review.

@kazu-yamamoto

This comment has been minimized.

Contributor

kazu-yamamoto commented Aug 17, 2015

The point is that loop is the outside of unlessClosed, right?

@kazu-yamamoto

This comment has been minimized.

Contributor

kazu-yamamoto commented Aug 17, 2015

@awpr Sorry for my previous comment. Your patch does not include "layout change only".

@kazu-yamamoto

This comment has been minimized.

Contributor

kazu-yamamoto commented Aug 18, 2015

Now I understand the patch completely. Again, sorry for my confusion.

@kazu-yamamoto

This comment has been minimized.

Contributor

kazu-yamamoto commented Aug 18, 2015

A new version has been released.

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