JDK-6852342 : GC-a-lot feature should be NoHandleMark-aware
  • Type: Bug
  • Component: hotspot
  • Sub-Component: gc
  • Affected Version: hs15
  • Priority: P4
  • Status: Open
  • Resolution: Unresolved
  • OS: generic
  • CPU: generic
  • Submitted: 2009-06-17
  • Updated: 2019-02-11
The Version table provides details related to the release that this issue/RFE will be addressed.

Unresolved : Release in which this issue/RFE will be addressed.
Resolved: Release in which this issue/RFE has been resolved.
Fixed : Release in which this issue/RFE has been fixed. The release containing this fix may be available for download as an Early Access Release or a General Availability Release.

To download the current JDK release, click here.
Other
tbdUnresolved
Related Reports
Relates :  
JDK-6928562 - Assert(_no_handle_mark_nesting==0,"allocating handle inside NoHandleMark"
Relates :  
JDK-6820167 - GCALotAtAllSafepoints + FullGCALot(ScavengeALot) options crash JVM
Description
There are portions of execution of threads during which they vow not to
allocate in the heap or to create handles. These are specific small
portions of code that are understood to not invoke GC or safepoints
on their own so the thread can operate at full speed, and as such are
marked by a NoHandleMark() or NoSafepointVerifier() scope.
It appears as though the GC-a-lot feature may occasionally run
afoul of these blocks, so the interaction of these two should
be investigated and cleaned up.

See comments section for more data.
Comments
EVALUATION Yes.
18-06-2009
SUGGESTED FIX The following changes are under test and review; some further small changes are possible before this integrates (subject to review of course). --- old/src/share/vm/memory/gcLocker.hpp Thu Jun 18 10:18:48 2009 +++ new/src/share/vm/memory/gcLocker.hpp Thu Jun 18 10:18:48 2009 @@ -242,31 +242,6 @@ #endif }; -// A SkipGCALot object is used to elide the usual effect of gc-a-lot -// over a section of execution by a thread. Currently, it's used only to -// prevent re-entrant calls to GC. -class SkipGCALot : public StackObj { - private: - bool _saved; - Thread* _t; - - public: -#ifdef ASSERT - SkipGCALot(Thread* t) : _t(t) { - _saved = _t->skip_gcalot(); - _t->set_skip_gcalot(true); - } - - ~SkipGCALot() { - assert(_t->skip_gcalot(), "Save-restore protocol invariant"); - _t->set_skip_gcalot(_saved); - } -#else - SkipGCALot(Thread* t) { } - ~SkipGCALot() { } -#endif -}; - // JRT_LEAF currently can be called from either _thread_in_Java or // _thread_in_native mode. In _thread_in_native, it is ok // for another thread to trigger GC. The rest of the JRT_LEAF --- old/src/share/vm/runtime/handles.cpp Thu Jun 18 10:18:50 2009 +++ new/src/share/vm/runtime/handles.cpp Thu Jun 18 10:18:49 2009 @@ -156,8 +156,10 @@ #ifdef ASSERT -NoHandleMark::NoHandleMark() { - HandleArea* area = Thread::current()->handle_area(); +NoHandleMark::NoHandleMark() : + _t(Thread::current()), + _skip_gc_a_lot(_t) { + HandleArea* area = _t->handle_area(); area->_no_handle_mark_nesting++; assert(area->_no_handle_mark_nesting > 0, "must stack allocate NoHandleMark" ); } @@ -164,13 +166,15 @@ NoHandleMark::~NoHandleMark() { - HandleArea* area = Thread::current()->handle_area(); + HandleArea* area = _t->handle_area(); assert(area->_no_handle_mark_nesting > 0, "must stack allocate NoHandleMark" ); area->_no_handle_mark_nesting--; } -ResetNoHandleMark::ResetNoHandleMark() { +ResetNoHandleMark::ResetNoHandleMark() : + _t(Thread::current()), + _no_skip_gc_a_lot(_t) { HandleArea* area = Thread::current()->handle_area(); _no_handle_mark_nesting = area->_no_handle_mark_nesting; area->_no_handle_mark_nesting = 0; --- old/src/share/vm/runtime/handles.hpp Thu Jun 18 10:18:51 2009 +++ new/src/share/vm/runtime/handles.hpp Thu Jun 18 10:18:51 2009 @@ -328,10 +328,51 @@ }; //------------------------------------------------------------------------------------------------------------------------ +// A SkipGCALot object is used to elide the usual effect of gc-a-lot +// over a section of execution by a thread. Currently, it's used only to +// prevent re-entrant calls to GC. +class SkipGCALot : public StackObj { + private: + bool _saved; + Thread* _t; + + public: +#ifdef ASSERT + SkipGCALot(Thread* t); + ~SkipGCALot(); +#else + SkipGCALot() { } + SkipGCALot(Thread* t) { } + ~SkipGCALot() { } +#endif +}; + +// A NoSkipGCALot object is used to elide the effect of SkipGCALot +// over a section of execution by a thread. Currently, it's used only within +// a ResetNoHandleMark to re-enable gc-a-lot within an outer HandleMark. +class NoSkipGCALot : public StackObj { + private: + bool _saved; + Thread* _t; + + public: +#ifdef ASSERT + NoSkipGCALot(Thread* t); + ~NoSkipGCALot(); +#else + NoSkipGCALot() { } + NoSkipGCALot(Thread* t) { } + ~NoSkipGCALot() { } +#endif +}; + +//------------------------------------------------------------------------------------------------------------------------ // A NoHandleMark stack object will verify that no handles are allocated // in its scope. Enabled in debug mode only. class NoHandleMark: public StackObj { + Thread* _t; + SkipGCALot _skip_gc_a_lot; public: #ifdef ASSERT NoHandleMark(); @@ -344,7 +385,9 @@ class ResetNoHandleMark: public StackObj { - int _no_handle_mark_nesting; + Thread* _t; + NoSkipGCALot _no_skip_gc_a_lot; + int _no_handle_mark_nesting; public: #ifdef ASSERT ResetNoHandleMark(); --- old/src/share/vm/runtime/handles.inline.hpp Thu Jun 18 10:18:53 2009 +++ new/src/share/vm/runtime/handles.inline.hpp Thu Jun 18 10:18:53 2009 @@ -71,3 +71,26 @@ NOT_PRODUCT(area->set_size_in_bytes(_size_in_bytes);) debug_only(area->_handle_mark_nesting--); } + + +#ifdef ASSERT +inline SkipGCALot::SkipGCALot(Thread* t) : _t(t) { + _saved = _t->skip_gcalot(); + _t->set_skip_gcalot(true); +} + +inline SkipGCALot::~SkipGCALot() { + assert(_t->skip_gcalot(), "Save-restore protocol invariant"); + _t->set_skip_gcalot(_saved); +} + +inline NoSkipGCALot::NoSkipGCALot(Thread* t) : _t(t) { + _saved = _t->skip_gcalot(); + _t->set_skip_gcalot(false); +} + +inline NoSkipGCALot::~NoSkipGCALot() { + assert(!_t->skip_gcalot(), "Save-restore protocol invariant"); + _t->set_skip_gcalot(_saved); +} +#endif // ASSERT --- old/src/share/vm/runtime/thread.hpp Thu Jun 18 10:18:55 2009 +++ new/src/share/vm/runtime/thread.hpp Thu Jun 18 10:18:54 2009 @@ -48,8 +48,12 @@ // Class hierarchy // - Thread +// - NamedThread +// - WorkerThread // - VMThread // - JavaThread +// - CompilerThread +// - LowMemoryDetectorThread // - WatcherThread class Thread: public ThreadShadow { @@ -189,10 +193,10 @@ // The two classes No_Safepoint_Verifier and No_Allocation_Verifier are used to set these counters. // NOT_PRODUCT(int _allow_safepoint_count;) // If 0, thread allow a safepoint to happen - debug_only (int _allow_allocation_count;) // If 0, the thread is allowed to allocate oops. + DEBUG_ONLY (int _allow_allocation_count;) // If 0, the thread is allowed to allocate oops. // Used by SkipGCALot class. - NOT_PRODUCT(bool _skip_gcalot;) // Should we elide gc-a-lot? + DEBUG_ONLY(bool _skip_gcalot;) // Should we elide gc-a-lot? // Record when GC is locked out via the GC_locker mechanism CHECK_UNHANDLED_OOPS_ONLY(int _gc_locked_out_count;) @@ -311,7 +315,7 @@ bool is_gc_locked_out() { return _gc_locked_out_count > 0; } #endif // CHECK_UNHANDLED_OOPS -#ifndef PRODUCT +#ifdef ASSERT bool skip_gcalot() { return _skip_gcalot; } void set_skip_gcalot(bool v) { _skip_gcalot = v; } #endif
18-06-2009