RFR(S): 8199813: SIGSEGV in ThreadsList::includes()
Daniel D. Daugherty
daniel.daugherty at oracle.com
Sat Mar 24 01:00:15 UTC 2018
Thanks for the quick review!!
On 3/23/18 5:45 PM, Gerald Thornbrugh wrote:
> Hi Dan,
> Your changes look good to me.
> On 03/23/2018 02:55 PM, Daniel D. Daugherty wrote:
>> I have a small fix for a Thread-SMR bug:
>> JDK-8199813 SIGSEGV in ThreadsList::includes()
>> Here's the webrev URL:
>> The failure mode for this bug is a SIGSEGV in ThreadsList::includes().
>> The cause of this failure is use of a ThreadsListHandle by a JavaThread
>> that is not yet on Threads list. This failure mode is intermittent since
>> the ThreadsListHandle refers to a ThreadsList that may or may not get
>> deleted by a racing thread. In particular, it has to get deleted in such
>> a way that use of the ThreadsList causes a crash, e.g., page is freed.
>> This fix has been tested on my Solaris-X64 server using my usual 24+
>> hour Thread-SMR stress testing config. It has also been through Mach5
>> hs-tier[1-5] testing. There have been no unexpected failures.
>> This fix is currently going thru a Mach5 builds-tier1, jdk-tier[1-3],
>> and hs-tier[1-3] run after resyncing; we don't want any new asserts()
>> popping up in the jdk/jdk CI...
>> Since this is a race fix, here are some details:
>> As is usual for race fixes, there are more comments than code here.
>> Use of a ThreadsListHandle by verify_not_published() before the
>> JavaThread is on the Thread list; this is the minimal fix for
>> the failure mode.
>> Save the shutdown thread in Threads::destroy_vm(). Also add a
>> comment explaining why IdealGraphPrinter::clean_up() is done
>> where it is.
>> Another use of a ThreadsListHandle before the JavaThread is on
>> the Threads list; found during testing of the fix by the new
>> assert() in the ThreadsListHandle ctr.
>> Add an assertion to the ThreadsListHandle ctr that catches
>> use of a ThreadsList that Threads::threads_do() won't be
>> able to protect.
>> Note: There is an exemption for the shutdown thread here.
>> Expose the shutdown thread so the new assert() in the
>> ThreadsListHandle ctr can do its job.
>> VM_Exit::_shutdown_thread should be volatile since it is
>> set by one thread and examined lock free by one or more
>> The callers of VM_Exit::set_vm_exited() should set the
>> _shutdown_thread field since they have the earliest context.
>> Save the shutdown thread in VM_Exit::doit().
>> Thanks, in advance, for any comments, suggestions, or feedback.
More information about the hotspot-runtime-dev