Try to remove SuppressWarnings("GuardedBy")

[ejona86]

#6566 introduced suppressions to allow being compatible with newer versions of Error Prone. But we should spend some time to see if we could tweak the code so that the suppressions would be unnecessary.

If we can't remove them or if it would make the code worse, we can leave them as-is and close this issue.

[vinodhabib]

Started Analysis and going through details.

[kannanjgithub]

This comment for example, says

// TODO(b/145386688): This access should be guarded by 'stream.transportState().lock'; instead
// found: 'this.lock'

Why should it be guarded instead by stream.transportState().lock? What was the change in behavior that CL/282908895 introduced in ErrorProne? At least the above usage seems like would make the code worse if we don't guard it on this.lock that it does today. @ejona86

[vinodhabib]

This comment for example, says

// TODO(b/145386688): This access should be guarded by 'stream.transportState().lock'; instead
// found: 'this.lock'

Why should it be guarded instead by stream.transportState().lock? What was the change in behavior that CL/282908895 introduced in ErrorProne? At least the above usage seems like would make the code worse if we don't guard it on this.lock that it does today. @ejona86

@ejona86 do you have any further updates on this?

[ejona86]

At least the above usage seems like would make the code worse if we don't guard it on this.lock that it does today.

I wasn't suggesting we remove necessary locks. I was suggesting organize the code such that the tooling can verify the locking. stream.transportState().lock is the same object as this.lock, but ErrorProne isn't able to figure that out because it requires data flow analysis.