Skip to content

Force cleanup before destroying socket#83

Open
fourk wants to merge 1 commit into
nlf:masterfrom
fourk:master
Open

Force cleanup before destroying socket#83
fourk wants to merge 1 commit into
nlf:masterfrom
fourk:master

Conversation

@fourk
Copy link
Copy Markdown

@fourk fourk commented Oct 16, 2014

I noticed that we were occasionally not seeing callbacks fire for reads. I then found that there was a 1:1 correlation between occasions where disconnect executed and this.task was non-null and occasions where we saw no callback for a read.

This patch seems to have fixed it for us.

Thanks for the library!

@fourk
Copy link
Copy Markdown
Author

fourk commented Oct 20, 2014

One thing that was unclear was whether this bug also affects streams. If so, the patch would potentially need to have && this.task.callback removed. We don't use streaming so it's unclear whether or not that behavior is bugged in the same way.

@nlf
Copy link
Copy Markdown
Owner

nlf commented Oct 22, 2014

I'm guessing you're probably right and this will have to handle streams too. I'm likely also going to have to make some pooling changes to make this as reliable as possible. Thanks for the patch though, this is a great step in the right direction.

@nlf nlf self-assigned this Oct 22, 2014
@nlf nlf added the bug label Oct 22, 2014
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants