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

Add suppressions for GuardedBy violations #6566

Merged
merged 7 commits into from Dec 30, 2019

Conversation

graememorgan
Copy link
Contributor

This supports releasing a new version of GuardedBy which finds more mistakes than it used to.

@linux-foundation-easycla
Copy link

linux-foundation-easycla bot commented Dec 21, 2019

CLA Check
The committers are authorized under a signed CLA.

@ran-su ran-su added the kokoro:run Add this label to a PR to tell Kokoro the code is safe and tests can be run label Dec 23, 2019
@grpc-kokoro grpc-kokoro removed the kokoro:run Add this label to a PR to tell Kokoro the code is safe and tests can be run label Dec 23, 2019
@dapengzhang0
Copy link
Member

@garrettjonesgoogle There might be more need to suppress

/git/grpc-java/okhttp/src/main/java/io/grpc/okhttp/OkHttpClientTransport.java:433: error: [GuardedBy] This access should be guarded by 'stream.transportState().lock'; instead found: 'this.lock'
    stream.transportState().start(nextStreamId);
                           ^
    (see https://errorprone.info/bugpattern/GuardedBy)
/git/grpc-java/okhttp/src/main/java/io/grpc/okhttp/OkHttpClientTransport.java:1139: error: [GuardedBy] This access should be guarded by 'stream.transportState().lock'; instead found: 'OkHttpClientTransport.this.lock'
          stream.transportState().transportDataReceived(buf, inFinished);
                                 ^
    (see https://errorprone.info/bugpattern/GuardedBy)
/git/grpc-java/okhttp/src/main/java/io/grpc/okhttp/OkHttpClientTransport.java:1189: error: [GuardedBy] This access should be guarded by 'stream.transportState().lock'; instead found: 'OkHttpClientTransport.this.lock'
            stream.transportState().transportHeadersReceived(headerBlock, inFinished);
                                   ^
    (see https://errorprone.info/bugpattern/GuardedBy)
/git/grpc-java/okhttp/src/main/java/io/grpc/okhttp/OkHttpClientStream.java:263: error: [GuardedBy] This access should be guarded by 'OkHttpClientStream.this.state.lock'; instead found: 'this.lock'
      state.onStreamAllocated();
           ^
    (see https://errorprone.info/bugpattern/GuardedBy)
/git/grpc-java/okhttp/src/main/java/io/grpc/okhttp/OkHttpClientStream.java:376: error: [GuardedBy] This access should be guarded by 'this.transport.lock'; instead found: 'this.lock'
        transport.removePendingStream(OkHttpClientStream.this);
                 ^
    (see https://errorprone.info/bugpattern/GuardedBy)
/git/grpc-java/okhttp/src/main/java/io/grpc/okhttp/OkHttpClientStream.java:419: error: [GuardedBy] This access should be guarded by 'this.transport.lock'; instead found: 'this.lock'
      transport.streamReadyToStart(OkHttpClientStream.this);
               ^
    (see https://errorprone.info/bugpattern/GuardedBy)

@graememorgan
Copy link
Contributor Author

@garrettjonesgoogle There might be more need to suppress

...

You're quite right. Done.

@suztomo
Copy link
Contributor

suztomo commented Dec 30, 2019

Note for myself: Once this PR is merged to master, I'll check #6574.

@ejona86 ejona86 added the kokoro:run Add this label to a PR to tell Kokoro the code is safe and tests can be run label Dec 30, 2019
@grpc-kokoro grpc-kokoro removed the kokoro:run Add this label to a PR to tell Kokoro the code is safe and tests can be run label Dec 30, 2019
@ejona86
Copy link
Member

ejona86 commented Dec 30, 2019

I've filed #6578 for us to try to clean up the suppressions.

@dapengzhang0
Copy link
Member

@graememorgan Thanks for the PR.

@dapengzhang0 dapengzhang0 merged commit d3c77f2 into grpc:master Dec 30, 2019
@lock lock bot locked as resolved and limited conversation to collaborators Apr 2, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants