RFR: 8183925: Decouple crash protection from watcher thread

[Robbin Ehn]

Thanks Thomas!

/Robbin

On 07/08/2017 08:28 AM, Thomas Stüfe wrote:
> Hi Robbin,
> 
> Change looks fine.
> 
> Kind Regards, Thomas
> 
> On Fri, Jul 7, 2017 at 8:59 AM, Robbin Ehn <robbin.ehn at oracle.com <mailto: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/2017-July/023863.html <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 <https://bugs.openjdk.java.net/browse/JDK-8183925>
>             Webrev: http://cr.openjdk.java.net/~rehn/8183925/webrev/index.html <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
> 
> 
>