RFR: 8183925: Decouple crash protection from watcher thread

[Thomas Stüfe]

Hi Robbin,

Change looks fine.

Kind Regards, Thomas

On Fri, Jul 7, 2017 at 8:59 AM, Robbin Ehn <robbin.ehn at oracle.com> wrote:

> On 07/07/2017 05:43 AM, Daniel D. Daugherty wrote:
>
>> On 7/6/17 4:33 AM, Robbin Ehn wrote:
>>
>>> Hi all, please review:
>>>
>>> While looking into:
>>> java_lang_Thread::get_thread_status(oop java_thread) {
>>>      4802:   assert(Thread::current()->is_Watcher_thread()
>>> As: http://mail.openjdk.java.net/pipermail/hotspot-runtime-dev/2
>>> 017-July/023863.html
>>>
>>> I saw that I have missed that crash protection doesn't work for the
>>> upcoming trace backend changes.
>>> Main reason for not seeing this is that the trace backend haven't
>>> crashed (yet).
>>>
>>> This code is not used in the open-jdk, compiled tested linux and windows.
>>>
>>> Bug: https://bugs.openjdk.java.net/browse/JDK-8183925
>>> Webrev: http://cr.openjdk.java.net/~rehn/8183925/webrev/index.html
>>>
>>
>> General comments
>>    - Please update copyright years as needed before pushing.
>>
>
> done
>
>
>> src/os/posix/vm/os_posix.hpp
>>      No comments.
>>
>> src/os/posix/vm/os_posix.cpp
>>      L1306:   assert(_protected_thread != NULL, "Cannot crash protect a
>> none Thread");
>>          Typo: 'none Thread' -> 'NULL thread'
>>
>>
> done
>
> src/os/windows/vm/os_windows.hpp
>>      No comments.
>>
>> src/os/windows/vm/os_windows.cpp
>>      L4842:   assert(_protected_thread != NULL, "Cannot crash protect a
>> none Thread");
>>          Typo: 'none Thread' -> 'NULL thread'
>>
>>
> done
>
>      Really don't like the variable named 'success', but that
>>      pre-dates these changes.
>>
>> src/os_cpu/bsd_x86/vm/os_bsd_x86.cpp
>> src/os_cpu/linux_aarch64/vm/os_linux_aarch64.cpp
>> src/os_cpu/linux_arm/vm/os_linux_arm.cpp
>> src/os_cpu/linux_s390/vm/os_linux_s390.cpp
>> src/os_cpu/linux_sparc/vm/os_linux_sparc.cpp
>> src/os_cpu/linux_x86/vm/os_linux_x86.cpp
>> src/os_cpu/solaris_sparc/vm/os_solaris_sparc.cpp
>> src/os_cpu/solaris_x86/vm/os_solaris_x86.cpp
>>      Same change to this set of files; no comments.
>>
>> src/share/vm/runtime/mutex.cpp
>>      L1392: assert(!os::ThreadCrashProtection::is_crash_protected(
>> thread),
>>      L1393:            "locking not allowed when crash protection is
>> set");
>>          nit - two extra spaces before '"' on L1393.
>>
>
> done
>
>
>> src/share/vm/runtime/os.cpp
>>      L576:   // checking for the WatcherThread and crash_protection first
>>      L579:   // try to find the thread after we see that the watcher
>> thread
>>          These comments mention "WatcherThread" and "watcher thread"
>>          so they need to be updated.
>>
>
> done
>
>
>>      L582:          "Can't malloc with crash protection from
>> WatcherThread");
>>          "from WatcherThread" should be "from crash protected thread"
>>          or something similar.
>>
>>          Or you could do something like the mutex.cpp mesg:
>>
>>          "locking not allowed when crash protection is set"
>>          "malloc() not allowed when crash protection is set"
>>
>
> done
>
>
>> src/share/vm/runtime/thread.hpp
>>      No comments.
>>
>> src/share/vm/runtime/thread.cpp
>>      No comments.
>>
>>
>> Thumbs up! I don't need to see another webrev if you make
>> the above minor changes...
>>
>
> Thanks you Dan!
>
> /Robbin
>
>
>> Dan
>>
>>
>>
>>> Thanks!
>>>
>>> /Robbin
>>>
>>
>>