[binder] Fix WireReaderImpl bugs & races#27303
Merged
TaWeiTu merged 1 commit intogrpc:masterfrom Sep 11, 2021
Merged
Conversation
TaWeiTu
added
the
release notes: no
sifmelcara
approved these changes
Sep 11, 2021
TaWeiTu
force-pushed
the
fix-wire-reader-race
branch
from
df5eb9c to
1724178
Compare
Contributor
Author
|
Modified tests slightly to make them work. @sifmelcara would you mind taking another look before I merge this? |
TaWeiTu
force-pushed
the
fix-wire-reader-race
branch
from
1724178 to
aac443b
Compare
Contributor
|
LGTM |
TaWeiTu
force-pushed
the
fix-wire-reader-race
branch
2 times, most recently
from
1d2ff5e to
f639c93
Compare
There was a bug found by the fuzzer where we might access wire_writer_ before finishing SETUP_TRANSPORT (and thus constructing wire_writer_). This PR fixes such issue by making sure that we won't proceed with any requests until the connection is fully established. Since binder transactions may be coming from multiple different threads, this PRs guard some of the WireReaderImpl's member with a mutex to make sure there's no races between threads.
TaWeiTu
force-pushed
the
fix-wire-reader-race
branch
from
f639c93 to
2fbff87
Compare
lidizheng
pushed a commit
to lidizheng/grpc
that referenced
this pull request
Sep 23, 2021
There was a bug found by the fuzzer where we might access wire_writer_ before finishing SETUP_TRANSPORT (and thus constructing wire_writer_). This PR fixes such issue by making sure that we won't proceed with any requests until the connection is fully established. Since binder transactions may be coming from multiple different threads, this PRs guard some of the WireReaderImpl's member with a mutex to make sure there's no races between threads.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
There was a bug found by the fuzzer where we might access wire_writer_ before
finishing SETUP_TRANSPORT (and thus constructing wire_writer_). This PR
fixes such issue by making sure that we won't proceed with any requests
until the connection is fully established.
Since binder transactions may be coming from multiple different threads,
this PRs guard some of the WireReaderImpl's member with a mutex to make
sure there's no races between threads.