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

fix: dont open a feed for a replication stream that has been destroyed #10

Merged
merged 1 commit into from Dec 30, 2016

Conversation

Projects
None yet
2 participants
@pfrazee
Collaborator

pfrazee commented Dec 29, 2016

Related to dat-land/hypercloud#16

This one was a tricky one!

There was a case where a feed's storage was being closed prematurely. This led to writes failing. Replication would then stall (see mafintosh/hypercore#54) but did appear to complete successfully if the connection was re-established.

Why was the FD closing prematurely? It's possible that the protocol handshake will uncover an extraneous connection, which needs to be destroyed. Sometimes one of those destroyed connections would be the first to enter the open() logic in this repo. The cleanup() check would see that the stream is destroyed, and that there are no other active streams, and so would call feed.close(). Afterward, other non-destroyed streams would continue processing, and try to write into the feed causing the failure.

It looks like there's no harm in just ignoring open() for any stream that's already destroyed, so that's what this PR does.

@karissa

This comment has been minimized.

Show comment
Hide comment
@karissa

karissa Dec 30, 2016

Contributor

wow nice catch!

Contributor

karissa commented Dec 30, 2016

wow nice catch!

@pfrazee

This comment has been minimized.

Show comment
Hide comment
@pfrazee

pfrazee Dec 30, 2016

Collaborator

Thanks! merging and publishing

Collaborator

pfrazee commented Dec 30, 2016

Thanks! merging and publishing

@pfrazee pfrazee merged commit 72639d2 into master Dec 30, 2016

@pfrazee pfrazee deleted the premature-close-fix branch Dec 30, 2016

@pfrazee

This comment has been minimized.

Show comment
Hide comment
@pfrazee

pfrazee Dec 30, 2016

Collaborator

Published 3.2.3

Collaborator

pfrazee commented Dec 30, 2016

Published 3.2.3

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