RFR: 8183925: Decouple crash protection from watcher thread
Robbin Ehn
robbin.ehn at oracle.com
Fri Jul 7 13:22:44 UTC 2017
Thanks Coleen!
/Robbin
On 07/07/2017 02:25 PM, coleen.phillimore at oracle.com wrote:
> This looks good.
> Coleen
>
> On 7/7/17 2:59 AM, Robbin Ehn 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/2017-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
>>>
>
More information about the hotspot-runtime-dev
mailing list