Started Analysis and going through details.

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

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?

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.