RFR: Add JVMTI raw monitor lock/unlock mechanisms
Arthur Eubanks
aeubanks at google.com
Wed Apr 24 17:18:00 UTC 2019
Yeah we should have another tsan header. Kind of like how our interface for
the other TSAN functions is through SharedRuntime. Only that file would
include tsanExternalDecls.hpp. The other files would include the new header
file.
On Wed, Apr 24, 2019 at 10:14 AM Jean Christophe Beyler <jcbeyler at google.com>
wrote:
> Hi Arthur,
>
> What do you mean: "pulling in tsanExternalDecls.hpp" is wrong? Seems like
> it is what we did internally and what you were doing for the lock webrev as
> well, ie have that as our include header. Are you suggesting we make
> another tsan header for these methods.
>
> Ok for all other points (except the indentation, just the way webrev does
> things by default, the indentation is actually correct in my code),
> Jc
>
> On Wed, Apr 24, 2019 at 9:58 AM Arthur Eubanks <aeubanks at google.com>
> wrote:
>
>> Overall:
>> Since AnnotateRWLock* are direct TSAN calls, I think we should have a
>> wrapper around them. Then we can have things like checking if the lock is
>> NULL, etc, in a central location. And pulling in tsanExternalDecls.hpp
>> seems wrong.
>> "#if INCLUDE_TSAN" ends with "#endif // INCLUDE_TSAN", could you add the
>> comment for the #endifs.
>>
>> test/hotspot/jtreg/tsan/TsanRunner.java
>> Could you rename "args" to "vmArgs"?
>>
>> src/hotspot/share/prims/jvmtiEnv.cpp.udiff
>> Indentation around 3258 is wrong.
>>
>>
>> On Tue, Apr 23, 2019 at 2:29 PM Jean Christophe Beyler <
>> jcbeyler at google.com> wrote:
>>
>>> Hi all,
>>>
>>> I added the locks for the JVMTI raw monitors and added two tests, it is
>>> failing the non racy ones due to the JVM locks not yet included but I
>>> think
>>> that will resolve itself soon :)
>>>
>>> I also moved the agent library to C++.
>>>
>>> FWIW, we also need to clean up our tests that check for failing, right
>>> now
>>> they pass because they generically just are checking for a TSAN race but
>>> not specifically which one, which is a bit wrong / imprecise.
>>>
>>> Webrev: http://cr.openjdk.java.net/~jcbeyler/jvmti/webrev.00/
>>>
>>> Let me know what you think,
>>> Jc
>>>
>>
>
> --
>
> Thanks,
> Jc
>
More information about the tsan-dev
mailing list