[RFR] Cleanup and add lots of new tests

Man Cao manc at google.com
Wed Jun 5 21:23:21 UTC 2019


Thanks for converting these tests!

For NonRacyStaticInitLoopTest.java, can we create a separate thread to read
from the static field? I think currently there is only one main thread that
initialize and read from the static field.

In NonRacySyncBlockExceptionLoopTest.java and
NonRacySyncBlockLoopTest.java, the @summary line could be more descriptive.

In NonRacySyncBlockLoopTest.java:
protected synchronized void run(int i) {
  synchronized (this) {
The run() method does not need to be synchronized, right?

For next code version, no need for a webrev, just inline an updated
NonRacyStaticInitLoopRunner
class in email would work.

-Man


On Wed, Jun 5, 2019 at 9:57 AM Jean Christophe Beyler <jcbeyler at google.com>
wrote:

> Fair enough. I think it adds really nothing from a TSAN point of view and I
> don't think it changes much. The only difference I really see is that the
> second element of the array is initialized by the main thread but no one
> touches that element.
>
> I let you decide if you want to leave it; I think it does not really make
> it more complex from a TSAN point of view :)
>
> LGTM either way and I don't need a webrev even if you do update it,
> Jc
>
> On Wed, Jun 5, 2019 at 9:02 AM Arthur Eubanks <aeubanks at google.com> wrote:
>
> > Looks good to me; by curiosity, why do we allocate two elements for the
> >> arrays?
> >>
> > I thought it'd be good if there was one element we accessed and one we
> > didn't. It almost certainly doesn't add any test coverage, but I kind of
> > liked that it was a little more complex than just a one element array.
> >
>
>
> --
>
> Thanks,
> Jc
>


More information about the tsan-dev mailing list