RFR(S): 8020701: Avoid crashes in WatcherThread

[Rickard Bäckman]

On Jul 17, 2013, at 7:03 PM, Daniel D. Daugherty wrote:

> On 7/17/13 5:58 AM, Rickard Bäckman wrote:
>> Hi all,
>> 
>> can I please have reviews for the following change?
>> 
>> We are adding a mechanism for avoiding some crashes in the WatcherThread using different OS-specific methods.
>> 
>> Webrev: http://cr.openjdk.java.net/~rbackman/8020701/webrev/
>> 
>> Thanks
>> /R
> 
> Thumbs up! The bug mentions a few tests where you think this new
> infrastructure will help diagnose the failures modes for those tests.
> However, are there any new tests targeted to exercise this code?

Not diagnose the failures, but avoid the crashes they are currently causing.
Since at least one of them haven't been fixed yet (working on it) it will work as
a good verifier at the moment. When the bug is gone, it is harder. It is a bit hard
to write a test that causes the VM to crash at random but specific places. Eventually
we could do something with the whitebox api, but the crashes would only happen at
very specific places in that case making them a bit less interesting.

> 
> 
> src/share/vm/runtime/os.hpp
>    No comments.
> 
> src/share/vm/runtime/os.cpp
>    No comments.
> 
> src/share/vm/runtime/thread.hpp
>    line 756: void set_crash_protection(os::WatcherThreadCrashProtection* crash_protection) { assert(Thread::current()->is_Watcher_thread(), "Can only be set by WatcherThread"); _crash_protection = crash_protection; }
>        This line is ridiculously long. Please reformat it to fit in 80 cols.

Ok

> 
> src/share/vm/runtime/thread.cpp
>    No comments.
> 
> src/os/posix/vm/os_posix.hpp
>    Nice comment. Hopefully it will keep anyone from doing something
>    dangerous in the future.

:)

> 
> src/os/posix/vm/os_posix.cpp'
>    Please add the following line above 268:
> 
>    * See the caveats for this class in os_posix.hpp.

Ok

> 
> src/os/windows/vm/os_windows.hpp
>    No comments.
> 
> src/os/windows/vm/os_windows.cpp
>    line 4692  * Protects the callback call so that S jumps back into this method
>        Stale comment? What is 'S'?

typo. Should be something like raised EXCEPTIONS causes a jump back into this method.

> 
>    Please add the following line above 4692:
> 
>    * See the caveats for this class in os_windows.hpp.
> 
> src/os_cpu/bsd_x86/vm/os_bsd_x86.cpp
>    line 405:   // (no destructors run)
>        Please change to "(no destructors can be run)"

Ok

> 
> src/os_cpu/linux_sparc/vm/os_linux_sparc.cpp
>    line 557:   // (no destructors run)
>        Please change to "(no destructors can be run)"
> 
> src/os_cpu/linux_x86/vm/os_linux_x86.cpp
>    line 229:   // (no destructors run)
>        Please change to "(no destructors can be run)"
> 
> src/os_cpu/solaris_sparc/vm/os_solaris_sparc.cpp
>    line 319:   // (no destructors run)
>        Please change to "(no destructors can be run)"
> 
> src/os_cpu/solaris_x86/vm/os_solaris_x86.cpp
>    line 378:   // (no destructors run)
>        Please change to "(no destructors can be run)"
> 
> src/share/vm/runtime/mutex.cpp
>    No comments.
> 
> Dan
> 

Thanks for the review!

/R