RFR (S): 8211909: JDWP Transport Listener: dt_socket thread crash

[David Holmes]

Thanks Serguei!

David

On 18/10/2018 9:00 AM, serguei.spitsyn at oracle.com wrote:
> Hi David,
> 
> Sorry for being late but just wanted to tell you that it looks good to me.
> Thank you for catching and taking care about it!
> 
> Thanks,
> Serguei
> 
> 
> On 10/14/18 22:12, David Holmes wrote:
>> Bug: https://bugs.openjdk.java.net/browse/JDK-8211909
>> webrev: http://cr.openjdk.java.net/~dholmes/8211909/webrev/
>>
>> The crash occurs when trying to access a thread that was returned as 
>> part of JVM TI GetThreadGroupChildren. The problem is that the JVM TI 
>> code tries to use the Threads_lock to ensure atomic access to the 
>> ThreadGroup's threads and child groups:
>>
>>     { // Cannot allow thread or group counts to change.
>>       MutexLocker mu(Threads_lock);
>>
>> but the Threads_lock does not control concurrency in the Java code 
>> that can cause threads to be added and removed, so we do not get a 
>> stable snapshot of the thread array and its length, and contents. To 
>> get a stable snapshot we have to use the same synchronization 
>> mechanism as used by the Java code: lock the monitor of the 
>> ThreadGroup instance.
>>
>> Two other pointless acquisitions of the Threads_lock, in GetThreadInfo 
>> and GetThreadGroupInfo, were also removed. These functions could 
>> report an inconsistent snapshot of the Thread or ThreadGroup state, if 
>> the mutable state is mutated concurrently. Again we could acquire the 
>> object's monitor to prevent this but it is unclear that there is any 
>> real benefit in doing so - after all the thread/group information 
>> could change immediately after it has been read anyway. So here I just 
>> delete the Threads_lock usage.
>>
>> Testing: mach5 tier 1-3
>>          JVM TI tests (serviceability/jvmti vmTestbase/nsk/jvmti/)
>>
>> A regression test may be possible if we call GetThreadGroupChildren in 
>> a loop, whilst threads add and remove themselves from the group 
>> concurrently. But it is awkward to write.
>>
>> Thanks,
>> David
>>
>> Thanks,
>> David
>