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

bufconn: Implement read/write deadlines #2959

Merged
merged 1 commit into from Aug 20, 2019

Conversation

Maydell
Copy link
Contributor

@Maydell Maydell commented Aug 6, 2019

This PR adds implementations for the Set(Read|Write)Deadline functions of the net.Conn interface. This functionality can be useful when testing I/O edge-cases or unreliable network connections.

@thelinuxfoundation
Copy link

Thank you for your pull request. Before we can look at your contribution, we need to ensure all contributors are covered by a Contributor License Agreement.

After the following items are addressed, please respond with a new comment here, and the automated system will re-verify.

Regards,
CLA GitHub bot

@thelinuxfoundation
Copy link

Thank you for your pull request. Before we can look at your contribution, we need to ensure all contributors are covered by a Contributor License Agreement.

After the following items are addressed, please respond with a new comment here, and the automated system will re-verify.

Regards,
CLA GitHub bot

@Maydell
Copy link
Contributor Author

Maydell commented Aug 6, 2019

I think I have set everything up now!

@Maydell Maydell force-pushed the bufconn-deadlines branch 2 times, most recently from ac4f525 to 8dcb2d4 Compare August 6, 2019 21:49
@menghanl menghanl requested a review from dfawley August 8, 2019 20:05
test/bufconn/bufconn.go Outdated Show resolved Hide resolved
test/bufconn/bufconn.go Outdated Show resolved Hide resolved
@dfawley dfawley assigned Maydell and unassigned dfawley Aug 15, 2019
@Maydell Maydell changed the title Implement read/write deadlines for bufconn [WIP] Implement read/write deadlines for bufconn Aug 16, 2019
@Maydell Maydell force-pushed the bufconn-deadlines branch 5 times, most recently from cb5ffd5 to 2e7d845 Compare August 16, 2019 08:48
@Maydell Maydell changed the title [WIP] Implement read/write deadlines for bufconn Implement read/write deadlines for bufconn Aug 16, 2019
@Maydell
Copy link
Contributor Author

Maydell commented Aug 16, 2019

Sorry for making such big changes after the initial review but I got a better idea to solve this PR.

Copy link
Member

@dfawley dfawley left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good, thanks! Just a couple minor things and some test-related things.

test/bufconn/bufconn.go Outdated Show resolved Hide resolved
test/bufconn/bufconn.go Outdated Show resolved Hide resolved
test/bufconn/bufconn_test.go Outdated Show resolved Hide resolved
}

p1, p2 := newPipe(5), newPipe(5)
c1, c2 := &conn{p1, p2}, &conn{p1, p2}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shouldn't c2 be &conn{p2, p1}?

Copy link
Contributor Author

@Maydell Maydell Aug 20, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As we write to c1 that would make c2 able to read from p2, and no longer be blocking. I changed it to c1, c2 := &conn{p1, p1}, &conn{p2, p2} to make this clearer.

test/bufconn/bufconn_test.go Outdated Show resolved Hide resolved
test/bufconn/bufconn_test.go Show resolved Hide resolved
test/bufconn/bufconn_test.go Show resolved Hide resolved
test/bufconn/bufconn_test.go Outdated Show resolved Hide resolved
@dfawley dfawley changed the title Implement read/write deadlines for bufconn bufconn: Implement read/write deadlines Aug 20, 2019
Copy link
Member

@dfawley dfawley left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks great, thank you for the contribution!

@dfawley dfawley merged commit 3bb34e5 into grpc:master Aug 20, 2019
@Maydell
Copy link
Contributor Author

Maydell commented Aug 20, 2019

Thank you! This was my first open source contribution so thank you for the guidance and feedback :)

@lock lock bot locked as resolved and limited conversation to collaborators Feb 16, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants